Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Escape should close combobox/listbox inside modal without closing modal #2610

Merged
merged 3 commits into from
Dec 10, 2024
Merged

Escape should close combobox/listbox inside modal without closing modal #2610

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e75cb1d
fix escape in combobox in dialog: no capture:true in radix esc handler
david-crespo Dec 10, 2024
397fd6d
try making @radix-ui/react-use-escape-keydown an explicit dep
david-crespo Dec 10, 2024
678dfb5
actually the problem was the node cache
david-crespo Dec 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/workflows/lintBuildTest.yml
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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()
})
Loading