From 873aa3774a4e69b15b596a6bccc9a7dc93a2e940 Mon Sep 17 00:00:00 2001 From: Knacky Date: Wed, 13 May 2026 08:31:13 +0200 Subject: [PATCH] fix(m5): modal layout for the test-template editor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The +New test modal capped at max-w-2xl rendered the 15-column MITRE matrix in a 672px frame with no height cap, so the matrix spilled to the right of the dialog, the form bottom dropped below the viewport, and neither scroll direction worked — buttons were unreachable. - Modal: add a `size` prop (default 2xl, back-compat) with a `7xl` preset. Cap height at calc(100vh-2rem), make the header sticky, and wrap children in a min-w-0 flex-1 overflow-y-auto body so tall content scrolls inside. - MitreTagPicker: move overflow-x-auto from the grid itself to a dedicated scroller wrapper, and add `min-w-0` so the constraint propagates from the modal body. The grid's 1680px intrinsic min-width previously prevented the parent's overflow-x-auto from kicking in. - AdminTestsPage: switch the form layout from `grid gap-3` to `flex flex-col gap-3 min-w-0` and set the modal size to 7xl. The CSS Grid form was propagating min-width: auto to all its items, which let the picker drag the body past the modal width. - AdminScenariosPage: bump the modal to size 3xl for breathing room around the catalogue picker. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 6 ++++ frontend/src/components/MitreTagPicker.tsx | 15 +++++++--- frontend/src/components/ui/Modal.tsx | 35 +++++++++++++++++++--- frontend/src/pages/AdminScenariosPage.tsx | 1 + frontend/src/pages/AdminTestsPage.tsx | 3 +- 5 files changed, 51 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 614e350..661d8d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,12 @@ All notable changes to this project will be documented here. Format: [Keep a Cha - **`LogRecord` key collision**: `log.info(..., extra={"name": ...})` raises `KeyError("Attempt to overwrite 'name' in LogRecord")` because `name` is reserved by Python's stdlib logging. Renamed to `template_name`. - **React `currentTarget` null in deferred state updaters**: `onChange={(e) => setX((prev) => ({ ...prev, q: e.currentTarget.value }))}` blanked the page on the first user input because `currentTarget` is cleared after the listener bubble ends, before React invokes the updater. Switched all M5 handlers to `e.target.value`, which persists on the synthetic event. +### Fixed (post-M5 UI — modal layout for the test-template editor) +- **Modal box capped its width at `max-w-2xl` and had no vertical scroll** (`frontend/src/components/ui/Modal.tsx`): opening **+ New test** rendered the 15-column MITRE matrix inside a 672 px frame with no height cap, so the matrix spilled to the right and the form bottom dropped below the viewport — buttons unreachable, no scroll. Added a `size` prop (default `2xl` for back-compat), `max-h-[calc(100vh-2rem)]` + `flex flex-col` on the dialog, and an inner `min-w-0 flex-1 overflow-y-auto` body so the header stays pinned while the form scrolls inside the modal. +- **MITRE matrix overflow-x failed to scroll inside the modal body** (`frontend/src/components/MitreTagPicker.tsx`): `overflow-x-auto` sat directly on the grid element, but the grid's intrinsic min-width (`15 × minmax(7rem, …)` = 1680 px) prevented it from shrinking below its content, so the grid spilled outside its parent instead of scrolling. Wrapped the grid in a dedicated `overflow-x-auto rounded min-w-0 w-full` scroller and added `min-w-0` to the picker root so the constraint propagates from the modal body. The grid now scrolls horizontally inside the modal. +- **`grid gap-3` form layout in the test-template modal propagated `min-width: auto`** (`frontend/src/pages/AdminTestsPage.tsx`): each grid item refused to shrink below its widest child, so the picker dragged the form (and the body) past the modal width. Switched the form to `flex flex-col gap-3 min-w-0`, which breaks the propagation while preserving vertical spacing. +- **Test-template modal now uses `size="7xl"`** and the scenario-template modal `size="3xl"` to match their content density. + ### Fixed (post-M5 review pass — spec-reviewer + code-reviewer) - **Filter combinator was OR, not AND** (`backend/app/services/test_templates.py:235`): `?tactic=TA0002&technique=T1059` returned templates matching *either* facet instead of *both*. Pre-fix also pooled all three UUIDs into a shared `IN` list across three columns, theoretically allowing a UUID collision to match across kinds. Refactored to one IN-subquery per facet, ANDed together via repeated `WHERE id IN (...)`. - **Concurrent reorder race on `set_scenario_tests`** (`backend/app/services/scenario_templates.py:207`): two parallel reorders on the same scenario could deadlock on the `UNIQUE(scenario_id, position)` constraint under READ COMMITTED. Added a per-scenario `pg_advisory_xact_lock(0x5C3, hash(scenario_id))` mirroring the M4 `/mitre/sync` pattern; different scenarios don't contend. diff --git a/frontend/src/components/MitreTagPicker.tsx b/frontend/src/components/MitreTagPicker.tsx index 4a51fef..e6312bc 100644 --- a/frontend/src/components/MitreTagPicker.tsx +++ b/frontend/src/components/MitreTagPicker.tsx @@ -83,7 +83,7 @@ export function MitreTagPicker({ value, onChange, className }: MitreTagPickerPro } return ( -
+
{/* Selection chips */} {value.length > 0 && (
@@ -132,9 +132,15 @@ export function MitreTagPicker({ value, onChange, className }: MitreTagPickerPro aria-label="MITRE ATT&CK matrix" /* `minmax(7rem, 1fr)` ensures every cell is wide enough for the * longest single word in MITRE names (no mid-word breaks), and - * stretches to fill the container otherwise. Horizontal scroll only - * kicks in on narrow viewports below ~1680px. */ - className="grid gap-px bg-border rounded overflow-x-auto" + * stretches to fill the container otherwise. The wrapper scrolls + * horizontally — placing `overflow-x-auto` on the grid itself fails + * because the grid's intrinsic min-width (15 × 7rem) prevents it + * from shrinking below its parent, so the grid spills out instead + * of scrolling. */ + className="overflow-x-auto rounded min-w-0 w-full" + > +
+
)}

diff --git a/frontend/src/components/ui/Modal.tsx b/frontend/src/components/ui/Modal.tsx index 0bdc464..99d4a17 100644 --- a/frontend/src/components/ui/Modal.tsx +++ b/frontend/src/components/ui/Modal.tsx @@ -4,6 +4,20 @@ import { Button } from '@/components/ui/Button'; import { SectionHeader } from '@/components/ui/SectionHeader'; import { type Accent } from '@/lib/cn'; +type ModalSize = 'md' | 'lg' | 'xl' | '2xl' | '3xl' | '4xl' | '5xl' | '6xl' | '7xl'; + +const SIZE_CLASS: Record = { + md: 'max-w-md', + lg: 'max-w-lg', + xl: 'max-w-xl', + '2xl': 'max-w-2xl', + '3xl': 'max-w-3xl', + '4xl': 'max-w-4xl', + '5xl': 'max-w-5xl', + '6xl': 'max-w-6xl', + '7xl': 'max-w-7xl', +}; + interface ModalProps { open: boolean; title: string; @@ -12,14 +26,27 @@ interface ModalProps { children: ReactNode; /** Optional name to give the dialog role for screen readers / Playwright. */ testid?: string; + /** Max-width preset. Defaults to `2xl` to keep historical behavior. */ + size?: ModalSize; } /** * Centered modal with a backdrop. Closes on Escape and on backdrop click. * The accessible name comes from the SectionHeader's `highlight`, so the dialog * can be located via `getByRole('dialog', { name: ... })`. + * + * The dialog caps its height at the viewport and scrolls its body internally, + * so tall content (MITRE matrix, long forms) never escapes the viewport. */ -export function Modal({ open, title, accent = 'cyan', onClose, children, testid }: ModalProps) { +export function Modal({ + open, + title, + accent = 'cyan', + onClose, + children, + testid, + size = '2xl', +}: ModalProps) { const ref = useRef(null); useEffect(() => { @@ -47,15 +74,15 @@ export function Modal({ open, title, accent = 'cyan', onClose, children, testid aria-modal="true" aria-label={title} data-testid={testid} - className="w-full max-w-2xl rounded-lg border border-border bg-bg-base p-6 shadow-2xl" + className={`flex w-full ${SIZE_CLASS[size]} max-h-[calc(100vh-2rem)] flex-col rounded-lg border border-border bg-bg-base shadow-2xl`} > -

+
- {children} +
{children}
); diff --git a/frontend/src/pages/AdminScenariosPage.tsx b/frontend/src/pages/AdminScenariosPage.tsx index ed0f18c..8c9d6bd 100644 --- a/frontend/src/pages/AdminScenariosPage.tsx +++ b/frontend/src/pages/AdminScenariosPage.tsx @@ -309,6 +309,7 @@ export function AdminScenariosPage() { open={isModalOpen} title={editing ? `Scenario · ${editing.name}` : 'New scenario template'} accent="purple" + size="3xl" onClose={() => { setCreating(false); setEditing(null); diff --git a/frontend/src/pages/AdminTestsPage.tsx b/frontend/src/pages/AdminTestsPage.tsx index ceb7b04..29f89aa 100644 --- a/frontend/src/pages/AdminTestsPage.tsx +++ b/frontend/src/pages/AdminTestsPage.tsx @@ -281,6 +281,7 @@ export function AdminTestsPage() { open={isModalOpen} title={editing ? `Test · ${editing.name}` : 'New test template'} accent="orange" + size="7xl" onClose={() => { setCreating(false); setEditing(null); @@ -288,7 +289,7 @@ export function AdminTestsPage() { testid="test-template-modal" > {error && {error}} -
+