Skip to content

Commit

Permalink
Escape should close combobox/listbox inside modal without closing mod…
Browse files Browse the repository at this point in the history
…al (#2610)

* fix escape in combobox in dialog: no capture:true in radix esc handler

* try making @radix-ui/react-use-escape-keydown an explicit dep

* actually the problem was the node cache
  • Loading branch information
david-crespo authored Dec 10, 2024
1 parent d031c8f commit 0c873cf
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 3 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/lintBuildTest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ jobs:
id: cache-node-modules
with:
path: node_modules
key: modules-${{ hashFiles('package-lock.json') }}
key: modules-${{ hashFiles('package-lock.json', 'patches/**') }}
- name: npm install
if: steps.cache-node-modules.outputs.cache-hit != 'true'
run: npm install
Expand All @@ -39,7 +39,7 @@ jobs:
id: cache-node-modules
with:
path: node_modules
key: modules-${{ hashFiles('package-lock.json') }}
key: modules-${{ hashFiles('package-lock.json', 'patches/**') }}
- name: Typecheck
run: npx tsc
- name: Lint
Expand Down Expand Up @@ -67,7 +67,7 @@ jobs:
uses: actions/cache@v4
with:
path: node_modules
key: modules-${{ hashFiles('package-lock.json') }}
key: modules-${{ hashFiles('package-lock.json', 'patches/**') }}
- name: Set env.PLAYWRIGHT_VERSION
run: |
PLAYWRIGHT_VERSION=$(npm ls --json @playwright/test | jq --raw-output '.dependencies["@playwright/test"].version')
Expand Down
15 changes: 15 additions & 0 deletions patches/@radix-ui+react-use-escape-keydown+1.1.0.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
diff --git a/node_modules/@radix-ui/react-use-escape-keydown/dist/index.mjs b/node_modules/@radix-ui/react-use-escape-keydown/dist/index.mjs
index 0788d1b..3bb001c 100644
--- a/node_modules/@radix-ui/react-use-escape-keydown/dist/index.mjs
+++ b/node_modules/@radix-ui/react-use-escape-keydown/dist/index.mjs
@@ -9,8 +9,8 @@ function useEscapeKeydown(onEscapeKeyDownProp, ownerDocument = globalThis?.docum
onEscapeKeyDown(event);
}
};
- ownerDocument.addEventListener("keydown", handleKeyDown, { capture: true });
- return () => ownerDocument.removeEventListener("keydown", handleKeyDown, { capture: true });
+ ownerDocument.addEventListener("keydown", handleKeyDown);
+ return () => ownerDocument.removeEventListener("keydown", handleKeyDown);
}, [onEscapeKeyDown, ownerDocument]);
}
export {
33 changes: 33 additions & 0 deletions test/e2e/firewall-rules.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -625,3 +625,36 @@ test('arbitrary values combobox', async ({ page }) => {
// same options show up after blur (there was a bug around this)
await expectOptions(page, ['db1', 'Custom: d'])
})

test("esc in combobox doesn't close form", async ({ page }) => {
await page.goto('/projects/mock-project/vpcs/mock-vpc/firewall-rules-new')

// make form dirty so we can get the confirm modal on close attempts
await page.getByRole('textbox', { name: 'Name' }).fill('a')

// make sure the confirm modal does pop up on esc when we're not in a combobox
const confirmModal = page.getByRole('dialog', { name: 'Confirm navigation' })
await expect(confirmModal).toBeHidden()
await page.keyboard.press('Escape')
await expect(confirmModal).toBeVisible()
await confirmModal.getByRole('button', { name: 'Keep editing' }).click()
await expect(confirmModal).toBeHidden()

const formModal = page.getByRole('dialog', { name: 'Add firewall rule' })
await expect(formModal).toBeVisible()

const input = page.getByRole('combobox', { name: 'VPC name' }).first()
await input.focus()

await expect(page.getByRole('option').first()).toBeVisible()
await expectOptions(page, ['mock-vpc'])

await page.keyboard.press('Escape')
// options are closed, but the whole form modal is not
await expect(confirmModal).toBeHidden()
await expect(page.getByRole('option')).toBeHidden()
await expect(formModal).toBeVisible()
// now press esc again to leave the form
await page.keyboard.press('Escape')
await expect(confirmModal).toBeVisible()
})

0 comments on commit 0c873cf

Please sign in to comment.