Batch Update: Use UI file & UX Improvements

I know I already have a meaty enough PR open for a UI change (#319), and that soon I'll be taking a look at another CtInfo dialog change for anti-cheat runtimes (#186) so no hurry to review this. I wanted to get it up and document the changes here while it was all fresh in my head.

Overview

This PR moves the Batch Update dialog to use a Qt UI file and makes a couple of UX tweaks. It's a similar vein to what I did for Custom Install Directory in #225.

Add a short label describing what Batch Update does (feedback very welcome on phrasing).
Remove the current Proton version from the dropdown list, i.e. if we're using GE-Proton8-25 and want to batch update, don't display GE-Proton8-25, as it doesn't make sense to batch update from the current tool, to the current tool.
Disable the dropdown if no compatibility tools to migrate to are found. For example if only GE-Proton8-25 is installed, we can't migrate to any other versions, so disable the dropdown.
Disable the "Batch Update" button on the dialog itself if no compatibility tools to migrate to are found.
- This is not disabled for the "Close the Steam Client beforehand" warning message because the user can do that on most systems without closing ProtonUp-Qt. Even though if Steam is open, the batch update will be overwritten, the Steam Client can be closed while this dialog is open on the Linux Desktop, and it should prompt users to close the Steam Client. We don't prevent users from making changes in the Games List when Steam is running, so I don't think we have to prevent them in Batch Update :-)
- This is not disabled on the ctinfo dialog as we'd have to fetch the installed compatibility tool info here which seems a little excessive just to disable a button, and also it may not be obvious without a tooltip why this is disabled (and there's no space anywhere for a label on this dialog really). Instead, we let the user open the dialog, and show warnings as below.
- This may also create some slightly tricky logic paths / conflicts with #319.
- This is also not disabled on ctinfo dialog as the above implementation would probably be necessary, and consider the following scenario: User has only one GE-Proton version and no other supported batch update tools -> sets new GE-Proton tool to download -> Opens ctinfo -> Realizes tool is downloading, decides to wait -> Tool downloads, and if the button is disabled they would have to re-open the dialog, or if not disabled but we pass a prefetched tool list to Batch Update, they open Batch Update screen -> it's missing info about the new tool, so they get the "No compatibility tool" message. Fetching information in the Batch Update dialog means we always have the latest tool information, unless they open Batch Update while a tool download is in progress.
  - In that scenario, if we fetched from ctinfo, since it remains open, a user would have to close the Batch Info and ctinfo dialog, whereas if the info is fetched in Batch Update, the user only has to close and re-open one dialog.
- We would have a similar problem if we passed information about all installed compat tools from the Main Window to ctinfo and then to Batch Update, as the installed tool info would still be out-of-date in the above scenario since the information gotten from Main Window and sent to ctinfo would still not have that tool that is currently downloading.
- We would run into similar problems when tools are removed.
Show a red warning label when no compatibility tool to migrate to is installed, to complement the disabled dropdown and disabled update button. This should help make it clear why the option is disabled.
Add a label to display the "old version", aka the current compatibility tool version that we're migrating from. When running in GameScope this is hopefully an extra bit of UX when the user can't look at the dialog behind very easily, it should provide a little bit of visual feedback to confirm what they're currently using and what they're moving to.
- We have this already because we have to pass the internal name of the current tool from ctinfo to the batch update dialog, in order to create the filtered list of installed versions excluding the current one, to allow the disabled dropdown.
Add dedicated "Close" button to be consistent with the rest of the UI and to allow easily closing the dialog in instances where the titlebar may not be visible (i.e. GameScope).
Adjust the dialog size slightly to accommodate these changes.
Probably other extremely minor UI sizing/layout tweaks I'm forgetting :sweat_smile:

I think these changes make the Batch Update dialog a little bit more user-friendly. I have provided some sample screenshots below that I hope will show the improvements.

Implementation

Fetching the Compatibility Tool List

As noted above, we fetch the list of installed compatibility tools in the Batch Update dialog. But to allow the dropdown to be disabled if no other supported tools are installed, we have to know the current ctool name. We can get this from the displayname of the ctinfo dialog, since Batch Update is called from that dialog we already have that compatibility tool context.

I also changed how we build the dropdown elements, using a list and calling addItems. The filtering is done in a list comprehension instead of in the for loop.

All these changes should have a negligible performance impact.

Warning Labels

I created a helper method to generate warning labels. This creates a QLabel and adds it to a FormLayout, which the Batch Update dialog uses. We give this as an argument to add_warning_message but we could hardcode it to just use self.ui.formLayout. Finally we also pass a stylesheet, because the Steam Running warning uses orange text since it's a warning that won't prevent the user from performing the batch update, but the missing compatibility tools message uses red text since we do prevent the user from performing the batch update. Taking an optional stylesheet made this simpler. Finally we of course give this method the text to use.

These labels are centered solely because I thought it looked nicer. I also used a full stop on both labels because it's there for the Steam Running warning on main, but it could be taken out if it's not necessary. On the Games List we use an exclamation mark, maybe we could do that here too :slightly_smiling_face:

These labels are generated and added programmatically in an if/elif/else block, because it doesn't make sense imo to show both at once. The "no compatibility tool" message takes priority because it doesn't matter if Steam is running or not, Batch Update cannot be performed. The user would have to close the dialog anyway to install the new tool or to update the list of compatibility tools if a new one was added while the dialog was open. That's why only one is shown at a time, and why the No Compatibility Tools message takes precedence. I hope I explained that well enough.

Doing things in this way also made managing the layout easier. The label row is "structurally important" to the UI, which I'm not very pleased about, but it was better than fitting it into the bottom left and making the dialog wider just to accommodate that label. Putting the labels here would be less natural than it is for the Games List which puts the Steam Running label here. It might also be tricky, as we would have to programmatically set the label text, as having both labels at once would still lead to "structurally important" label placement.

On the subject of structurally important labels, that's why we have the "spacer label" in the else block. It allows for that bit of spacing between the form layout and the Batch Update and Close buttons, which imo it looks better to have the space there than under the buttons.

Comparison

Main Branch

This PR

No Supported Compatibility Tools

Steam Client Running

Happy Migration Path

Considerations

I have a couple of thoughts on the current state of this PR. While everything is functional there are some points that might merit discussion :slightly_smiling_face:

Batch Update is not disabled if no games are installed because we're handling that differently in #319.
Not sure if "Batch Update" should be right-aligned with the "Close" button or if we should put it on the left. I put it on the right to match the general "Confirm/Cancel" layout, to visually differentiate it from the "Batch Update" button on the ctinfo dialog behind it (see screenshot below), and to match what we do in other dialogs (i.e. Custom Install Directory).
Related to the above, should the text still say "Batch Update" or should we change it to something more 'standard' such as "Ok" or "Confirm". Likewise, perhaps "Close" should say "Cancel" like how the Games List dialog says when a modification is made?
If there are tools to batch update to, should we default to a compatibility tool at all, or set the default entry to something like -- Select --? Then, we can disable the Batch Update button if no tool is selected. But for UX we would only add this if
- This is not consistent with other dropdowns such as install tool, but it is consistent with having the "-" option in the Games List when selecting a compatibility tool. The Custom Install Directory dialog defaults a dropdown value but that's fine because it can't be added without a directory, which is blank by default, so its UX is different to the install dialog and Games List.
I am unsure if the "Old Version" label in Batch Update should be the 1st or 2nd row. On one hand, "logically" the ordering makes sense, but since the user interacts with the dropdown and not the "old version" label, it may make more sense to have this on the 1st row...
All feedback on label phrasing is welcome! I'm not super attached to any of the wording so anything you think is an improvement can be changed.
Super duper mega semantic: Should we camel case the "Batch update" button in ctinfo? :sweat_smile:

Here is the screenshot to illustrate how the "Batch Update" button on the ctinfo dialog and Batch Update dialog are more visually distinct by having the Batch Update's button on the right:

I think that covers it. I hope these are good improvements, in my opinion it makes the UI a bit cleaner and feel a bit more consistent with the rest of ProtonUp-Qt, and gives some practical advantages such as being able to close the dialog with a dedicated button. But I made the PR so of course I'm a little biased :wink:

Please let me know if you have any other feedback on these changes or any further changes that should be made while I'm poking around this part of the code.

Thanks! :-)

Made a small phrasing change to the description text and updated the screenshots accordingly.

I'm wondering if we should add a small horizontal line between the description text and the form layout. We don't do this on the Custom Install Directory, but since we don't have any text labels, the distinction is more obvious. But since the first element in the form layout here is a label, the text can kind of blend together.

If we move the dropdown to be the first element, it would fix this problem, otherwise a horizontal rule may help a little. We'd have to slightly increase the dialog height by 10 to accomplish this though. I did a quick implementation in Qt Designer and to be honest I'm not sure what the best approach is.

Horizontal Rule

New Version being 1st Row (No Horizontal Rule)

New Version being 1st Row (With Horizontal Rule)

Not sure if adding this line, moving the "New Version" row to be the 1st row, both, or no change at all is the best approach :sweat_smile: Also, having "New Version" as the first row throws me off slightly, since the width of the "Old Version:" label is slightly shorter, so the labels go in descending order of width instead of ascending order, which in this instance I find the latter (current implementation in this PR) more aesthetically pleasing.

If we don't want a horizontal rule another potential change is to make the "Old/New" version labels bold, although I don't think I like this change much compared to the above proposal. It would be inconsistent and a bit too "loud". A horizontal line is much more subtle imo.

It's late and maybe I'm getting too deep in a rabbit hole of minor UI details but any input is greatly appreciated :smile:

Thanks!

Disable the "Batch Update" button on the dialog itself if no compatibility tools to migrate to are found.

Aren't we already doing that with self.ui.btnBatchUpdate.setEnabled(len(self.games) > 0) in the Ct Info Dialog?

I also changed how we build the dropdown elements, using a list and calling addItems. The filtering is done in a list comprehension instead of in the for loop. All these changes should have a negligible performance impact.

I'd think that it improves the performance as this reduces the amount of calls to the Qt bindings.

it doesn't make sense imo to show both at once. The "no compatibility tool" message takes priority because it doesn't matter if Steam is running or not, Batch Update cannot be performed.

That makes sense.

Not sure if "Batch Update" should be right-aligned with the "Close" button or if we should put it on the left.

I think it's fine like it is now. It also matched the install dialog where the "install" button is also right-aligned.

Related to the above, should the text still say "Batch Update" or should we change it to something more 'standard' such as "Ok" or "Confirm".

I think we can also leave this as is. Makes it more obvious when only skimming the text.

If there are tools to batch update to, should we default to a compatibility tool at all, or set the default entry to something like -- Select --?

Hmm. I think currently it shows the first compatibility tool sorted alphabetically which results in showing the newest one, which is fine I guess.

I am unsure if the "Old Version" label in Batch Update should be the 1st or 2nd row. On one hand, "logically" the ordering makes sense, but since the user interacts with the dropdown and not the "old version" label, it may make more sense to have this on the 1st row...

I think the user would read the dialog from top to bottom, so having everything informative first and then the options is good, the exception here being the warning text. But that shouldn't matter too much as it is in a highlighting color.

All feedback on label phrasing is welcome! I'm not super attached to any of the wording so anything you think is an improvement can be changed.

I guess it is fine :)

Super duper mega semantic: Should we camel case the "Batch update" button in ctinfo? 😅

If you mean writing both words uppercase like Pascal case but with a space inbetween "Batch Update", that's fine. I guess it would make sense as "Batch Update" isn't a sentence. :smile:

Here is the screenshot to illustrate how the "Batch Update" button on the ctinfo dialog and Batch Update dialog are more visually distinct by having the Batch Update's button on the right:

Yes, look quite distinctive, then let's leave it like this.

I hope these are good improvements, in my opinion it makes the UI a bit cleaner and feel a bit more consistent with the rest of ProtonUp-Qt, and gives some practical advantages such as being able to close the dialog with a dedicated button. But I made the PR so of course I'm a little biased 😉

Yeah that's very good indeed, thanks. The close button will also help on the Steam Deck or if the CSD breaks again ...

I'm wondering if we should add a small horizontal line [...]

Hmm, not sure if a separating line belongs there. I guess it looks a bit cleaner, but it could give the user the incentive to skip the text.

Aren't we already doing that with self.ui.btnBatchUpdate.setEnabled(len(self.games) > 0) in the Ct Info Dialog?

Note quite, this is for the compatibility tools. Say we're using GE-Proton8-25, and we have 10 games using that tool. We can still press "Batch Update", but if we only have that tool installed, and no other custom tools, then there are no tools to Batch Update to. The dropdown would show the current tool on main, but this PR removes that, so the dropdown on batch update would be empty.

We don't disable the "Batch update" button on the CtInfo dialog if no other compatibility tools to batch update to are found. I considered making that change but figured there was no need to make the change there, CtInfo in most cases doesn't need to be aware of other installed compatibility tools. But the Batch Update dialog will be aware of those tools and will be fetching them anyway, so I figured it makes more sense to still allow showing the dialog, but disabling the dialog functionality, with a warning message explaining why the action cannot be performed. That way a user can see what the issue is instead of the button on the CtInfo dialog mysteriously disappearing/reappearing or sometimes being enabled/disabled, it's a more visual way for a user to know why the action cannot be performed. And it means we don't have to make the CtInfo dialog aware of other installed tools, we have to fetch them for Batch Update anyway so we just keep the logic on the Batch Update dialog instead of introducing new logic to check installed compatibility tools in the CtInfo dialog.

Sorry if I didn't explain it right but the change I made was to disable "Batch Update" on the Batch Update dialog if there are no other tools to move the games to.

I think it's fine like it is now. It also matched the install dialog where the "install" button is also right-aligned.

Sounds good to me :+1:

I think the user would read the dialog from top to bottom, so having everything informative first and then the options is good, the exception here being the warning text. But that shouldn't matter too much as it is in a highlighting color.

All good then :smile: Happy to leave it as-is.

If you mean writing both words uppercase like Pascal case but with a space inbetween "Batch Update", that's fine. I guess it would make sense as "Batch Update" isn't a sentence. 😄

Heh, this was super unimportant I know, but I like being thorough and getting as much feedback as possible.

Hmm, not sure if a separating line belongs there. I guess it looks a bit cleaner, but it could give the user the incentive to skip the text.

Ah that's a good point, maybe we can just leave it as-is then (no changes for it were pushed to this PR, I made those changes quickly and took a screenshot, didn't commit them).

Thanks for the responses as always, I'll take a look at addressing some of the feedback now :-)

Okay, think I got all the feedback addressed:

CtInfo: Batch update -> Batch Update
Batch Update: Fix the translation logic by moving where we call self.tr and how we create the message label
CtInfo: Remove unused imports

Note quite, this is for the compatibility tools

Ah, that makes sense. I didn't consider that there are two different things we are dealing here.

Heh, this was super unimportant I know, but I like being thorough and getting as much feedback as possible.

Making ProtonUp-Qt look as professional as possible isn't that unimportant :smile:

Okay, think I got all the feedback addressed:

Thanks! I will merge this now.

DavidoTek / ProtonUp-Qt