From aae6e1f76e3f076ea8f06d8a535d8b9489f8b84b Mon Sep 17 00:00:00 2001 From: Ali Farrokhnejad Date: Thu, 17 Sep 2026 23:21:53 +0300 Subject: [PATCH 1/3] Fix confirmation dialog layering --- src/components/ConfirmDialog.tsx | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/src/components/ConfirmDialog.tsx b/src/components/ConfirmDialog.tsx index 7e663e0..350b04d 100644 --- a/src/components/ConfirmDialog.tsx +++ b/src/components/ConfirmDialog.tsx @@ -1,6 +1,7 @@ 'use client'; import { useEffect, useId, useRef, type ReactNode } from 'react'; +import { createPortal } from 'react-dom'; import { Button } from '@/components/ui/Button'; interface ConfirmDialogProps { @@ -41,16 +42,22 @@ export function ConfirmDialog({ if (!open) return; const previousActiveElement = document.activeElement as HTMLElement | null; + const previousOverflow = document.body.style.overflow; + document.body.style.overflow = 'hidden'; cancelRef.current?.focus(); const handleKeyDown = (event: KeyboardEvent) => { - if (event.key === 'Escape' && !cancelDisabled) { + if (event.key === 'Escape') { event.preventDefault(); - onCancel(); + event.stopPropagation(); + if (!cancelDisabled) onCancel(); return; } if (event.key !== 'Tab') return; + // A confirmation can sit above another modal/sheet. Keep keyboard handling + // inside the top-most dialog instead of letting the parent focus trap run. + event.stopPropagation(); const focusableElements = panelRef.current?.querySelectorAll( 'button:not(:disabled), [href], input:not(:disabled), select:not(:disabled), textarea:not(:disabled), [tabindex]:not([tabindex="-1"])' ); @@ -67,18 +74,19 @@ export function ConfirmDialog({ } }; - document.addEventListener('keydown', handleKeyDown); + document.addEventListener('keydown', handleKeyDown, true); return () => { - document.removeEventListener('keydown', handleKeyDown); + document.removeEventListener('keydown', handleKeyDown, true); + document.body.style.overflow = previousOverflow; previousActiveElement?.focus(); }; }, [cancelDisabled, onCancel, open]); - if (!open) return null; + if (!open || typeof document === 'undefined') return null; - return ( + return createPortal(
{ if (event.target === event.currentTarget && !cancelDisabled) onCancel(); }} @@ -108,7 +116,8 @@ export function ConfirmDialog({
- + , + document.body ); } From ba557713650702e9644c61870d3e7c3c83184ded Mon Sep 17 00:00:00 2001 From: Ali Farrokhnejad Date: Thu, 17 Sep 2026 23:22:48 +0300 Subject: [PATCH 2/3] Cover nested confirmation dialog layering --- e2e/transactions-dialog-layering.spec.ts | 45 ++++++++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 e2e/transactions-dialog-layering.spec.ts diff --git a/e2e/transactions-dialog-layering.spec.ts b/e2e/transactions-dialog-layering.spec.ts new file mode 100644 index 0000000..dccc904 --- /dev/null +++ b/e2e/transactions-dialog-layering.spec.ts @@ -0,0 +1,45 @@ +import { expect, test, type Page } from '@playwright/test'; + +async function completeFreshOnboarding(page: Page) { + await expect(page.getByRole('heading', { name: 'Track money without slowing down.' })).toBeVisible(); + await page.getByRole('button', { name: 'Get started' }).click(); + await expect(page.getByRole('heading', { name: 'Choose the currencies you use.' })).toBeVisible(); + await page.getByLabel('Cash', { exact: true }).fill('1000'); + await page.getByRole('button', { name: 'Continue' }).click(); + await expect(page.getByRole('heading', { name: 'Make daily logging faster.' })).toBeVisible(); + await page.getByRole('button', { name: 'Continue' }).click(); + await expect(page.getByRole('heading', { name: 'You’re ready.' })).toBeVisible(); + await page.getByRole('button', { name: 'Open Ravel' }).click(); +} + +test('delete confirmation stays above the transaction editor and owns Escape', async ({ page }) => { + await page.goto('/app'); + await completeFreshOnboarding(page); + + await page.goto('/app/add?mode=quick&type=expense&amount=10&title=Layering%20test&method=cash'); + await page.getByRole('button', { name: 'Save expense', exact: true }).click(); + await expect(page.getByText('Transaction saved.', { exact: true })).toBeVisible(); + + await page.getByRole('link', { name: 'View transactions', exact: true }).click(); + await page.getByRole('button', { name: /Layering test/ }).click(); + + const editDialog = page.getByRole('dialog', { name: 'Edit transaction' }); + await expect(editDialog).toBeVisible(); + await editDialog.getByRole('button', { name: 'Delete transaction', exact: true }).click(); + + const deleteDialog = page.getByRole('dialog', { name: 'Delete transaction' }); + await expect(deleteDialog).toBeVisible(); + await expect(editDialog).toBeVisible(); + + const editLayer = await editDialog.evaluate((node) => + Number.parseInt(getComputedStyle(node.parentElement as HTMLElement).zIndex, 10) + ); + const deleteLayer = await deleteDialog.evaluate((node) => + Number.parseInt(getComputedStyle(node.parentElement as HTMLElement).zIndex, 10) + ); + expect(deleteLayer).toBeGreaterThan(editLayer); + + await page.keyboard.press('Escape'); + await expect(deleteDialog).toBeHidden(); + await expect(editDialog).toBeVisible(); +}); From 1579e9815ac2ba182493761339736841149ccad6 Mon Sep 17 00:00:00 2001 From: Ali Farrokhnejad Date: Thu, 17 Sep 2026 23:32:10 +0300 Subject: [PATCH 3/3] Stabilize nested dialog browser regression --- e2e/transactions-dialog-layering.spec.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/e2e/transactions-dialog-layering.spec.ts b/e2e/transactions-dialog-layering.spec.ts index dccc904..d08e440 100644 --- a/e2e/transactions-dialog-layering.spec.ts +++ b/e2e/transactions-dialog-layering.spec.ts @@ -10,13 +10,18 @@ async function completeFreshOnboarding(page: Page) { await page.getByRole('button', { name: 'Continue' }).click(); await expect(page.getByRole('heading', { name: 'You’re ready.' })).toBeVisible(); await page.getByRole('button', { name: 'Open Ravel' }).click(); + await expect(page.getByRole('heading', { name: 'Dashboard', exact: true })).toBeVisible(); } test('delete confirmation stays above the transaction editor and owns Escape', async ({ page }) => { await page.goto('/app'); await completeFreshOnboarding(page); - await page.goto('/app/add?mode=quick&type=expense&amount=10&title=Layering%20test&method=cash'); + await page.goto('/app/add'); + await expect(page.getByRole('heading', { name: 'Add transaction', exact: true })).toBeVisible(); + await page.getByLabel('Amount', { exact: true }).fill('10'); + await page.getByLabel('What was it?').fill('Layering test'); + await page.getByRole('button', { name: 'Cash', exact: true }).click(); await page.getByRole('button', { name: 'Save expense', exact: true }).click(); await expect(page.getByText('Transaction saved.', { exact: true })).toBeVisible();