dojo / widgets

:rocket: Dojo - UI widgets.
https://widgets.dojo.io
Other
89 stars 66 forks source link

Usage of event.which for key constants (deprecated) #566

Open sebilasse opened 6 years ago

sebilasse commented 6 years ago

Please see https://github.com/dojo/widgets/search?q=which&unscoped_q=which There are multiple references to event.which which has been removed from the Web standards and for which Mozilla says "Be aware that this feature may cease to work at any time."

It will be replaced by event.key https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/key so it might be better to use (event.key || event.which) now. Maybe it could be nice to have a meta for Keys (?)

There are also several "mimics" for .which which could be replaced by .key and an Enum in https://github.com/dojo/widgets/blob/master/src/common/util.ts where the strings could equal .key. E.g. "Left", "Up", "Right", "Down" becomes "ArrowLeft" etc. and Space becomes ' ' Personally I am using

enum _Keys {
  Unidentified = 0,
  Cancel = 3,
  Help = 6,
  Backspace = 8,
  Tab = 9,
  Clear = 12,
  Enter = 13,
  Shift = 16,
  Control = 17,
  Alt = 18,
  Pause = 19,
  CapsLock = 20,
  Escape = 27,
  Convert = 28,
  NonConvert = 29,
  Accept = 30,
  ModeChange = 31,
  ' '  = 32,
  PageUp = 33,
  PageDown = 34,
  End = 35,
  Home = 36,
  ArrowLeft = 37,
  ArrowUp = 38,
  ArrowRight = 39,
  ArrowDown = 40,
  Select = 41,
  Print = 42,
  Execute = 43,
  PrintScreen = 44,
  Insert = 45,
  Delete = 46,
  /* Printable ASCII */
  OS = 91,
  OSRight = 92,
  ContextMenu = 93,
  /* ... NumPad */
  '*' = 106,
  '+' = 107,
  '-' = 109,
  '.' = 110,
  '/' = 111,
  /* ... F Keys */
  NumLock = 144,
  ScrollLock = 145,
  VolumeMute = 181,
  VolumeDown = 182,
  VolumeUp = 183,
  Meta = 224,
  AltGraph = 225,
  Attn = 246,
  CrSel = 247,
  ExSel = 248,
  EraseEof = 249,
  Play = 250,
  ZoomOut = 251
}

export type Keys = keyof typeof _Keys;
export function keyConstant(event: KeyboardEvent): string {
  if (typeof event.key === 'string') { return event.key }
  // event has no .key
  const i: number = event.which || event.keyCode || event.charCode;
  if (!i || typeof i !== 'number') { return 'Unidentified' }
  if (i === 17 || i === 162 || i === 163) { return 'Control' }
  if (i === 91 || i === 92 || i === 93 || i === 224) { return 'Meta' }
  if (i > 64 && i < 91) {
    const key = String.fromCharCode(i);
    return !event.shiftKey ? key.toLowerCase() : key
  } else if (i > 111 && i < 136) {
    return `F${i - 111}`
  } else if (i > 95 && i < 106) {
    return String.fromCharCode(i - 48)
  }
  return _Keys[i] || 'Unidentified'
}

usage

if (keyConstant(event) === Keys.Enter) { dragon.fire() }
if (keyConstant(event) === 'Enter') { dragon.fire() }

my 2 cents: key is also a much better naming than .which which is confusing because a KeyboardEvent also has "generic" properties shared with other Events …

tomdye commented 5 years ago

@sebilasse thanks for bringing this to our attention, it appears that IE and Edge only partially support this right now. We will look into switching over our usage as we make changes to improve our widgets and support a material theme.