keyUpEvents/keyDownEvents check `code`, not `key`

[Saadnajmi]

Problem Description

Per the keyboard API spec, the props keyDownEvents and keyUpEvents are used to specify which events should be marked as handled natively (so that they are only handled in JS). The spec says you specify an array of IHandledKeyboardEvent objects, each of which contains a key field and booleans for the modifier keys:

  const handledNativeKeyboardEvents: IHandledKeyboardEvent[] = [
     { key: 'Esc' },
     { key: 'Enter', ctrlKey : true, eventPhase : EventPhase.Capturing }
  ];

However, if we look a the actual type and implementation , we aren't checking the key field, but instead the code field.

<Pressable
  keyDownEvents={[
    {code: 'KeyW', handledEventPhase: 3},
    {code: 'KeyE', handledEventPhase: 1},
  ]}
  ...
  >

These two differ quite heavily and make it harder to write cross platform code. Both rn-win32 and rn-macOS check the key field in their respective implementation.

This came up as an issue while I was working on FluentUI React Native, where I'm trying to write cross-platform keyboard handlers and having a very hard time :')

Steps To Reproduce

Look at PressableExample.windows.js and see that all the keyboarding examples have objects that look like [{code: 'keyW'}, {code: 'keyH'}]

Expected Results

Should be more like [{key:'W'},{key:'H'}]

CLI version

n/a

Environment

n/a

Target Platform Version

None

Target Device(s)

Desktop

Visual Studio Version

None

Build Configuration

None

Snack, code example, screenshot, or link to a repository

I guess, this is my keyboard handler. The windows version of it is wrong because it uses key :')

https://github.com/microsoft/fluentui-react-native/blob/main/packages/utils/interactive-hooks/src/useKeyProps.ts

[rozele]

Should we support both?

Code aligns to https://www.w3.org/TR/uievents-code/ Key aligns to https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/key/Key_Values

It's not clear which is a better standard, but I would think we'd want to lean more on W3 standards than MDN specs.

[rozele]

Not sure if we already have another issue for this, but I think it's perhaps even more important to reconcile whether we want all key events on focused views to be emitted (as on Windows) or only those declared (as on macOS). Seems to me that emitting all events is perhaps more aligned with some of the PointerEvents APIs (e.g., onPointerDown / onPointerUp), but there are others that emit only when subscribed (e.g., onPointerEnter / onPointerMove). Perhaps we should get a bit more "creative" on Windows, and only emit the event if there's an actual subscriber in the path (though arguably apps that have things like shortcuts configured on the root would pretty much always have onKeyUp/onKeyDown subscribed).

If we were to follow the pattern of always emitting key events (which I believe is a bit more difficult on macOS than it is on Windows), we may not even need keyDown/UpEvents, if we were to lean on synchronous events in Fabric - the idea being that if you wanted to mark an event as handled, we could subscribe onKeyDown/UpSync, set preventDefault, and have this prevent native behaviors...

[Saadnajmi]

I've been going with "long term goal is to emit all events on macOS" and have a draft to support that: https://github.com/microsoft/react-native-macos/pull/1615 . I haven't considered how synchronous events would change the API as that seemed a ways away.

I think both key and code are valid properties with semantically different meanings. We can have both and maybe even keyDownEvents can filter on either/or. The bug is simply that right now, it filters on "code" which is not obvious and disagrees with the spec I linked / makes it harder to write cross platform code.

[rozele]

Indeed - we have this pain today in Messenger, and use this ugly little trick:

export function makeWindowsKeyEventData(data: KeyEventData) {
  let code = data.key;

  // See https://www.w3.org/TR/uievents/#code-examples
  const keyToCodeMap = {
    ',': 'Comma',
    '-': 'Minus',
    '/': 'Slash',
    '=': 'Equal',
    '[': 'BracketLeft',
    ']': 'BracketRight',
    '{': 'BracketLeft',
    '}': 'BracketRight',
  };

  if (code.length === 1) {
    if (code >= '0' && code <= '9') {
      code = 'Digit' + code;
    } else if (code >= 'A' && code <= 'z') {
      code = 'Key' + code.toUpperCase();
    } else if (keyToCodeMap.hasOwnProperty(code)) {
      code = keyToCodeMap[code];
    }
  }

  // For consistency with macOS, changing the handled event phase for
  // 'keyDownEvents' and 'keyUpEvents' to 'capturing' to prevent default
  // behavior.
  const CAPTURING_EVENT_PHASE = 1;
  return {...data, code, handledEventPhase: CAPTURING_EVENT_PHASE};
}

But I'm certain it's not a complete mapping, just maps the characters we care about in Messenger thus far 😬

[Saadnajmi]

@rozele why does the event phase go to capturing for macOS consistency? I thought you would prevent defaults by virtue of having the event in your keyUp/DownEvents array?

[rozele]

Not sure, I assumed Capturing event phase was default as well. I think there was a time where setting the Bubbling phase did not prevent native behaviors in XAML Islands, but tried to repro recently and it was no longer the case.

This is still open though: https://github.com/microsoft/microsoft-ui-xaml/issues/4763

🤷‍♂️

[Saadnajmi]

@rozele In macOS natively, there is no capturing phase, only a bubbling phase. That's why I was confused.

There's a way to capture events at the application level (https://developer.apple.com/documentation/appkit/nsevent/1534971-addlocalmonitorforeventsmatching?language=objc) so we could theoretically implement something close to a capturing phase.

[TatianaKapos]

Marking needs attention, this should either be assigned a dev for v74 or be put on the backlog!