From 095ec09067beb413f1b549788d5078d09f5c93db Mon Sep 17 00:00:00 2001 From: Danilo Alonso Date: Fri, 9 Oct 2026 23:57:18 -0400 Subject: [PATCH] fix: keep modals above the global findings banner --- src/app/components/ui/JsonValue.tsx | 10 ++-- src/app/components/ui/Modal.tsx | 4 +- src/app/styles.css | 9 ++- test/checks/test-modal-above-banner.ts | 83 ++++++++++++++++++++++++++ 4 files changed, 95 insertions(+), 11 deletions(-) create mode 100644 test/checks/test-modal-above-banner.ts diff --git a/src/app/components/ui/JsonValue.tsx b/src/app/components/ui/JsonValue.tsx index ab4999a..0099018 100644 --- a/src/app/components/ui/JsonValue.tsx +++ b/src/app/components/ui/JsonValue.tsx @@ -1,4 +1,5 @@ import { useMemo, useState } from 'react'; +import { createPortal } from 'react-dom'; import { describeJson, formatJsonFull, formatJsonPreview } from '../../logic/json-value'; import { highlightJson } from '../../logic/json-highlight'; import { Modal } from './Modal'; @@ -10,8 +11,8 @@ import { Modal } from './Modal'; * The dialog is always `stacked`, which covers both callers without either of * them having to know where it sits — from the dictionary page it is the only * modal on screen, and from an entity or process dialog it layers over the - * opener. It renders inline rather than through a portal so that a click on its - * backdrop is caught by the opener's stop-propagation handler. + * opener. It is portaled to `document.body` because the dictionary view is its own + * stacking context, which would trap the dialog under the global findings banner. */ export function JsonValue({ value, label }: { value: object; label: string }) { const [expanded, setExpanded] = useState(false); @@ -35,7 +36,7 @@ export function JsonValue({ value, label }: { value: object; label: string }) { > ⤢ - {expanded && ( + {expanded && createPortal( setExpanded(false)} @@ -47,7 +48,8 @@ export function JsonValue({ value, label }: { value: object; label: string }) { className="json-value-full" dangerouslySetInnerHTML={{ __html: highlighted }} /> - + , + document.body, )} ); diff --git a/src/app/components/ui/Modal.tsx b/src/app/components/ui/Modal.tsx index 4f2ff6a..8ca1014 100644 --- a/src/app/components/ui/Modal.tsx +++ b/src/app/components/ui/Modal.tsx @@ -6,8 +6,8 @@ import type { ReactNode } from 'react'; * so without this only-the-top-entry check a stacked dialog would dismiss the * modal that opened it along with itself. * - * Backdrop clicks need no equivalent guard: a stacked backdrop renders inside - * its opener's `.modal`, whose own handler already stops propagation. + * Backdrop clicks need no equivalent guard: React bubbles a stacked dialog's click + * through its opener's `.modal`, portal or not, and that handler stops propagation. */ const modalStack: object[] = []; diff --git a/src/app/styles.css b/src/app/styles.css index 1173489..64bb912 100644 --- a/src/app/styles.css +++ b/src/app/styles.css @@ -428,13 +428,12 @@ a { display: flex; align-items: center; justify-content: center; - z-index: 100; + /* Above the global findings banner (200): a tall modal reaches the top edge, and the banner would cover its close button. */ + z-index: 210; } -/* A dialog opened from inside another one. It renders within the opener's - `.modal`, so it needs to clear that subtree's stacking order to sit on top. */ .modal-backdrop-stacked { - z-index: 110; + z-index: 220; } .modal { @@ -1153,7 +1152,7 @@ button.flow-crumb__label:hover { Zoom control (CP22) — shared, view-agnostic; Graph view only for now (CP23 will wire it to the Flows view too). Placed bottom-right, above the FAB (bottom:24px + 48px height + 12px gap = 84px). - z-index 55: above canvas (1), below FAB (60) and modals (100+). + z-index 55: above canvas (1), below FAB (60) and modals (210+). ========================================================================= */ .zoom-control { diff --git a/test/checks/test-modal-above-banner.ts b/test/checks/test-modal-above-banner.ts new file mode 100644 index 0000000..c1b2f44 --- /dev/null +++ b/test/checks/test-modal-above-banner.ts @@ -0,0 +1,83 @@ +/** test-modal-above-banner.ts — an open modal stays clickable while the global findings banner is showing. */ + +import { chromium, type Page } from 'playwright'; +import { resolve, join } from 'path'; +import { serveCommand } from '../../src/server/server'; +import { assert } from '../assert'; + +const ROOT = resolve(import.meta.dir, '../..'); +const BROKEN = join(ROOT, 'models/broken-demo'); +const JSON_MODEL = join(ROOT, 'models/key-inherited'); + +const WATCHDOG_MS = 60_000; +const watchdog = setTimeout(() => { + console.error(`FAIL: test-modal-above-banner.ts exceeded its ${WATCHDOG_MS}ms watchdog`); + process.exit(1); +}, WATCHDOG_MS); + +async function closeIsTopmost(page: Page, closeSelector: string) { + return page.evaluate((sel) => { + const closes = document.querySelectorAll(sel); + const close = closes[closes.length - 1]; + const banner = document.querySelector('.graph-global-banner'); + if (!close || !banner) return { found: false, top: false, overlaps: false }; + const c = close.getBoundingClientRect(); + const b = banner.getBoundingClientRect(); + const x = c.left + c.width / 2; + const y = c.top + c.height / 2; + const hit = document.elementFromPoint(x, y); + return { found: true, top: hit !== null && (hit === close || close.contains(hit)), overlaps: y >= b.top && y <= b.bottom }; + }, closeSelector); +} + +const brokenHandle = serveCommand(BROKEN, { port: 3307 }); +const jsonHandle = serveCommand(JSON_MODEL, { port: 3308 }); +const browser = await chromium.launch(); + +try { + const page = await browser.newPage({ viewport: { width: 1280, height: 420 } }); + await page.goto('http://localhost:3307/#entity=Product'); + await page.waitForSelector('.graph-global-banner', { timeout: 15_000 }); + await page.waitForSelector('.modal-close', { timeout: 15_000 }); + const entity = await closeIsTopmost(page, '.modal-close'); + assert(entity.found && entity.overlaps, `FAIL: fixture: the entity modal's close button sits inside the banner band ${JSON.stringify(entity)}`); + assert(entity.top, `FAIL: the entity modal's close button is topmost under the banner ${JSON.stringify(entity)}`); + await page.locator('.modal-close').click({ timeout: 5_000 }); + await page.waitForSelector('.modal-close', { state: 'detached', timeout: 5_000 }); + + // No demo model has both global errors and JSON example values, so the banner is injected with its real class. + const dict = await browser.newPage({ viewport: { width: 1280, height: 420 } }); + await dict.goto('http://localhost:3308/#view=dict'); + await dict.waitForSelector('.json-value-expand', { timeout: 15_000 }); + await dict.evaluate(() => { + const banner = document.createElement('div'); + banner.className = 'graph-global-banner'; + banner.setAttribute('role', 'alert'); + banner.style.height = '160px'; + document.body.appendChild(banner); + }); + await dict.locator('.json-value-expand').first().click({ timeout: 5_000 }); + await dict.waitForSelector('.modal-json .modal-close', { timeout: 5_000 }); + const fromDict = await closeIsTopmost(dict, '.modal-json .modal-close'); + assert(fromDict.found && fromDict.overlaps, `FAIL: fixture: the dictionary JSON dialog's close button sits inside the banner band ${JSON.stringify(fromDict)}`); + assert(fromDict.top, `FAIL: a JSON dialog opened from the dictionary is topmost under the banner ${JSON.stringify(fromDict)}`); + await dict.locator('.modal-json .modal-close').click({ timeout: 5_000 }); + await dict.waitForSelector('.modal-json', { state: 'detached', timeout: 5_000 }); + + const stacked = await browser.newPage({ viewport: { width: 1280, height: 900 } }); + await stacked.goto('http://localhost:3308/#entity=PaymentMethod'); + await stacked.waitForSelector('.modal .json-value-expand', { timeout: 15_000 }); + await stacked.locator('.modal .json-value-expand').first().click({ timeout: 5_000 }); + await stacked.waitForSelector('.modal-backdrop-stacked', { timeout: 5_000 }); + await stacked.mouse.click(5, 5); + await stacked.waitForSelector('.modal-backdrop-stacked', { state: 'detached', timeout: 5_000 }); + const backdropsLeft = await stacked.locator('.modal-backdrop').count(); + assert(backdropsLeft === 1, `FAIL: a backdrop click closes only the stacked JSON dialog, not its opener (backdrops left: ${backdropsLeft})`); + + console.log(process.exitCode ? '\nmodal-above-banner: failures above.' : '\nAll modal-above-banner checks passed.'); +} finally { + await browser.close(); + brokenHandle.stop(true); + jsonHandle.stop(true); + clearTimeout(watchdog); +}