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

API to get "keyboard layout aware key label" (same as menus do today) #3631

Closed
alexdima opened this issue Nov 30, 2015 · 3 comments
Closed

Comments

@alexdima
Copy link

From microsoft/vscode#713

Today, when we make a menu item, we give an accelerator that looks like "CtrlOrCmd+\" for Split Editor. When the keyboard layout is set to German (Switzerland), the menu correctly renders this as Ctrl+ä:

image

When we render the same keybinding straight in JavaScript to the DOM, we lack this keyboard layout information and we render incorrectly Ctrl+\:

image

This leads to a lot of confusion for users using non standard US keyboard layouts.

I want to ask for an api exposed to JavaScript in the renderer that would do the following:

  • acceleratorToLabel("Ctrl+\") ==> "Ctrl+ä" when the keyboard layout is German (Switzerland)
  • acceleratorToLabel("Ctrl+\") ==> "Ctrl+\" when the keyboard layout is US standard.
  • i.e. expose the same thing the menu already does

I am also willing to work on a PR for this, but I am not sure where this conversion happens. I think Electron gives Chromium a ui::Accelerator for each accelerator, but I could not figure out where or how Chromium converts the ui::Accelerator to a label for the menu item.

@zcbenz Can you please give me some guidance / starting points on how I could implement this or if you think it is a good idea or not or if you'd want to do it.

Thank you!

@alexdima
Copy link
Author

@zcbenz I think the method I'm looking to expose would be ui::Accelerator::GetShortcutText. What do you think?

@zcbenz
Copy link
Member

zcbenz commented Dec 1, 2015

Yeah ui::Accelerator::GetShortcutText should be the right way to go. One problem is ui::Accelerator is designed to be used only in the main process, using it in renderer process it very likely to cause crashes (I didn't try though).

On the API itself, I think we can add an i18n module, with a simple method doing what you want:

require('electron').i18n.USShortcutToLocalizedShortcut(shortcut);

And in future we can fill it with more i18n/l10n methods.

@alexdima
Copy link
Author

alexdima commented Dec 4, 2015

@zcbenz Sorry for the noise, I have almost completed a PR for this (alexandrudima/electron@fa26312153761f98311582468310327505389987) but then I noticed that ui::Accelerator::GetShortcutText does what I hoped it does only on windows. Under linux and mac it just falls back to the US standard layout.

I found a good method that would do what I wanted, but it sits inside chromedriver and is not available when chromium is built.

I extracted and improved their code and published a new npm module native-keymap and adopted it in VSCode. It doesn't do anything yet for mac, I will tackle that when I get my hands on a mac, but it already works fine for windows and linux allowing me to render the keybindings better:

image

@alexdima alexdima closed this as completed Dec 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants