Shortcut Categories

[VanSHOE]

This adds shortcut categories as defined here

The shortcut categories are currently draft categories and may change later on.
A known issue is that the opening and closing of categories can be a bit stuttery when opening/ closing categories but it mostly happens when they are just about to be opened/ closed

• I signed CLA
• I made sure the code in the PR follows the coding rules
• I made sure the code compiles on my machine
• I made sure there are no unnecessary changes in the code
• I made sure the title of the PR reflects the core meaning of the issue you are solving
• I made sure the commit message(s) contain a description and answer the question "Why do those changes fix that particular issue?" or "Why are those changes really necessary as improvements?"
• I made sure the commit message title starts with "fix #424242:" if there is a related issue
• I created the test (mtest, vtest, script test) to verify the changes I made

[shoogle]

Works pretty well with the mouse, although it breaks accessibility with the keyboard. Try navigating the Preferences > Shortcuts page with the Tab key and arrow keys on your keyboard and you'll see what I mean (compare the behaviour of your PR to the latest nightly builds on master). Ideally the Spacebar and Return keys would be used to expand categories like in the Palettes.

There's no stuttering for me when expanding/collapsing the categories, though the text seems to remain visible for a few milliseconds longer than I would expect. The text should needs to disappear before the rectangles are fully collapsed. Perhaps the should lose opacity gradually rather than instantly disappearing? Or maybe the text could be gradually covered by the rectangle below?

Category names should be in bold according to the design. I would also suggest using separate columns for icons and action names to avoid the left edge of action names being staggered like this...

Icons should be centred in the first column and action names left-aligned in the next column.

[bkunda]

This is coming along really nicely @VanSHOE!
I would suggest removing the accordion animation altogether – then there should be no stuttering/lag at all. We don't have animations like this elsewhere in the app, so it would work just fine I think to just have the category contents appear/disappear when their menu header is opened/closed.
I do need to finesse and proof some of these category definitions. I already spotted one error (certainly my fault), and I expect there will be others. But actually having them "working" now and appearing as they should in the UI is going to help tremendously with this. Thank you so much!

[VanSHOE]

@bkunda I see, I will remove the animation
However, the opening/ closing still will not be instant though (will happen after a delay) it is better than an animation that stutters

[cbjeukendrup]

It might be better to get rid of the vector and do this method as a switch statement:

switch (category) {
case ...: return ...;
...
}

// Unreachable
return {};

This way, you don't need to worry about syncing the order in the enum and in this method, and you'll get a compiler warning here if you forget to handle new cases here.

Also, maybe better rename the method to categoryName(...) (we tend not to use get prefixes).

[VanSHOE]

@cbjeukendrup I can do this however, the categories vector is required to send all sections to QML as well. It is in ShortcutsPage.qml

  Component.onCompleted: {
      shortcutsModel.load()

      shortcutsView.allSections = shortcutsModel.sections() // <-- Here
      topPanel.setSearchText(root.shortcutCodeKey)
  }

So I thought maybe the get category could just use this list itself to preserve Lines of Code. Please let me know if I should still go with the switch case approach in section name

Edit: Also in QML, these names are directly used to check whether they are collapsed or not, so using the list itself would ensure that there is no mis-match among the names (because same source)
But if I use switch, then if any typo happens in naming and it does not match what it says in the list, it would cause bugs

[cbjeukendrup]

I think you can actually get rid of getCategory and just do the stuff in ShortcutsModel::sectionName(...).

[cbjeukendrup]

Not sure if this is the best translation context. @shoogle What do you think? Maybe "action/category", to keep it consistent with "action" which is the context of UIActions?
(probably "framework/" is not right anyway because it's not part of the framework module anymore)

[VanSHOE]

Please let me know if this is supposed to be done. Till then I'll push some of the other changes

[cbjeukendrup]

I have a feeling that we should think a little bit about how the tree view is implemented in QML. Currently, we're basically abusing a ListView and setting the height of invisible items to zero. This has bad implications for keyboard navigation (it will treat the "hidden" rows as visible, because technically they are).

You could try to use QML's TreeView component (rather than ListView). TreeView is from QtQuick.Controls 1, and thus has a totally different idiom than ListView (it's a bit more complicated).

It would be quite a big operation, because it would mean that the ShortcutsModel will also need to become a QAbstractItemModel rather than a QAbstractListModel.

I'm not sure whether this is worthwhile. I can't predict whether this will give better performance or not at all. On the other hand, one could argue that it is a lot cleaner. Maybe @shoogle and other developers could advise about this?

(One other note about this: Qt 6.0-6.2 did not contain a TreeView component at all, but since Qt 6.3 there's a new one, which is more in ListView style, and works completely different from the QtQuick.Controls 1 one at the QML side. So, using TreeView now will mean having to implement it when we switch to Qt 6.)

[VanSHOE]

@cbjeukendrup Keyboard navigation with respect to the list items was fixed, i.e. if you collapse them they wont be selectable with keyboard anymore. Please let me know in case its not working.

The issue however is that sections themselves can't be selected with a keyboard

[cbjeukendrup]

Ah, missed that, my fault. So that's one less problem to worry about...

[shoogle]

using TreeView now will mean having to implement it when we switch to Qt 6

If the Qt 5 TreeView is implemented very differently to the Qt 6 TreeView then I think it would be better to stick with the current approach (i.e. the hacked ListView) for as long as we use Qt 5, and only think about using a proper TreeView after we have switched to Qt 6.

sections themselves can't be selected with a keyboard

Of course if this is not solvable then that might be an argument for using a proper TreeView now, but hopefully it will be solvable one way or another.

[cbjeukendrup]

Hi @VanSHOE! We would like to revisit this PR, and we are planning to include it in MuseScore 4.1. The first thing that's necessary for that is a rebase, both to fix the conflicts and to get new artifacts so that we can test it again (the old builds have expired). It would be great if you could do that!

[cbjeukendrup]

It looks like the Internal category is not really used; its role already seems to be fulfilled by Undefined. Do we need it?

[VanSHOE]

@cbjeukendrup
Back then, I believe I set Undefined for those shortcuts that I did not really know the category of while Internal meant confirmed private actions. We can remove them but I thought it might be nice to keep them separate in case the Undefined ones do end up getting a category at some point

[VanSHOE]

@cbjeukendrup

Sure Im on it!

[bkunda]

Unfortunately we're going to have to bump this to 4.2 for now because we are out of time for introducing new features in 4.1, and this one still needs some consideration by @RomanPudashkin and @cbjeukendrup.