Closed diegonvs closed 4 years ago
I'm not sure what keys we need to support, but the list of non-standard event names that we'd need to cover might be pretty short; ie. this list from MDN might get us pretty far:
switch (event.key) {
case "Down": // IE/Edge specific value
case "ArrowDown":
// Do something for "down arrow" key press.
break;
case "Up": // IE/Edge specific value
case "ArrowUp":
// Do something for "up arrow" key press.
break;
case "Left": // IE/Edge specific value
case "ArrowLeft":
// Do something for "left arrow" key press.
break;
case "Right": // IE/Edge specific value
case "ArrowRight":
// Do something for "right arrow" key press.
break;
case "Enter":
// Do something for "enter" or "return" key press.
break;
case "Esc": // IE/Edge specific value
case "Escape":
// Do something for "esc" key press.
break;
For the react based instances where we use keyCode
, do you think that will be an issue since it is coming through react's synthetic events?
Ah good point. I can think of two ways to find out: test it in the browser or look in the React source.
I am too lazy to test but not lazy enough not to look at the source:
/**
* Normalization of deprecated HTML5 `key` values
* @see https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent#Key_names
*/
const normalizeKey = {
Esc: 'Escape',
Spacebar: ' ',
Left: 'ArrowLeft',
Up: 'ArrowUp',
Right: 'ArrowRight',
Down: 'ArrowDown',
Del: 'Delete',
Win: 'OS',
Menu: 'ContextMenu',
Apps: 'ContextMenu',
Scroll: 'ScrollLock',
MozPrintableKey: 'Unidentified',
};
That's the list of keys that they normalize, so coverage seems pretty good.
I'm planning to replace the usages on Clay, Should I add this map on @clayui/shared
and reuse it or creating a map with the current key usages on each component that will need these values? 🤔
How Do you guys measure if something could be added to @clayui/shared
or not?
How Do you guys measure if something could be added to @clayui/shared or not?
We don't have anything clear cut for this. For adding to each each of the use cases, I would say lets just start with copy/paste for each map and then we can evaluate down the road if we want to abstract to @clayui/shared
What is your proposal?
While reviewing a PR, @pat270 brings an important point: .keyCode for identifying a pressed key is no longer recommended.
See: https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/keyCode
The idea is replace these usages, @pat270 recommended using
event.key
due to compatibility with IE11, but according https://caniuse.com/#feat=keyboardevent-key:Has non-standard key identifiers and incorrect behaviour with AltGraph.
.When grepping our Clay codebase, I noticed We're using on the Modal, MultiSelect and on FocusScope component.
Why would adopting this proposal be beneficial?
Avoid future problems with Keyboard Events.
What are the alternatives to this proposal?
Use
.code
(but no support for IE11)