Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implemented Shortcut to cycle open tabs #24184

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

shubham-shinde-442
Copy link
Contributor

Resolves: #23702

Shortcut to cycle open tabs.

  • I signed the CLA
  • The title of the PR describes the problem it addresses
  • Each commit's message describes its purpose and effects, and references the issue it resolves
  • If changes are extensive, there is a sequence of easily reviewable commits
  • The code in the PR follows the coding rules
  • There are no unnecessary changes
  • The code compiles and runs on my machine, preferably after each commit individually
  • I created a unit test or vtest to verify the changes I made (if applicable)

@@ -92,6 +92,51 @@
<key>nav-escape</key>
<seq>Esc</seq>
</SC>
<SC>
<key>next-tab</key>
<seq>Ctrl+Tab</seq>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On macOS, it needs to be Meta instead of Ctrl.
For macOS, Qt namely automatically translates Ctrl to Cmd, and Cmd+Tab and Cmd+Shift+Tab are reserved system shortcuts.
For in-app tab switching, most macOS apps use the actual Ctrl key, and to get this with Qt, you need to write Meta.

Comment on lines 49 to 50
dispatcher()->reg(this, "eighth-tab", [this]() { navigateToSpecificTab("eighth-tab"); });
dispatcher()->reg(this, "last-tab", [this]() { navigateToSpecificTab("last-tab"); });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would alter the navigateToSpecificTab method to take an int index rather than an action code.
Inside that method, you would need to do some sanity checks. (if (index < 0 || index >= m_notations.size()) { return; }
And here, you would need to write the index instead of the action code. Shouldn't be complicated though, for example these last two lines might become

Suggested change
dispatcher()->reg(this, "eighth-tab", [this]() { navigateToSpecificTab("eighth-tab"); });
dispatcher()->reg(this, "last-tab", [this]() { navigateToSpecificTab("last-tab"); });
dispatcher()->reg(this, "eighth-tab", [this]() { navigateToSpecificTab(7); });
dispatcher()->reg(this, "last-tab", [this]() { navigateToSpecificTab(m_notations.size() - 1); });

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

@shoogle
Copy link
Contributor

shoogle commented Aug 24, 2024

See #23702 (comment):

  • Ctrl+Tab can cycle tabs but only while the score has focus.
  • Ctrl+number can't be used for tabs, but Shift+Tab followed by a number or letter can be used.
    • Feel free to implement that in this PR or leave it out if you prefer.
    • If implemented, ideally it should be done in a general way that works for all UI lists, not just the tab bar.
  • Tab key should visit every control.
    • That's a separate issue for a separate PR.
    • Please don't try to implement it until I've written a proper specification for it

@cbjeukendrup
Copy link
Contributor

  • Shift+Tab followed by a number or letter
  • This would be a "key sequence", and weren't those banned in MS4 because they "were too much of a complication"?
  • Shift+Tab is already in use as the 'opposite' of tab, i.e. move to previous control, isn't it?

@shoogle
Copy link
Contributor

shoogle commented Aug 24, 2024

It's not a key sequence.

When you press Shift+Tab from the score it puts focus on the tab bar:

image

Now the program is in a different "context" and we can listen for a different set of shortcuts.

For example, press 'B' and it should focus the Bb Clarinet tab.

image

Quickly press 'A' (or just press 'B' again) and it should put focus on the Bassoon tab.

image

Press Space or Enter to load this tab (possibly press Tab) and then we're back in the Score context again.

(This score is the Wind Quartet template from the New Score dialog.)

@cbjeukendrup
Copy link
Contributor

Ah, I see, so you mean like #16508. That should probably be implemented just in QML, bypassing the keyboard shortcuts system, so this shouldn't be affected by contexts. But it might be better to save that for a different PR.

@shubham-shinde-442
Copy link
Contributor Author

Should this be implemented for all QML dropdowns, menus, lists, and treeviews, or just for tab bar? I'm happy to work on it but would appreciate some guidance on implementation.

@shoogle
Copy link
Contributor

shoogle commented Aug 25, 2024

@shubham-shinde-442, it should be implemented for all of them except menus for the time being because it will interfere with Alt+letter mnemonics, so we need to decide what to do about that. (One option is to force people to keep holding the Alt key if they want to use mnemonics, but that might be unpopular. In Microsoft apps you can release Alt before you use a sequence of letters to navigate through a menu: Alt, F (File), S to save).

It should also be implemented for controls within a navigation panel, such as button in a toolbar. If you're in the note input toolbar and you press 'Q' it should focus the "Quarter note" button.

If there's an opportunity to share code rather than copy-pasting then it should be taken. If not then it can be split across several PRs, one for tab bars, one for dropdowns, one for lists, etc.

I've updated the description in #16508 with more details. See PR #5829 for a previous implementation in MuseScore 3.

I'm on vacation for the next 3 weeks so my capacity to help is limited, but I'm happy to test PR builds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Shortcut to cycle through open tabs
3 participants