Stop Combobox Esc propagation

[frontsideair]

Thank you for contributing to Reach UI! Please fill in this template before submitting your PR to help us process your request more quickly.

• Use a meaningful title for the pull request. Include the name of the package modified.
• Test the change in your own code (Compile and run).
• Add or edit tests to reflect the change (Run with yarn test).
• Add or edit Storybook examples to reflect the change (Run with yarn start).
• Ensure formatting is consistent with the project's Prettier configuration.
• Add documentation to support any new features.

This pull request:

• Creates a new package
• Fixes a bug in an existing package
• Adds additional features/functionality to an existing package
• Updates documentation or example code
• Other

If creating a new package:

• Make sure the new package directory contains each of the following, and that their structure/formatting mirrors other related examples in the project:
  • examples directory
  • src directory with an index.tsx entry file
  • At least one example file per feature introduced by the new package
  • Base styles in a style.css file (if needed by the new package)

---

This PR is to fix #395 but the suggested fix doesn't seem like it works. At least the test would be useful.

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit b9a83df:

Sandbox	Source
reach/reach-ui: reach-ui	Configuration
Reach UI escape issue	Issue #395

[indiesquidge]

@frontsideair thanks for your PR contribution!

The issue with your test seems to be the use of queryByTestId. Using ByTestId is not recommended if there are other queries more inline with how the software is used by people or screen readers. We can use ByRole for most things associated with Combobox, and indeed your tests pass with these query selectors.

Given #395 also cares about the event propagating, we should also assert that our dialog remains open when escape is pressed. Perhaps something like this:

it("should not close the dialog when Esc key is pressed", async () => {
  let { getByRole, queryByRole } = render(<BasicDialog />);
  let input = getByRole("combobox");

  expect(getByRole("dialog")).toBeInTheDocument();

  await userEvent.type(input, "e");

  expect(getByRole("listbox")).toBeInTheDocument();

  await userEvent.type(input, "{esc}");

  // escape should close our listbox
  expect(queryByRole("listbox")).not.toBeInTheDocument();

  // but our dialog should still be open
  expect(queryByRole("dialog")).toBeInTheDocument();
});

[indiesquidge]

@frontsideair it's worth pointing out that your goal of preventing the escape button from propagating up to the dialog your combobox is rendered in and closing the dialog can be prevented in your product code, without having to change Reach UI's Combobox.

All Reach UI event handlers are composed in such a way that allows the consumer of the component to still access the event in their own handler.

<ComboboxInput
+ onKeyDown={(event) => {
+   if (event.key === "Escape") {
+     event.stopPropagation();
+   }
+ }}
/>