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

Surface keyboard shortcuts in Menu when shown #791

Closed
timheuer opened this issue May 14, 2019 · 6 comments · Fixed by #924
Closed

Surface keyboard shortcuts in Menu when shown #791

timheuer opened this issue May 14, 2019 · 6 comments · Fixed by #924
Assignees
Labels
Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal.

Comments

@timheuer
Copy link
Member

timheuer commented May 14, 2019

Proposal: Ensure the Menu items show the keyboard shortcuts
Guidance: Keyboard accelerators

image
(note: yes the CTRL+Down is incorrect for settings, working on fixing that)

When enabling keyboard users, helping them not have to hunt for productive shortcuts is a good user experience. When using menus, it is best practice if a shortcut exists, to surface that to the user like above.

Changes are simple since the keyboard shortcuts are already there, changes to TerminalApp\App.cpp are all that are needed.

Questions:
[] How would this affect custom keybindings in #537 ?

@miniksa
Copy link
Member

miniksa commented May 14, 2019

@timheuer are you working on this? Should we assign you?

@miniksa miniksa added the Area-UserInterface Issues pertaining to the user interface of the Console or Terminal label May 14, 2019
@timheuer
Copy link
Member Author

Yes. There is a potential blocker in UWP APIs that I'm investigating, but I can take this.

@mdtauk
Copy link

mdtauk commented May 15, 2019

The JumpList items should match the order, and the profile order needs to persist so those keyboard accelerators can become almost muscle memory

@timheuer
Copy link
Member Author

@mdtauk this is not the JumpList item. But the menu already matches the order in profile.json file.

@mdtauk
Copy link

mdtauk commented May 15, 2019

@timheuer noted :) - just wanted to emphasise the idea of keeping these consistent if the Shortcut Keys are numerical. Ctrl+Shift+1, 2, 3 etc

@timheuer
Copy link
Member Author

The existing keybindings here are activated within the app. I can’t recall if you can trigger a jump list via a shortcut but good to add this feedback on that work item.

@DHowett-MSFT DHowett-MSFT added Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Terminal The new Windows Terminal. labels May 16, 2019
zadjii-msft pushed a commit that referenced this issue May 22, 2019
* Adding vsconfig file for VS2019 help to prompt for missing components requried.

* Adding a keybinding for launching the settings.  Suggested fix for #683

* Modified to comma per PR feedback

* Implements 791 for profile and settings shortcuts (most frequent and have shortcuts)

* Quick change for consistency (missed in first checkin due to using ENUM) on using 'Ctrl' instead of 'Control'

* Adding UI shortcut generation to new keybinding mappings.  Resolving #791

* Making a few changes on reviewer feedback for shortcut UI.

* Additional reviewer feedback on variable name change (not a member var)
@ghost ghost added the Needs-Tag-Fix Doesn't match tag requirements label May 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants