Fix several issues with the GroupedListWidget

[criezy]

This pull request fixes the following issues:

• Failure to select some items (which later caused a crash) when grouping items, some groups are closed, and no filter is used. (bug #15959).
• Filter not being applied when rebuilding the game list (e.g. after closing the game options), while using a grouping criteria and a filter is defined.
• Losing the current selection when changing the filter or changing the grouping criteria.
• Start Game and Game Options buttons remaining enabled in the Launcher despite deselecting the currently selected game when opening or closing a group. This was fixed by selecting the first item in the group when opening a group, and trying to preserve the previous selection when closing a group. I am not sure about this change however. As we click on a group header in the list we may indeed expect other items to be deselected. So I could revert that last commit and instead add a call to sendCommand(kListSelectionChangedCmd, -1); to properly disable the Start Game and Game Options buttons after deselecting the item. Let me know if you would prefer that approach.

This also cleans a little bit the function to set the selected item in the GroupedListWidget.

[bluegr]

Thank you. Could you please fix the conflicts, which were introduced after the merge of PR #6701?

[sluicebox]

What timing! Hope my PR didn't cause too much trouble. I'm looking forward to testing this out; it looks like great improvements.

[criezy]

The conflict has been fixed.

What timing! Hope my PR didn't cause too much trouble.

Yes, I got the crash I had reported earlier again last night and finally decided to bite the bullet and debug it, and I fell into a rabbit hole. Fortunately the two PRs complemented each others well and there was only one small conflict easily resolved.

[sluicebox]

Looks good! One issue that came up when testing is that closing a group now scrolls to the selection, causing jumping that didn't occur before:

1. Select game
2. Scroll so that the game is not visible
3. Close a group -- list scrolls back to selected game, hiding the group you closed

[criezy]

Looks good! One issue that came up when testing is that closing a group now scrolls to the selection, causing jumping that didn't occur before:

This issue is related to the last bullet point in the PR description and the main reason why I wrote that "I am not sure about this change however".

The last commit in this pull request changes the behaviour when opening or closing a group:

• When opening a group it selects the first item in this group.
• When closing a group it tries to preserve the selection (this succeeds if the selection is not in the group being closed). And this indeed causes the jump you noticed if the selected item is not visible.

In current master, opening or closing a group clears the selection. It forgets to update the launcher button state (e.g. disable the Start Game button), but that could easily be fixed. And as we click on a header item to close or open a group, deselecting the previously selected item seems a perfectly reasonable behaviour. So we could decide to drop my last commit and instead fix the missing launcher button state update. Or we could use only part of that commit.

As I see it we have several options:

1. Clear the selection when opening or closing a group. This is the current behaviour in master.
2. Select the first item in the group when opening a group, and clearing the selection when closing a group.
3. Select the first item in the group when opening a group, and preserve the selection if possible when closing a group. This is the current behaviour in this pull request.
4. Same as 3., but remove the jump to the selected item.

[bluegr]

@criezy From the available options, I believe that the second one makes more sense, i.e:
2. Select the first item in the group when opening a group, and clearing the selection when closing a group.

Preserving the selection when closing a group will be confusing, as the user will no longer view that selection. Selecting the first item in a group when opening it sounds like a good idea

[lephilousophe]

I think that 3 means preserving the selection if it's not in the closed group.
I'd vote on 4

[i30817]

Losing the current selection when changing the filter or changing the grouping criteria.

thank you, this drives me crazy and it's so common in programs that implement filters or multiple playlists (eyes RetroArch).

[sluicebox]

I think that in general, opening and closing a group should not affect selection or scrolling, because they're unrelated operations. (Also, when in doubt, pull up a file browser in your operating system and see how it behaves; they're all pretty much in agreement over these basics.)

Some exceptions that seem reasonable:

• Opening a group can select the first item if there's no current selection.
• Closing a group can change the selection if the group contains the selected item. (It probably has to, so either clear it or select the next available item.)

A common theme in the GUI issues I encounter here are unexpected side effects and uncommented opinions that once they get discovered and discussed are recognized as odd. I think "do less" is a good GUI guideline, and only "do more" when it can be justified in comments. (If a GUI choice can't be easily explained, that's a sign!)

I'm not a user of the grouped list though, so weigh my opinions accordingly. I may start using it, grouping is great. But I don't understand the point of being able to open and close groups in our GUI; that feels like a feature in search of a use case. I spent my time in the list selecting games, not sure what the rest of you are up to =)

[i30817]

Admiring the game list mostly. More seriously, Ags has like, a thousand freeware games and it's fairly easy to accumulate a collection over the years. Ags is also one of the first engines on that category list, so if you want to check out anything else, it's much easier to collapse the category than scroll down...

Personally I'd prefer the year of release category if only there weren't so many games without one. It's also a bit funny when the name itself includes a date and it's much later (because it's a later version than the actual first release, some of the king's quest games versions are like this I think)

[criezy]

Thank you all for your feedback. Indeed following what is normally done in tree views is a good idea to avoid breaking users expectations. And the standard behaviour is to preserve the selection both when closing and opening a group. So once I am back at my computer I will change that last commit to:

• Keep the current selection when closing or opening a group (unless the current selection is in the group we are closing as we cannot keep the selection in this case).
• Remove the jump to the current item in this case.

We could tweak that behaviour for the case were no game was selected before we open a group to select the first game in that group. I am not however sure this would be a good idea as this would deviate from the standard behaviour and also mean that when we open a group it would sometimes select the first game in the group and sometimes not, which could be a bit confusing.

[lephilousophe]

On KDE, collapsing a tree, clears the selection if the entry disappears. Else it is kept.
On Windows (explorer), it seems to keep the selected collapsed entry and highlights the parent one, but it may be because the list view on the right displays the contents of the selected folder in the tree view.

[criezy]

The Qt QTreeView also keeps the selected collapsed entry (but does not select the parent). But in our case here selecting the parent makes no sense (we can only select games) and keeping the selection of a collapsed entry would be complicated as the selection is tracked using the index of listed items (which the collapsed entry is no longer part of). So the behaviour I pushed clears the selection in this case. But otherwise it now tries to preserve the selection.

I will wait for all the checks to complete, but I think it is good to merge now.