Skip to content

Commit 0c873cf

Browse files
authored
Escape should close combobox/listbox inside modal without closing modal (#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
1 parent d031c8f commit 0c873cf

File tree

3 files changed

+51
-3
lines changed

3 files changed

+51
-3
lines changed

.github/workflows/lintBuildTest.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ jobs:
2020
id: cache-node-modules
2121
with:
2222
path: node_modules
23-
key: modules-${{ hashFiles('package-lock.json') }}
23+
key: modules-${{ hashFiles('package-lock.json', 'patches/**') }}
2424
- name: npm install
2525
if: steps.cache-node-modules.outputs.cache-hit != 'true'
2626
run: npm install
@@ -39,7 +39,7 @@ jobs:
3939
id: cache-node-modules
4040
with:
4141
path: node_modules
42-
key: modules-${{ hashFiles('package-lock.json') }}
42+
key: modules-${{ hashFiles('package-lock.json', 'patches/**') }}
4343
- name: Typecheck
4444
run: npx tsc
4545
- name: Lint
@@ -67,7 +67,7 @@ jobs:
6767
uses: actions/cache@v4
6868
with:
6969
path: node_modules
70-
key: modules-${{ hashFiles('package-lock.json') }}
70+
key: modules-${{ hashFiles('package-lock.json', 'patches/**') }}
7171
- name: Set env.PLAYWRIGHT_VERSION
7272
run: |
7373
PLAYWRIGHT_VERSION=$(npm ls --json @playwright/test | jq --raw-output '.dependencies["@playwright/test"].version')
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
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
2+
index 0788d1b..3bb001c 100644
3+
--- a/node_modules/@radix-ui/react-use-escape-keydown/dist/index.mjs
4+
+++ b/node_modules/@radix-ui/react-use-escape-keydown/dist/index.mjs
5+
@@ -9,8 +9,8 @@ function useEscapeKeydown(onEscapeKeyDownProp, ownerDocument = globalThis?.docum
6+
onEscapeKeyDown(event);
7+
}
8+
};
9+
- ownerDocument.addEventListener("keydown", handleKeyDown, { capture: true });
10+
- return () => ownerDocument.removeEventListener("keydown", handleKeyDown, { capture: true });
11+
+ ownerDocument.addEventListener("keydown", handleKeyDown);
12+
+ return () => ownerDocument.removeEventListener("keydown", handleKeyDown);
13+
}, [onEscapeKeyDown, ownerDocument]);
14+
}
15+
export {

test/e2e/firewall-rules.e2e.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -625,3 +625,36 @@ test('arbitrary values combobox', async ({ page }) => {
625625
// same options show up after blur (there was a bug around this)
626626
await expectOptions(page, ['db1', 'Custom: d'])
627627
})
628+
629+
test("esc in combobox doesn't close form", async ({ page }) => {
630+
await page.goto('/projects/mock-project/vpcs/mock-vpc/firewall-rules-new')
631+
632+
// make form dirty so we can get the confirm modal on close attempts
633+
await page.getByRole('textbox', { name: 'Name' }).fill('a')
634+
635+
// make sure the confirm modal does pop up on esc when we're not in a combobox
636+
const confirmModal = page.getByRole('dialog', { name: 'Confirm navigation' })
637+
await expect(confirmModal).toBeHidden()
638+
await page.keyboard.press('Escape')
639+
await expect(confirmModal).toBeVisible()
640+
await confirmModal.getByRole('button', { name: 'Keep editing' }).click()
641+
await expect(confirmModal).toBeHidden()
642+
643+
const formModal = page.getByRole('dialog', { name: 'Add firewall rule' })
644+
await expect(formModal).toBeVisible()
645+
646+
const input = page.getByRole('combobox', { name: 'VPC name' }).first()
647+
await input.focus()
648+
649+
await expect(page.getByRole('option').first()).toBeVisible()
650+
await expectOptions(page, ['mock-vpc'])
651+
652+
await page.keyboard.press('Escape')
653+
// options are closed, but the whole form modal is not
654+
await expect(confirmModal).toBeHidden()
655+
await expect(page.getByRole('option')).toBeHidden()
656+
await expect(formModal).toBeVisible()
657+
// now press esc again to leave the form
658+
await page.keyboard.press('Escape')
659+
await expect(confirmModal).toBeVisible()
660+
})

0 commit comments

Comments
 (0)