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

Related to #1220 - keyboard bindings/mnemonics #1221

Merged
merged 8 commits into from Dec 11, 2018

Conversation

Projects
None yet
2 participants
@tostringtheory
Copy link
Contributor

tostringtheory commented Oct 17, 2018

This pull request begins to address adding mnemonics to insomnia menu items for easy accessibility on the Windows platform, as well as adding a global shortcut listener for a hotkey to access the insomnia shortcuts list. Most changes from this checkin appear to be from auto-linting.

@welcome

This comment has been minimized.

Copy link

welcome bot commented Oct 17, 2018

💖 Thanks for opening this pull request! 💖

To help make this a smooth process, please be sure you have first read the
contributing guidelines.

@gschier

This comment has been minimized.

Copy link
Collaborator

gschier commented Oct 18, 2018

I think this PR looks pretty good so far! Just a few things.

  1. What is the purpose of the &? Is this some sort of special thing that Windows/Linux understands? A link to some docs would be appreciated 😄
  2. Can you post before/after screenshots (or a recording if possible) to better showcase what this actually does?
  3. What's left to do on this?
@gschier

This comment has been minimized.

Copy link
Collaborator

gschier commented Oct 19, 2018

Was looking how to implement a custom app menu and noticed the GitHub app is using mnemonics too! https://github.com/desktop/desktop/blob/e69467d1239c1b2cb0e290b61f8f703aaa9deeea/app/src/main-process/menu/build-default-menu.ts

@tostringtheory

This comment has been minimized.

Copy link
Contributor

tostringtheory commented Dec 2, 2018

@gschier - thanks for looking over this! Sorry for the ghost - dealing with sudden death in the family. Happy to finally be getting back to normalcy though. In response to your questions:

1 - The '&' character in a menu label on Windows (and Linux I believe) denotes a 'mnemonic' for that menu label. It is used to indicate basically the ALT shortcut key for that menu item. In Windows, this is what drives the function where you see the underlines appear on a character of menu items when you press ALT, and natively makes the menu items focus when pressing ALT+{letter after mnemonic}. As for a link, you can check out this MSDN article on Mnemonics - https://docs.microsoft.com/en-us/cpp/windows/defining-mnemonics-access-keys?view=vs-2017

2 - I will definitely post a picture of this in the next revision of the PR (hopefully today)

3 - I only implemented mnemonics on the top level menu and the file menu as an example of mnemonic usage to see if you were OK with the method of implementation. I'm going to implement the rest of the menu items now and submit a PR.

@tostringtheory

This comment has been minimized.

Copy link
Contributor

tostringtheory commented Dec 2, 2018

For some example images, (not quite sure how best to visualize this without a video) - but I'll try, here's the current application. When the user presses ALT, you can see that none of the menu bar items show any difference:

image

With the updates, the menu bar displays underlines under the mnemonic keys:

image

This allows me to press ALT+A for example (since A is the mnemonic specified for the Application menu), and it will open the Application menu, showing the mnemonics for the application menu-items as well:

image

As well, I've bound a CMD/CTRL+SHIFT shortcut for the keyboard shortcuts list for usability, as well as F1 for the Help site to coincide with the standard for F1=help in an application.

Last, I applied a minor style update to the tabs styling to highlight the tab with focus, so if you're tabbing through a modal/tab, you can tell when you reach the tab list, and can navigate via left/right when it is highlighted:

image

I also looked into how to make the tab interfaces listen for CTRL+[SHIFT]+TAB navigation, but at this time am not able to see the best method to implement. I think this is good for now, unless you see any changes?

@gschier
Copy link
Collaborator

gschier left a comment

Thanks for this! Just a few minor comments.

@gschier gschier force-pushed the getinsomnia:develop branch from 9b1c261 to 6f355bc Dec 11, 2018

@gschier gschier merged commit 19bd9c1 into getinsomnia:develop Dec 11, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@welcome

This comment has been minimized.

Copy link

welcome bot commented Dec 11, 2018

Congrats on merging your first pull request! 🎉🎉🎉 You're helping make Insomnia awesome! 🙌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment