dolphin icon indicating copy to clipboard operation
dolphin copied to clipboard

Qt: Show currently configured Memory Card path in the config window.

Open AdmiralCurtiss opened this issue 2 years ago • 16 comments

More convenience stuff.

Current state: No textbox is shown for options that don't have a path to configure. If a memory card is selected, the textbox is only shown if the path is non-default. If AGP is selected, the textbox is always shown.

The region shown is whatever the user has selected last if available, which I suspect is the least confusing option.

If you try to manually write or paste an invalid path (missing region, already in other card, whatever) it will revert your change.

AdmiralCurtiss avatar Jun 16 '22 00:06 AdmiralCurtiss

Just tested this and although I'm not a fan of UI controls popping out of nowhere (e.g. depending of what you choose from the dropdown list), I particularly can't think of other approaches, taking into consideration the previous attempts from PR #10379 didn't turn out objectively better either.

Speaking of PR #10379, could you also bring back GCI folder path but with the current design? It was available on that PR but it's gone on this one. Other than that, this LGTM...

mbc07 avatar Jun 16 '22 08:06 mbc07

I intentionally didn't include the GCI stuff here because I figured it would be easier to review and merge those separately. It'll come, don't worry.

AdmiralCurtiss avatar Jun 16 '22 11:06 AdmiralCurtiss

So I'm totally fine with this idea, but the UI is definitely a little rough. So after pondering it a bit... why not use our cool tooltips? We already have a system in place for nice tooltips. You could just put this information into a tooltip! No GUI changes would even be required and yet the information will be readily available.

The main functionality change to this idea is that you can't copy paste from a tooltip. So it would exclusively be showing the user the location. What are your thoughts on that?

MayImilae avatar Jun 16 '22 13:06 MayImilae

I think having the ability to copy-paste the path is preferable to not having it, but if clicking the ... opens the dialog in the current location of the card anyway it's still accessible that way. So sure, why not.

AdmiralCurtiss avatar Jun 16 '22 13:06 AdmiralCurtiss

I don't feel like these tooltips are designed for this purpose -- they explicitly want a 'title' and a 'description' -- so this probably needs some fixups to actually look nice.

AdmiralCurtiss avatar Jun 16 '22 14:06 AdmiralCurtiss

Also seeing this right next to the GBA Settings at the bottom there makes me think this is the wrong approach, it's really weird to have two completely different design paradigms for file paths in the same window.

AdmiralCurtiss avatar Jun 16 '22 14:06 AdmiralCurtiss

I don't think tooltips are suitable in that scenario, they're actually worse than the path box showing/hiding out of nowhere depending of the drop-down menu selection and not consistent with anything else from the GameCube tab.

On Windows at least, it would also unnecessarily introduce more steps to perform the same task (copying the file path): you would need to click on the ellipsis button, then right-click the desired memory card file while holding the Shift key in order to be able to select the "copy file path" option in the context menu from the file selection dialog, compared to just selecting it on Dolphin's GUI and using Ctrl+C/V.

Once GCI folder path gets reimplemented, the tooltip approach would fall apart even more...

mbc07 avatar Jun 16 '22 20:06 mbc07

it would also unnecessarily introduce more steps to perform the same task (copying the file path)

But is that the user's goal?

MayImilae avatar Jun 17 '22 05:06 MayImilae

From my understanding, the motivation behind the first implementation in PR #10379 (UX wise), that eventually evolved into this one, was to more prominently expose the Memory Card path in the GUI, as for some users it wasn't exactly clear that it could be changed from within the GUI (see issue 12169).

With that in mind, why should we go with a less functional approach (tooltips -- not used for that particular purpose anywhere else), when we already have a more functional approach (input box) already established and used literally everywhere else we expose similar functionality in the GUI? (to name a few: stuff from Paths tab, GBA stuff already present on the GameCube tab, Memory Card paths on Memory Card manager)

Another point: If I can already see the path of several user configurable settings and easily edit/copy/paste them without extra steps (hovering over a button, or opening the file selection dialog to be able to copy the path), I kinda expect to be able to do the same with any other user configurable path that gets exposed in the GUI now or in the future, due to the already set precedents. If we go with tooltips, that would make this setting the only exception, with actually less functionality.

So, TL;DR, while the input box showing/hiding out of nowhere isn't exactly ideal, I still strongly think it's objectively better (both in functionality and also in consistency with similar controls already present on Dolphin's GUI) than anything currently possible with our custom tooltips.

mbc07 avatar Jun 17 '22 06:06 mbc07

If the big issue here is really the popping in and out (I personally don't mind it but hey) we can easily just make the field visible but greyed out if an option without a path to configure is currently selected. Though I will note that this is not unprecedented -- the Audio tab already does pop in a new field if you switch to WASAPI.

AdmiralCurtiss avatar Jun 17 '22 11:06 AdmiralCurtiss

Like so.

...I guess we could align those better, but you know what I mean, yeah?

AdmiralCurtiss avatar Jun 17 '22 11:06 AdmiralCurtiss

There. That looks nice, doesn't it?

AdmiralCurtiss avatar Jun 17 '22 11:06 AdmiralCurtiss

To be clear, the "show/hide controls based on a dropdown selection" pattern isn't exactly pretty but it's fine, it's also used on Emulated Wiimote Settings (extension setting) in addition to the Audio tab. The new pattern you proposed (enable/disable control based on dropdown selection) is also perfectly fine and prominently used on several other places in Dolphin's GUI as well. Both LGTM.

(I'm only against the tooltip approach as suggested by May, as I don't think they're a good candidate for what this PR wants to accomplish)

mbc07 avatar Jun 17 '22 22:06 mbc07

Does someone want to review this, code-wise? I'd like to get this in and JMC said he's fine with the UI.

AdmiralCurtiss avatar Jul 10 '22 19:07 AdmiralCurtiss

So looking at the code and my own experience testing it just now, it appears that this memory card path line is displayed for all users. If that is true, then I'm not ok with this PR as it is right now. I think this is serving too niche of an audience to be worth the GUI clutter cost for everyone else. Like, realistically, how much of our userbase changes the default memory card configuration? Generously, we're talking single percentage points - outside of absurd gaming collections ala JMC no one has any reason to touch it. How many of those users will move the memory card out of the Dolphin folder? A few percentage points of that. And then a fraction of those will want ready access to the memory card path? Well a larger percentage, but a percentage of a percentage of a percentage. The users who will benefit from this will be a fraction of a percent.

This is something that is only useful to a fraction of a percent of users, and it's cluttering up an already pretty cluttered section of our GUI. In my opinion as a designer, in the current state where everyone sees this path addition, this is not worth the GUI clutter. This is way too niche.

So here is my proposal to address this - if someone changes the memory card path out of the Dolphin global user directory, make this path appear. That's it really. That way, the users who will benefit from it will see this. And no one else will. Simple.

MayImilae avatar Jul 11 '22 08:07 MayImilae

Frankly I don't understand it but sure, that's easy enough to do.

AdmiralCurtiss avatar Jul 11 '22 20:07 AdmiralCurtiss

This has been sitting here forever with no comments, I'm just gonna merge this...

AdmiralCurtiss avatar Sep 08 '22 19:09 AdmiralCurtiss