Format arrow shortcuts as arrow glyphs

[jasongrout]

If a shortcut has an arrow key, right now the text (e.g., "ArrowRight") is printed when the shortcut is formatted for display in

lumino/packages/commands/src/index.ts

Lines 1133 to 1165 in 4f24322

	/**
	* Format a keystroke for display on the local system.
	*/
	export
	function formatKeystroke(keystroke: string): string {
	let mods = '';
	let parts = parseKeystroke(keystroke);
	if (Platform.IS_MAC) {
	if (parts.ctrl) {
	mods += '\u2303 ';
	}
	if (parts.alt) {
	mods += '\u2325 ';
	}
	if (parts.shift) {
	mods += '\u21E7 ';
	}
	if (parts.cmd) {
	mods += '\u2318 ';
	}
	} else {
	if (parts.ctrl) {
	mods += 'Ctrl+';
	}
	if (parts.alt) {
	mods += 'Alt+';
	}
	if (parts.shift) {
	mods += 'Shift+';
	}
	}
	return mods + parts.key;
	}

A better convention, I think, is to replace arrows in the shortcut key with actual unicode arrow glyphs, i.e., "ArrowRight" with unicode 2192: →

[jasongrout]

Here are some lists of more glyphs for specific keys on a mac: https://apple.stackexchange.com/questions/55727/where-can-i-find-the-unicode-symbols-for-mac-functional-keys-command-shift-e

[vidartf]

Someone should check that the glyphs are accessible too :)

[jasongrout]

It looks like there has been some work on this (thanks!). It looks like conversation around this is now spread across #252, #255, and #256. To have a consolidated place for the conversation, I'm writing my thoughts here.

I wasn't clear above that I meant to suggest that we just change the line at

lumino/packages/commands/src/index.ts

Line 1164 in 4f24322

return mods + parts.key;

Instead of always returning mods + parts.key, we should instead probably format parts.key if parts.key is something like ArrowUp, ArrowDown, etc. In other words, that line would be return mods + formattedKey where formattedKey would default to parts.key, but would be the unicode arrow right symbol if parts.key was ArrowUp, etc.

It looks like #252 instead pursued the approach of treating arrow keys as essentially modifiers, which caused some issues noted in #255, and further work in this direction was undertaken in #256.

I propose we revert #252 and close #256 and instead just format parts.key in the line noted above if parts.key is ArrowUp, etc. I'm sorry my suggestion wasn't clearer on this in the original description.

Actually, if we wanted to make a more fundamental change, I think the mapping of a key name to a unicode string for display should happen in the Keyboard Layout code in the keyboards package. Only the keyboard layout really knows what keys are in the layout, and it should be the thing to tell how to display those keys in the interface. For example, what if a keyboard layout has an AltGr modifier, or doesn't have the Control modifier? Our hardcoded assumptions in formatKeystroke above make some assumptions about what modifiers are in the keyboard layout in order to display them. Ideally, I think the keyboard layout would have a separate formatKey function that would take a key name and return an OS-specific unicode string representing that key. Then we would just call this formatKey function on each of the key names in the shortcut (modifier or not) to obtain a display string. I've done some exploratory work in this direction in jasongrout@3c6b837. However, this approach is much more disruptive (though still backwards compatible) than what I'm suggesting above, so perhaps we should just go with the above suggestion unless someone has the time to dive into keyboard layouts, which is indeed a tricky and inconsistent subject.

[jasongrout]

I propose we revert #255 and close #256 and instead just format parts.key in the line noted above if parts.key is ArrowUp, etc. I'm sorry my suggestion wasn't clearer on this in the original description.

We should also check with accessibility experts Vidar's good point above, that converting "ArrowUp" to "↑" may be a problem for screen readers. If displaying "↑" instead of "ArrowUp" is a problem for screen readers that we cannot fix, it may be better to just revert #252 and close this issue.

[krassowski]

I like the exploration in jasongrout@3c6b837, especially formatting space as a visible character, though I'm afraid that the current implementation of commands will ignore shortcuts with spaces anyways (wrong, Space works well). I believe Unicode glyphs are good for accessibility, but indeed it would be good to confirm.

Long-term, another thing I would like for shortcuts is to have a utility to wrap the keys in kbd (<kbd>Crtl</kbd>), which will improve accessibility too; it can be styled to keep a very simple look but it also allows displaying the keys nicely as overlays with different styling. It could also allow to unambiguously say Right Alt + X. The formatKey that you propose looks like a good step in this direction.

Reverting works for me as well - no strong preference. If we choose to close my PR, let's remember to recycle the added unit test.

[krassowski]

Here is an old accessibility evaluation: https://www.deque.com/blog/dont-screen-readers-read-whats-screen-part-1-punctuation-typographic-symbols/ (2014).

Some things are surprising (see "JAWS Bugs" section) which shows that we need to check on a character-by-character basis. The context matters too and some characters will be skipped in a sentence (say comma), but still, correctly read out loud in the context of a keyboard shortcut.

At the time when the article was written in 2014 right/left arrows were read fine when standalone in two of the evaluated readers (NVDA, VoiceOver) but JAWS 15 was not up to the task. Up/down arrows only worked in VoiceOver. There is a subtle difference: they evaluated characters encoded as HTML entities, not Unicode.

I would expect that today in 7 years from that the situation improved.

[jasongrout]

NVDA and JAWS are windows-only, while this change I think is mac-specific (i.e., I think only MacOS actually shows the arrow glyph instead of printing the name, so we should probably be consistent), so hopefully we only have to worry about VoiceOver.

[jasongrout]

I've opened #257 to explore moving formatting and modifier keys logic over to keyboard layouts. It is a draft PR that I don't know if it is worth pursuing until we have (a) a usecase for other keyboard layouts with other modifiers or (b) can make breaking changes.

I also opened #258 with the simpler approach mentioned above, and pulled in @krassowski's unit tests as well. I think we should review and merge #258. I think we should possibly close #257 as an experiment, and open it again later if someone wants to tackle the few other tricky issues with completing it.