From e75cb1db333f64ec01412f19d4fb8e1093bb33a1 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Tue, 10 Dec 2024 12:56:15 -0600 Subject: [PATCH 1/3] fix escape in combobox in dialog: no capture:true in radix esc handler --- ...ix-ui+react-use-escape-keydown+1.1.0.patch | 15 +++++++++ test/e2e/firewall-rules.e2e.ts | 33 +++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 patches/@radix-ui+react-use-escape-keydown+1.1.0.patch diff --git a/patches/@radix-ui+react-use-escape-keydown+1.1.0.patch b/patches/@radix-ui+react-use-escape-keydown+1.1.0.patch new file mode 100644 index 0000000000..601c9d66a4 --- /dev/null +++ b/patches/@radix-ui+react-use-escape-keydown+1.1.0.patch @@ -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 { diff --git a/test/e2e/firewall-rules.e2e.ts b/test/e2e/firewall-rules.e2e.ts index 38c9346f4d..71e33a7bb9 100644 --- a/test/e2e/firewall-rules.e2e.ts +++ b/test/e2e/firewall-rules.e2e.ts @@ -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() +}) From 397fd6d58887cf110e8ec19561de3798433e8398 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Tue, 10 Dec 2024 13:11:47 -0600 Subject: [PATCH 2/3] try making @radix-ui/react-use-escape-keydown an explicit dep --- package-lock.json | 1 + package.json | 1 + 2 files changed, 2 insertions(+) diff --git a/package-lock.json b/package-lock.json index 158ee58e87..ccfce27e85 100644 --- a/package-lock.json +++ b/package-lock.json @@ -17,6 +17,7 @@ "@radix-ui/react-dialog": "^1.0.5", "@radix-ui/react-focus-guards": "1.0.1", "@radix-ui/react-tabs": "^1.1.0", + "@radix-ui/react-use-escape-keydown": "^1.1.0", "@react-aria/live-announcer": "^3.3.4", "@react-spring/web": "^9.7.4", "@tanstack/react-query": "^5.56.2", diff --git a/package.json b/package.json index b1c51def4f..dbbf617c78 100644 --- a/package.json +++ b/package.json @@ -39,6 +39,7 @@ "@radix-ui/react-dialog": "^1.0.5", "@radix-ui/react-focus-guards": "1.0.1", "@radix-ui/react-tabs": "^1.1.0", + "@radix-ui/react-use-escape-keydown": "^1.1.0", "@react-aria/live-announcer": "^3.3.4", "@react-spring/web": "^9.7.4", "@tanstack/react-query": "^5.56.2", From 678dfb581c7c894efaa145ae7b6457dafc9506de Mon Sep 17 00:00:00 2001 From: David Crespo Date: Tue, 10 Dec 2024 13:15:01 -0600 Subject: [PATCH 3/3] actually the problem was the node cache --- .github/workflows/lintBuildTest.yml | 6 +++--- package-lock.json | 1 - package.json | 1 - 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/.github/workflows/lintBuildTest.yml b/.github/workflows/lintBuildTest.yml index 9d3c4fbcd8..852297af60 100644 --- a/.github/workflows/lintBuildTest.yml +++ b/.github/workflows/lintBuildTest.yml @@ -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 @@ -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 @@ -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') diff --git a/package-lock.json b/package-lock.json index ccfce27e85..158ee58e87 100644 --- a/package-lock.json +++ b/package-lock.json @@ -17,7 +17,6 @@ "@radix-ui/react-dialog": "^1.0.5", "@radix-ui/react-focus-guards": "1.0.1", "@radix-ui/react-tabs": "^1.1.0", - "@radix-ui/react-use-escape-keydown": "^1.1.0", "@react-aria/live-announcer": "^3.3.4", "@react-spring/web": "^9.7.4", "@tanstack/react-query": "^5.56.2", diff --git a/package.json b/package.json index dbbf617c78..b1c51def4f 100644 --- a/package.json +++ b/package.json @@ -39,7 +39,6 @@ "@radix-ui/react-dialog": "^1.0.5", "@radix-ui/react-focus-guards": "1.0.1", "@radix-ui/react-tabs": "^1.1.0", - "@radix-ui/react-use-escape-keydown": "^1.1.0", "@react-aria/live-announcer": "^3.3.4", "@react-spring/web": "^9.7.4", "@tanstack/react-query": "^5.56.2",