Improve library sidebar UX (right-click and selection after add, rename, delete, duplicate etc.)

[ronso0]

I'd like to fix the last annoyance with the current selection/activation behaviour.

• if you right-click an unselected item it is visually selected but not activated (i.e. not loaded to the tracks table). So the first inconsistency is that the item of the actually loaded content is not selected anymore.
• the right-clicked item is the new starting point for navigation, very annoying if you were in "Tracks" and just want to clean up playlists or the history at the bottom of the sidebar)
• any newly created (or duplicated) playlist or crate is selected. What for? Why should I look at an empty tracks table with no option to add tracks, this is only possible from within other track views (ignoring drag'n'drop from decks here).

My proposal:

• show a focus border on right-clicked items (as long as sidebar has focus, i.e. until menu receives focus when using the keyboard/controller to navigate)
• only select items after the track menu actions if they were previously selected, else refocus the previously selected item
  This is now ready for testing!
  I'll double-check the implementation and maybe rebase if needed, so please don't spend time on the diff until I give 👍

New features:

• on right-click, sidebar items will be focused¹ (not selected) as long as the sidebar has focus -- on next focusInEvent the selection is refocused = selected item is always the starting point for navigation
  ¹_{highlight border like in tracks view, currently only styled in LateNight PaleMoon}
• in History, clicking one of the YEAR nodes now clears the tracks view
• right-click a YEAR node now opens a context menu with "Delete all child playlists", which requires to be confirmed twice

TODO

• enable item focus indicator in all skins

Test cases for selection:

Playlists / Crates / History
selection in sidebar and tracks view should not change when you

• select an item in one of the other sidebar features
• right-click a crate or playlist
  a) delete, rename, duplicate or lock it
  b) import a playlist file (= append it to the selected playlist / crate) or
  c) simply close the context menu without clicking an action

Playlists / Crates / History

• select a crate or playlist
• right-click, select Remove
  ⇒ item below or above should be selected and loaded to the tracks view

Playlists / Crates

• right-click the respective root item while any other item is selected
• choose "Create new ..."
  ⇒ sidebar should scroll to new item
  ⇒ new empty item should not be selected
  ⇒ tracks view should not change

Playlists / Crates

• right-click root, select "Import ..."
• the (last) imported playlist / crate should be selected and loaded to the tracks view

History

• select root item (loads current setlog to tracks view)
• right-click the actual 'current' playlist (must have at least one track)
• choose "Finish current and start new"
  ⇒ new setlog will be created and marked with →
  ⇒ root item remains selected
  ⇒ current tracks view should be cleared (new setlog)

• some small fixes

I hope this can also serve as base for any QML implementation of a sidebar/topbar/whatever feature view we come up with.

[daschuer]

Is this intended to go to 2.4?

[ronso0]

Yup, for 2.4

I already rebased locally and will update this soon.
I had to start over (after I considered it 80% done) because of some silent assumptions / dependencies on the current way of right-click/select and some specialialities of SetlogFeature, and that required more lke a round-house kick than the rather cosmetic fixes I pushed first.

[ronso0]

This is now ready for testing!
I updated the PR description with new features and some test cases
I'll still double-check the implementation and maybe rebase if needed, so please don't spend time on the diff until I give 👍

[daschuer]

I have just tested it and that is an improvement. However, the scrolling is now "broken" when you for instance rename a playlist.
In this case the renamed playlist is scrolled out of sight, when returning from the context menu.

[ronso0]

Everything should now work as expected.

TODO:
enable item focus indicator in all skins (currently added to LateNight only)

[ronso0]

All done!

New features in History:
right-click menu now allows to

• un/lock all childs of a YEAR folder
• delete all childs of a YEAR folder (with two confirmation dialogs!)

All skins should now mark the (unselected) right-clicked sidebar item with a border.

[ronso0]

Sorry I had to force-push. The code suggestions I commited here changed some variable names which I didn't spot in the diff (or relied on it being correct, i.e. this would built)

[ronso0]

idStringList is not const anymore so we can remove the locked playlist ids, without creating another QStringList

[daschuer]

We don't want non const references. I think in this case it would be correct to std::move the idStringList into the function, than it is clear that the caller must not use it afterwards.

Suggested change

	bool PlaylistDAO::deleteUnlockedPlaylists(QStringList& idStringList) {
	bool PlaylistDAO::deleteUnlockedPlaylists(QStringList&& idStringList) {

[daschuer]

We don't want non const references. I think in this case it would be correct to std::move the idStringList into the function, than it is clear that the caller must not use it afterwards.

Suggested change

	bool PlaylistDAO::deleteUnlockedPlaylists(QStringList& idStringList) {
	bool PlaylistDAO::deleteUnlockedPlaylists(QStringList&& idStringList) {

[ronso0]

Thanks @daschuer!
It now uses std::move operator (even though I'm not 100% sure I understood all of it) and I tweaked the year delete dialogs.

[ronso0]

I just noticed two regressions:

• in Playlists the (track count) duration is removed when selecting a playlist (same for Crates, my bad)
• after playlist rename the items are not resorted

I'll look into it.

[ronso0]

Okay, done. Fortunately that was easy (I'm not very much into the code anymore.. x) )

[daschuer]

Looks and works good. Thank you.

[ronso0]

Oh, thanks! I was just doing intense testing and fixed some minor issues.
Follow-up is here #11574