From c25a6d2c5671cd1cc973449e40df7d7dc478df1c Mon Sep 17 00:00:00 2001 From: Nicolas Gonzalez Date: Tue, 15 Sep 2026 10:03:52 +0200 Subject: [PATCH 01/12] feat(panels): lay the right panel out in sections instead of cards Every control in a pane sat in its own bordered box, so a pane read as a stack of five identical containers and the borders carried no information: they said "control" for things that were already obviously controls, and said nothing about which of them belonged together. The structure now comes from a heading that owns what follows it, whitespace between groups, and one rule where a group actually ends. What keeps a fill is what you can press. Sliders lose their box and put the label above a full-width track; their readout goes from mint to muted, because out of a box it sat on the same ground as the mint track directly under it and the two competed over the same fact. The change is made on the shared primitives, so it reaches the captions pane and the V4 floating inspector at the same time rather than leaving the right rail disagreeing with itself tab by tab. The wallpaper picker comes back inline. It went into a popover because 18 swatches plus tabs and the upload button ran past 300px and pushed padding, roundness and shadow off a laptop screen (#84), which is a real failure and not one to repeat: the section is now capped at min(320px, 40vh) and the grid scrolls inside that budget, so it spends a fixed share of the pane however many wallpapers ship. Its tabs, grid and upload button are unchanged. What changed is that they are on the pane rather than over it. Co-Authored-By: Claude Opus 5 (1M context) --- .../ai-edition/NewEditorShell.module.css | 71 ++++++++--- .../ai-edition/RightPanes.i18n.test.tsx | 10 +- src/components/ai-edition/RightPanes.tsx | 110 +++++------------- 3 files changed, 86 insertions(+), 105 deletions(-) diff --git a/src/components/ai-edition/NewEditorShell.module.css b/src/components/ai-edition/NewEditorShell.module.css index d2458328c..aee6bbb21 100644 --- a/src/components/ai-edition/NewEditorShell.module.css +++ b/src/components/ai-edition/NewEditorShell.module.css @@ -851,18 +851,24 @@ align-items: center; justify-content: space-between; gap: 12px; - padding: 12px 14px; - margin: 8px var(--sp-4); - border: 1px solid var(--border); - border-radius: 10px; - background: var(--bg); + padding: 2px 0; + margin: 10px var(--sp-4); } -.paneRow .label { font: 500 var(--fs-app) var(--font-body); color: var(--fg-2); } +.paneRow .label { font: 500 var(--fs-app) var(--font-body); color: var(--fg); } .paneRow .info { color: var(--meta); font-size: var(--fs-app-sm); margin-left: 4px; } +/* Panel layout: sections, not cards. + Every control used to sit in its own bordered box, so a pane read as a stack of + five identical containers and the borders carried no information -- they said + "control" for things that were already obviously controls. The structure now comes + from a heading that owns what follows it, whitespace between groups, and one rule + where a group actually ends. What keeps a fill is what you can press: pills, + buttons, thumbnails. See `.paneDivider` below. */ .sectionLabel { - font: 500 var(--fs-app-sm) var(--font-body); - color: var(--fg-2); - margin: 4px var(--sp-4) 8px; + font: 600 var(--fs-app) var(--font-body); + color: var(--fg); + /* More above than below: the gap is what ties the heading to its controls, so it + has to be smaller than the gap separating it from the group before. */ + margin: 18px var(--sp-4) 10px; } /* ─── action menu (a button's list of things it could do) ─────── */ /* Sits where a Toggle would; reads as the current value, not as on/off. */ @@ -935,8 +941,33 @@ align-items: center; justify-content: space-between; gap: 8px; - margin: 4px var(--sp-4) 8px; + margin: 18px var(--sp-4) 10px; +} +/* The one border left in the flow. It marks where a group ENDS, which a per-control + border never did -- five boxes in a column say nothing about which of them belong + together. */ +.paneDivider { + height: 1px; + margin: 18px var(--sp-4); + border: 0; + background: var(--border); } + +/* The wallpaper picker, inline in the pane instead of over it. + The height cap is the whole reason this can sit inline at all: 18 swatches plus the + tabs and the upload button run past 300px, which is what pushed padding, roundness + and shadow off a laptop screen and sent the picker into a popover in the first place + (#84). Bounded, the section spends a fixed share of the pane and the grid scrolls + inside it, so what is below stays reachable no matter how many wallpapers ship. + `40vh` rather than a px figure so a tall window gives the grid the room it has. */ +.bgInline { + max-height: min(320px, 40vh); + overflow-y: auto; + overscroll-behavior: contain; +} +/* The picker's own rows are padded for a popover that has no other content; inline they + sit under a heading that already provides the space above. */ +.bgInline .paneTabs { margin-top: 0; } .sectionHead .sectionLabel { margin: 0; } /* Deliberately quieter than the settings it overwrites: it is reached on purpose, not stumbled into, and it must not read as a fifth control in the group. */ @@ -962,6 +993,10 @@ align-items: center; justify-content: center; gap: 8px; + /* Full width, like every other control in the pane now that none of them sit in a + box. Half a row wide, it read as one option among several rather than as the one + way in that it is. */ + width: calc(100% - 2 * var(--sp-4)); margin: 0 var(--sp-4) 12px; padding: 12px; border: 1px dashed var(--border); @@ -1115,21 +1150,21 @@ .sliderGrid { display: grid; grid-template-columns: 1fr; - gap: 8px; + /* Was 8px between bordered boxes; with the boxes gone the gap is the only thing + keeping one slider's track off the next one's label. */ + gap: 16px; padding: 0 var(--sp-4) var(--sp-4); } .sliderCell { - border: 1px solid var(--border); - border-radius: 10px; - background: var(--bg); - padding: 10px 12px; display: grid; grid-template-rows: auto auto; - gap: 8px; + gap: 7px; } .sliderCell .head { display: flex; align-items: center; gap: 6px; } -.sliderCell .label { font: 500 11px var(--font-body); color: var(--fg-2); } -.sliderCell .val { font: 500 11px var(--font-mono); color: var(--brand); margin-left: auto; } +.sliderCell .label { font: 500 var(--fs-app) var(--font-body); color: var(--fg); } +/* Muted, not brand. Out of a box the readout sits on the same ground as the label, and + a mint number there competed with the mint track directly under it for the same fact. */ +.sliderCell .val { font: 500 11px var(--font-mono); color: var(--muted); margin-left: auto; } .sliderCell input[type=range] { width: 100%; height: 4px; diff --git a/src/components/ai-edition/RightPanes.i18n.test.tsx b/src/components/ai-edition/RightPanes.i18n.test.tsx index 0be64122b..c16bbbcb5 100644 --- a/src/components/ai-edition/RightPanes.i18n.test.tsx +++ b/src/components/ai-edition/RightPanes.i18n.test.tsx @@ -37,12 +37,12 @@ describe("right-rail panes are localized", () => { expect(screen.getByRole("button", { name: "Aide" })).toBeInTheDocument(); }); - it("renders the floating background picker in French once it is opened", () => { - // The picker collapsed into a popover so the frame sliders stay above the fold, so - // its strings are only in the DOM after the trigger is clicked — the assertion has - // to open it, or it would pass on an empty document forever. + it("renders the inline background picker in French", () => { + // The picker is in the pane, not behind a trigger: it went into a popover to keep + // the frame sliders above the fold, and came back out once its grid was given a + // height budget of its own to scroll inside. So its strings are in the DOM on + // render, with nothing to click first. renderIn("fr", ); - fireEvent.click(screen.getByRole("button", { name: "Arrière-plan" })); expect(screen.getByRole("button", { name: "Téléverser une image" })).toBeInTheDocument(); // wallpaper swatches interpolate their index through the catalog expect(screen.getByRole("button", { name: "Fond 1" })).toBeInTheDocument(); diff --git a/src/components/ai-edition/RightPanes.tsx b/src/components/ai-edition/RightPanes.tsx index bfa10f273..0bcc9c3b4 100644 --- a/src/components/ai-edition/RightPanes.tsx +++ b/src/components/ai-edition/RightPanes.tsx @@ -276,10 +276,10 @@ export function isSupportedBackgroundImage(type: string, fileName: string): bool * that turns the pick into a `data:` URL. * * A hook rather than part of `WallpaperPicker` because WHERE the input may be mounted is - * the caller's problem. `BackgroundSection` renders the picker inside a Popover, and - * opening the OS file dialog takes focus, which closes the Popover — an input mounted - * inside it would unmount mid-pick and drop the file. That caller mounts `input` outside - * the Popover; inline callers mount it next to the picker. + * the caller's problem. A Popover-hosted picker cannot hold it: opening the OS file dialog + * takes focus, which closes the Popover, and an input mounted inside would unmount mid-pick + * and drop the file. `BackgroundSection` is inline now and mounts `input` beside the picker; + * the hook stays a hook because the Popover callers do not go away. */ function useWallpaperFileInput(onPicked: (dataUrl: string) => void): { pick: () => void; @@ -332,7 +332,6 @@ function useWallpaperFileInput(onPicked: (dataUrl: string) => void): { function BackgroundSection() { const ts = useScopedT("settings"); const { settings, set, setLive, commit, hasDocument } = useEditorSettings(); - const [pickerOpen, setPickerOpen] = useState(false); const { pick: handlePickFile, input: fileInput } = useWallpaperFileInput((dataUrl) => set({ wallpaper: dataUrl }), ); @@ -340,56 +339,26 @@ function BackgroundSection() { return ( <>
{ts("background.title")}
- {/* The picker FLOATS instead of sitting inline. Inline, the 18-swatch grid was - ~300px of the pane on its own and pushed padding/roundness/shadow — the - controls #84 is actually about — below the fold on a laptop window. A user - who opened the one appearance tab saw wallpapers and nothing else, which is - the same failure the facet merge set out to fix, one level down. Same - trade the aspect-ratio menu makes in the timeline toolbar: big choice, - small trigger. */} - - - - - -
- void set({ wallpaper: url })} - onLiveChange={(url) => setLive({ wallpaper: url })} - onCommit={commit} - onPickFile={handlePickFile} - /> -
- - - {/* Stays mounted OUTSIDE the popover: opening the OS file dialog takes focus, - which closes the popover and would unmount the input mid-pick, dropping the - file. It has no layout to cost us here. */} + {/* The picker sits INLINE again. It floated because the 18-swatch grid was ~300px + of the pane on its own and pushed padding/roundness/shadow -- the controls #84 + is about -- below the fold, and a user who opened the one appearance tab saw + wallpapers and nothing else. Collapsing it to a trigger fixed that by taking + the space away; this takes back the space and BOUNDS it instead, so the grid + scrolls inside its own budget and the settings under it stay reachable. The + tabs, the grid and the upload button are the picker's own, unchanged: what + changed is that they are on the pane rather than over it. */} +
+ void set({ wallpaper: url })} + onLiveChange={(url) => setLive({ wallpaper: url })} + onCommit={commit} + onPickFile={handlePickFile} + /> +
{fileInput} +
{/* Reads in the order it acts: pick a background, then blur it. Lived under "Effects" while that was a separate facet, which is how a control named "Blur BG" ended up in the tab that doesn't say background. */} @@ -406,38 +375,14 @@ function BackgroundSection() { }} /> + {/* The group is the background AND the blur applied to it, so the rule goes here + rather than above the toggle: between the grid and this it would have split + one idea in two. */} +
); } -/** - * The CSS `background` shorthand that paints a wallpaper value as a swatch — the same - * painting the grid thumbs do, hoisted out so the collapsed trigger shows exactly what the - * grid would show as selected. Bundled wallpapers resolve to their small pre-generated - * thumbnail; colours and gradients are their own literal; a custom `data:` URL passes - * through `resolveImageWallpaperUrl` untouched. - */ -function backgroundSwatchStyle(value: string): CSSProperties { - const classified = classifyWallpaper(value); - if (classified.kind !== "image") return { background: classified.value }; - const bundled = WALLPAPER_PATHS.indexOf(classified.path); - try { - const url = resolveImageWallpaperUrl( - bundled >= 0 ? WALLPAPER_THUMB_PATHS[bundled] : classified.path, - ); - return { background: `center/cover no-repeat url(${url})` }; - } catch { - // resolveImageWallpaperUrl THROWS for an image path outside /wallpapers/ — a guard - // that exists to stop the app loading arbitrary files. The swatch grid only ever - // feeds it constants, but this call site feeds it whatever the document holds, and a - // throw here happens during render: one project saved by an older build with a path - // we no longer allow would take the whole pane down instead of drawing a dull square. - return { background: "var(--surface-2)" }; - } -} - -// keep the user's last data: URL after they switch tabs so the Image -// tab can keep showing it without immediately pushing it back through `set`. function useMemoCustomWallpapers(current: string): string[] { const [cached, setCached] = useState([]); const lastValue = useRef(current); @@ -2593,6 +2538,7 @@ export function VideoEffectsPane() { (zooms, layout changes) — see `effects.motion_blur` driving the tap count in frame_geometry.rs. It is the one control here that never touches the background, so it does not belong under "Frame" either. */} +
{ts("effects.motion")}
Date: Tue, 15 Sep 2026 11:11:51 +0200 Subject: [PATCH 02/12] fix(panels): let the background grid hand its leftover scroll to the pane `overscroll-behavior: contain` on the bounded grid was the reflex and it was wrong. The grid fills most of the pane, so the wheel lands on it almost wherever the pointer is, and containing the scroll meant that once the grid reached its end the pane never moved: measured at grid scrollTop 91 of 91 with the pane still at 0 of 445. Everything under the background section -- frame, format, shadow, roundness, padding, motion -- was unreachable by wheel, which read as the pane simply not having changed. Chaining restored, so the leftover delta goes up to the pane. Verified in the running app: the pane reaches its end and the four sliders come into view. Co-Authored-By: Claude Opus 5 (1M context) --- src/components/ai-edition/NewEditorShell.module.css | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/components/ai-edition/NewEditorShell.module.css b/src/components/ai-edition/NewEditorShell.module.css index aee6bbb21..ce74f0b46 100644 --- a/src/components/ai-edition/NewEditorShell.module.css +++ b/src/components/ai-edition/NewEditorShell.module.css @@ -963,7 +963,13 @@ .bgInline { max-height: min(320px, 40vh); overflow-y: auto; - overscroll-behavior: contain; + /* Chaining left ON, deliberately. `overscroll-behavior: contain` is the reflex here + and it was wrong: the grid fills most of the pane, so the wheel lands on it almost + wherever the pointer is, and containing the scroll meant that once the grid hit its + end the pane never moved at all -- everything under the background section became + unreachable by wheel. Chaining hands the leftover delta up, which is the behaviour + a nested scroller owes its parent when it is this large a share of it. */ + overscroll-behavior-y: auto; } /* The picker's own rows are padded for a popover that has no other content; inline they sit under a heading that already provides the space above. */ From e4899dd52d59bfe591e2a6354f7ad9b4f35a2859 Mon Sep 17 00:00:00 2001 From: Nicolas Gonzalez Date: Tue, 15 Sep 2026 11:18:09 +0200 Subject: [PATCH 03/12] feat(inspector): seat the facet rail against the panel it drives The rail floated to the RIGHT of the panel with a 10px gap, so the control that picks a facet sat on the far side of the thing it picked, and the two read as unrelated cards. It now leads: rail on the left, flush against the panel, the pair sharing one edge and one height. A DOM reorder rather than `row-reverse`, so the visual order and the tab order agree -- you meet the chooser, then what it chose. Both go full height when the panel is open. Closed, the rail is a floating pill of its own height again: stretching an empty rail the height of the stage would be a tall column of nothing over the video, which is why the joined state is behind an attribute and not a blanket rule. Co-Authored-By: Claude Opus 5 (1M context) --- .../ai-edition/v4/EditorShellV4.module.css | 22 +++++++++++++++ .../ai-edition/v4/FloatingInspector.tsx | 28 +++++++++++-------- 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/src/components/ai-edition/v4/EditorShellV4.module.css b/src/components/ai-edition/v4/EditorShellV4.module.css index 4b456274d..bc4f3e7bb 100644 --- a/src/components/ai-edition/v4/EditorShellV4.module.css +++ b/src/components/ai-edition/v4/EditorShellV4.module.css @@ -636,9 +636,31 @@ z-index: 20; pointer-events: none; } +/* Open, the rail and the panel are ONE object: same height, touching, sharing the edge + between them. Closed, the rail is on its own again and goes back to being a floating + pill of its own height — stretching an empty rail to the full stage would be a tall + column of nothing over the video. Hence the attribute rather than a blanket rule. */ +.inspectorWrap[data-open] { + align-items: stretch; + gap: 0; +} +.inspectorWrap[data-open] .facetRail { + /* Buttons stay at the top of a rail that is now as tall as the panel. */ + justify-content: flex-start; + border-top-right-radius: 0; + border-bottom-right-radius: 0; + /* The panel's own left border is the shared one; two of them would draw 2px. */ + border-right: 0; +} +.inspectorWrap[data-open] .inspector { + border-top-left-radius: 0; + border-bottom-left-radius: 0; +} .inspector { pointer-events: auto; width: 300px; + /* `stretch` on the wrap gives it the full height; this keeps it from exceeding that + on its own, and keeps the closed/legacy case honest. */ max-height: 100%; display: flex; flex-direction: column; diff --git a/src/components/ai-edition/v4/FloatingInspector.tsx b/src/components/ai-edition/v4/FloatingInspector.tsx index 2a4a4904c..347fa32b1 100644 --- a/src/components/ai-edition/v4/FloatingInspector.tsx +++ b/src/components/ai-edition/v4/FloatingInspector.tsx @@ -120,18 +120,11 @@ export function FloatingInspector({ const audioTrackSelected = Boolean(tl.selectedAudioTrackId); const effectiveOpen = open || selection !== null || audioTrackSelected; return ( -
- {effectiveOpen ? ( -
- {selection ? ( - tl.clearSelection()} /> - ) : audioTrackSelected ? ( - tl.clearSelection()} /> - ) : ( - - )} -
- ) : null} +
+ {/* The rail leads, the panel follows. It is the chooser and the panel is what it + chose, so meeting it first is the right reading order for a keyboard and a + screen reader alike -- which is why this is a DOM reorder and not + `row-reverse`, where the two would disagree. */}
{FACETS.map(({ id, labelKey, icon: Icon }) => (
+ {effectiveOpen ? ( +
+ {selection ? ( + tl.clearSelection()} /> + ) : audioTrackSelected ? ( + tl.clearSelection()} /> + ) : ( + + )} +
+ ) : null}
); } From ffb588e86cda5e209c182ec3d672d00977efd8d9 Mon Sep 17 00:00:00 2001 From: Nicolas Gonzalez Date: Tue, 15 Sep 2026 11:26:35 +0200 Subject: [PATCH 04/12] test(inspector): pin the data-open contract the joined layout rests on From CodeRabbit on #671. `data-open` is not decoration: it is what joins the rail to the panel and gives the pair one height, and its absence is what lets the rail go back to a pill of its own size rather than stretching an empty column the height of the stage. Nothing asserted it, so either half of that switch could have been lost silently. All three ways of turning it on are covered -- the panel being open, a selection, a selected audio track -- plus the off case. The selection path renders the real `SelectionPane`, which is local to the component, so the editor-settings hook is stubbed to the one field it reads; the region list is left empty on purpose, since the attribute answers to `selection !== null` and not to what the pane makes of it. Co-Authored-By: Claude Opus 5 (1M context) --- .../ai-edition/v4/FloatingInspector.test.tsx | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/src/components/ai-edition/v4/FloatingInspector.test.tsx b/src/components/ai-edition/v4/FloatingInspector.test.tsx index 39f644435..d21187686 100644 --- a/src/components/ai-edition/v4/FloatingInspector.test.tsx +++ b/src/components/ai-edition/v4/FloatingInspector.test.tsx @@ -31,6 +31,19 @@ vi.mock("../CaptionsPane", () => ({ CaptionsPane: () =>
CaptionsPane
, })); +// `SelectionPane` is local to the component under test, so the selection path renders it +// for real and it reads the editor settings. Stubbed to the one field it consults here, +// rather than standing up the project store to assert an attribute on a wrapper. +vi.mock("@/lib/ai-edition/store/useEditorSettings", () => ({ + useEditorSettings: () => ({ + settings: { autoFocusAll: false }, + set: vi.fn(), + setLive: vi.fn(), + commit: vi.fn(), + hasDocument: false, + }), +})); + import { FloatingInspector } from "./FloatingInspector"; describe("FloatingInspector", () => { @@ -73,6 +86,43 @@ describe("FloatingInspector", () => { expect(onToggleOpen).toHaveBeenCalledTimes(1); }); + // `data-open` is not decoration: it is what joins the rail to the panel and gives the + // pair one height. Closed, the rail has to go back to being a pill of its own size, or + // an empty rail stretches the height of the stage. So the attribute carries the whole + // of that switch and each way of turning it on is worth pinning. + const wrap = () => document.querySelector('[class*="inspectorWrap"]'); + + it("leaves data-open off while nothing is open or selected", () => { + render(); + expect(wrap()).not.toHaveAttribute("data-open"); + }); + + it("sets data-open when the panel is open", () => { + render(); + expect(wrap()).toHaveAttribute("data-open", "true"); + }); + + it("sets data-open on a selection, even with the panel collapsed", () => { + // No matching region, so `SelectionPane` renders null. That is the point: the + // attribute answers to `selection !== null`, not to what the pane makes of it. + const tl = { + ...defaultProps.tl, + selection: { kind: "annotation", id: "ann_1" }, + annotationRegions: [], + } as unknown as React.ComponentProps["tl"]; + render(); + expect(wrap()).toHaveAttribute("data-open", "true"); + }); + + it("sets data-open on a selected audio track, even with the panel collapsed", () => { + const tl = { + ...defaultProps.tl, + selectedAudioTrackId: "audio-1", + } as unknown as React.ComponentProps["tl"]; + render(); + expect(wrap()).toHaveAttribute("data-open", "true"); + }); + it("renders close button on AudioTrackPane when audio track is selected and deselects on click", () => { const clearSelection = vi.fn(); const tl = { From 8626c37b886ac3a1b6634f341067b9ed810ebb9e Mon Sep 17 00:00:00 2001 From: Nicolas Gonzalez Date: Tue, 15 Sep 2026 11:53:55 +0200 Subject: [PATCH 05/12] fix(hud): give the HUD its own focus ring instead of the system accent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No control in the HUD styled its focus, so every one of them fell through to the platform default ring. macOS paints that ring in the user's system accent colour, so a user who set the accent to orange got a thick orange band around a bar drawn entirely in mint and slate — heavier than any other line in a 34px bar, and in a hue the HUD never uses. It is not only a keyboard cost. `:focus-visible` matches on a programmatic focus() and when the overlay window takes focus back, both measured here, and this window loses and regains focus constantly. The ring reappeared on a button the user had clicked with the mouse minutes earlier. One rule on `.hudAnchor` covers the bar, the popovers and the notices, so a control added later inherits it rather than falling through again. It sits ahead of the specific rules in the file, so the language menu items and the about actions — which deliberately answer focus with a background rather than a ring — still win on source order. Measured in the running HUD: a mouse click still draws nothing (`:focus-visible` false, `outline: none`); Tab and focus() now draw `solid 1px rgba(16, 185, 129, 0.75)` at 2px offset instead of the UA's `auto 1px`; a language menu item still resolves to `outline: none`. Co-Authored-By: Claude Opus 5 (1M context) --- src/components/launch/LaunchWindow.module.css | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/components/launch/LaunchWindow.module.css b/src/components/launch/LaunchWindow.module.css index 8821194b1..a356d2837 100644 --- a/src/components/launch/LaunchWindow.module.css +++ b/src/components/launch/LaunchWindow.module.css @@ -7,6 +7,26 @@ -webkit-app-region: no-drag; } +/* + * Nothing in the HUD styled its own focus ring, so every control fell through to + * the platform default. On macOS that default is painted in the system accent + * colour, at a weight no other line in a 34px bar comes close to — a user with + * the accent set to orange gets a thick orange band around a mint-and-slate bar. + * + * It is not only a keyboard cost. `:focus-visible` also matches on a programmatic + * focus() and on the overlay window taking focus back, and this window loses and + * regains focus constantly, so the ring reappears unbidden on a button the user + * clicked with the mouse minutes ago. + * + * One rule on the anchor covers the bar, the popovers and the notices. It sits + * here, ahead of the specific rules, so the menu items further down — which + * deliberately answer focus with a background instead of a ring — still win. + */ +.hudAnchor :focus-visible { + outline: 1px solid rgba(16, 185, 129, 0.75); + outline-offset: 2px; +} + /* * The HUD is one bottom-anchored, upward-growing stack: the bar sits at the * bottom, popovers and notices float above it in DOM order. Everything that From d266849a792e2e0f206a322bf345a22bbae217d9 Mon Sep 17 00:00:00 2001 From: Nicolas Gonzalez Date: Tue, 15 Sep 2026 14:59:48 +0200 Subject: [PATCH 06/12] feat(toasts): give a toast a cross to close it, inside its top end corner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A toast could only be waited out. That is fine for a confirmation and wrong for an error: the 3s timer retires a long description before it has been read, and a run of failures stacks over the editor with nothing to clear it. Sonner ships a close button, so the work is where it puts one. Its default is the START side — top-LEFT in a left-to-right UI — and half outside the box, hanging off the corner like a badge. Nothing else here closes there: every dialog, pane and inspector puts its cross inside the top end corner, and a dismiss control that moves with the surface is one the eye has to hunt for. Three things had to move together, which is why this is a stylesheet block rather than one prop: - the side, through sonner's own custom properties; - the overhang, by dropping its `translate(35%, -35%)`; - `top`, which sonner hardcodes to 0 instead of exposing as a variable, so it takes a rule on the element. Flipped per direction rather than hard-coded to `right`. `ar` ships today and the app does not set `dir` yet — this must not be the thing that breaks when it does. The leading `html` in each selector is load-bearing too: sonner injects its stylesheet at runtime, after ours, so an override of equal specificity would lose on source order. Inside the box, sonner's bordered opaque chip stops making sense — it reads as a second surface floating on the toast — so the button is a plain icon that only gains weight on hover, and the toast reserves `padding-inline-end` so a long title cannot run under it. The label comes from our own strings: sonner's default is an untranslated "Close toast", and the rest of the app speaks 13 languages. The tests pin what jsdom can actually see — the button exists, and it carries the localized label — plus the three custom properties the placement rests on, in sonner's stylesheet and ours. That last one is the real guard: on an upgrade that renames them the override silently stops applying, the cross goes back to the top left, and every other test stays green. Also drops an unused `fireEvent` import in RightPanes.i18n.test.tsx that was failing this branch's "Typecheck (tests)" job. Co-Authored-By: Claude Opus 5 (1M context) --- .../ai-edition/RightPanes.i18n.test.tsx | 2 +- src/components/ui/sonner.test.tsx | 77 +++++++++++++++++++ src/components/ui/sonner.tsx | 15 ++++ src/i18n/locales/ar/common.json | 3 +- src/i18n/locales/cs/common.json | 3 +- src/i18n/locales/en/common.json | 3 +- src/i18n/locales/es/common.json | 3 +- src/i18n/locales/fr/common.json | 3 +- src/i18n/locales/it/common.json | 3 +- src/i18n/locales/ja-JP/common.json | 3 +- src/i18n/locales/ko-KR/common.json | 3 +- src/i18n/locales/pt-BR/common.json | 3 +- src/i18n/locales/ru/common.json | 3 +- src/i18n/locales/tr/common.json | 3 +- src/i18n/locales/vi/common.json | 3 +- src/i18n/locales/zh-CN/common.json | 3 +- src/i18n/locales/zh-TW/common.json | 3 +- src/index.css | 50 ++++++++++++ 18 files changed, 171 insertions(+), 15 deletions(-) create mode 100644 src/components/ui/sonner.test.tsx diff --git a/src/components/ai-edition/RightPanes.i18n.test.tsx b/src/components/ai-edition/RightPanes.i18n.test.tsx index c16bbbcb5..06a71c1b5 100644 --- a/src/components/ai-edition/RightPanes.i18n.test.tsx +++ b/src/components/ai-edition/RightPanes.i18n.test.tsx @@ -6,7 +6,7 @@ // localized text is what actually reaches the DOM. import "@testing-library/jest-dom"; -import { cleanup, fireEvent, render, screen } from "@testing-library/react"; +import { cleanup, render, screen } from "@testing-library/react"; import type { ReactElement } from "react"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { I18nProvider } from "@/contexts/I18nContext"; diff --git a/src/components/ui/sonner.test.tsx b/src/components/ui/sonner.test.tsx new file mode 100644 index 000000000..a34108fab --- /dev/null +++ b/src/components/ui/sonner.test.tsx @@ -0,0 +1,77 @@ +// @vitest-environment jsdom +// A toast used to be dismissable only by waiting. That is fine for a confirmation and +// wrong for an error: the 3s timer can retire a long description before it has been read, +// and a run of failures stacks up over the editor with nothing to clear it. +// +// The placement itself (top END corner, mirrored for RTL) is CSS custom properties in +// src/index.css, which jsdom does not apply — what is pinned here is that the button +// exists at all and that it speaks the app's language rather than sonner's hardcoded +// "Close toast". +import "@testing-library/jest-dom"; +import { readFileSync } from "node:fs"; +import path from "node:path"; +import { render, screen } from "@testing-library/react"; +import { toast } from "sonner"; +import { afterEach, describe, expect, it } from "vitest"; +import { I18nProvider } from "@/contexts/I18nContext"; +import { Toaster } from "./sonner"; + +afterEach(() => { + toast.dismiss(); +}); + +describe("Toaster", () => { + it("gives every toast a close button", async () => { + render( + + + , + ); + toast("Export finished"); + expect(await screen.findByText("Export finished")).toBeInTheDocument(); + expect(await screen.findByRole("button", { name: /close notification/i })).toBeInTheDocument(); + }); + + // Sonner's default label is the untranslated "Close toast", and a screen reader in any + // of the other twelve locales would hear it. + it("labels it from the app's own strings, not sonner's default", async () => { + render( + + + , + ); + toast.error("Could not export"); + const close = await screen.findByRole("button", { name: /close notification/i }); + expect(close).toHaveAttribute("data-close-button", "true"); + expect(screen.queryByRole("button", { name: /close toast/i })).not.toBeInTheDocument(); + }); +}); + +// The placement is three CSS custom properties that sonner owns. jsdom cannot tell us +// where the button lands, but it can tell us the contract still exists — and that is the +// failure mode that matters: on a sonner upgrade that renames them, the override quietly +// stops applying and the cross goes back to the top LEFT with every test still green. +describe("close button placement contract", () => { + const root = path.resolve(__dirname, "..", "..", ".."); + const sonnerCss = readFileSync(path.join(root, "node_modules/sonner/dist/styles.css"), "utf8"); + const appCss = readFileSync(path.join(root, "src/index.css"), "utf8"); + + for (const variable of [ + "--toast-close-button-start", + "--toast-close-button-end", + "--toast-close-button-transform", + ]) { + it(`sonner still positions the close button with ${variable}`, () => { + expect(sonnerCss).toContain(variable); + expect(appCss).toContain(variable); + }); + } + + // Both directions, because the override exists to MOVE the button, and doing that in + // one direction only would leave ar mirrored the wrong way once the app sets `dir`. + for (const dir of ["ltr", "rtl"]) { + it(`overrides it for dir="${dir}"`, () => { + expect(appCss).toContain(`[data-sonner-toaster][dir="${dir}"]`); + }); + } +}); diff --git a/src/components/ui/sonner.tsx b/src/components/ui/sonner.tsx index 3076ac1fe..10a3ab5f9 100644 --- a/src/components/ui/sonner.tsx +++ b/src/components/ui/sonner.tsx @@ -1,9 +1,11 @@ import { Toaster as Sonner } from "sonner"; +import { useScopedT } from "@/contexts/I18nContext"; import { cn } from "@/lib/utils"; type ToasterProps = React.ComponentProps; const Toaster = ({ className, ...props }: ToasterProps) => { + const tc = useScopedT("common"); return ( { className, )} duration={3000} + // A toast that can only be waited out is the one case where the 3s timer works + // against the reader: an error with a long description is dismissed before it is + // finished, and a stack of them hides the editor with no way to clear it. The + // cross is placed on the END side by `src/index.css` — sonner puts it on the + // start side, which is not where anything else in this app closes. + closeButton toastOptions={{ + // Sonner's default is the untranslated "Close toast"; the rest of the app + // speaks 13 languages. + closeButtonAriaLabel: tc("actions.closeNotification"), classNames: { toast: "group toast border border-white/10 bg-[#09090b] text-slate-200 shadow-lg backdrop-blur-xl", description: "group-[.toast]:text-slate-400", actionButton: "group-[.toast]:bg-primary group-[.toast]:text-primary-foreground", cancelButton: "group-[.toast]:bg-muted group-[.toast]:text-muted-foreground", + // The button's colours live in src/index.css with its placement: sitting + // inside the toast, the two are one decision, and splitting them across a + // class list and a stylesheet is how they drift. + closeButton: "group-[.toast]:text-slate-400 group-[.toast]:hover:text-slate-100", }, }} {...props} diff --git a/src/i18n/locales/ar/common.json b/src/i18n/locales/ar/common.json index 530deaf55..59cdbdfc1 100644 --- a/src/i18n/locales/ar/common.json +++ b/src/i18n/locales/ar/common.json @@ -45,7 +45,8 @@ "hide": "إخفاء OpenScreen", "hideOthers": "إخفاء الآخرين", "unhide": "إظهار الكل", - "saveDiagnostics": "حفظ التشخيصات" + "saveDiagnostics": "حفظ التشخيصات", + "closeNotification": "إغلاق الإشعار" }, "updates": { "available": "يتوفر OpenScreen {{latestVersion}}. الإصدار المثبت هو {{currentVersion}}.", diff --git a/src/i18n/locales/cs/common.json b/src/i18n/locales/cs/common.json index c68a3e456..97e1111f3 100644 --- a/src/i18n/locales/cs/common.json +++ b/src/i18n/locales/cs/common.json @@ -45,7 +45,8 @@ "hide": "Skrýt OpenScreen", "hideOthers": "Skrýt ostatní", "unhide": "Zobrazit vše", - "saveDiagnostics": "Uložit diagnostiku" + "saveDiagnostics": "Uložit diagnostiku", + "closeNotification": "Zavřít oznámení" }, "updates": { "available": "OpenScreen {{latestVersion}} je k dispozici. Používáte verzi {{currentVersion}}.", diff --git a/src/i18n/locales/en/common.json b/src/i18n/locales/en/common.json index d1dd7d085..b47789b36 100644 --- a/src/i18n/locales/en/common.json +++ b/src/i18n/locales/en/common.json @@ -45,7 +45,8 @@ "hide": "Hide OpenScreen", "hideOthers": "Hide Others", "unhide": "Show All", - "saveDiagnostics": "Save Diagnostics" + "saveDiagnostics": "Save Diagnostics", + "closeNotification": "Close notification" }, "updates": { "available": "OpenScreen {{latestVersion}} is available. You are using {{currentVersion}}.", diff --git a/src/i18n/locales/es/common.json b/src/i18n/locales/es/common.json index 92de74416..1919e489b 100644 --- a/src/i18n/locales/es/common.json +++ b/src/i18n/locales/es/common.json @@ -45,7 +45,8 @@ "hide": "Ocultar OpenScreen", "hideOthers": "Ocultar otros", "unhide": "Mostrar todo", - "saveDiagnostics": "Guardar diagnósticos" + "saveDiagnostics": "Guardar diagnósticos", + "closeNotification": "Cerrar la notificación" }, "updates": { "available": "OpenScreen {{latestVersion}} está disponible. Estás usando {{currentVersion}}.", diff --git a/src/i18n/locales/fr/common.json b/src/i18n/locales/fr/common.json index 6d5454603..d3ab8e030 100644 --- a/src/i18n/locales/fr/common.json +++ b/src/i18n/locales/fr/common.json @@ -45,7 +45,8 @@ "hide": "Masquer OpenScreen", "hideOthers": "Masquer les autres", "unhide": "Tout afficher", - "saveDiagnostics": "Enregistrer les diagnostics" + "saveDiagnostics": "Enregistrer les diagnostics", + "closeNotification": "Fermer la notification" }, "updates": { "available": "OpenScreen {{latestVersion}} est disponible. Vous utilisez la version {{currentVersion}}.", diff --git a/src/i18n/locales/it/common.json b/src/i18n/locales/it/common.json index d831ec93b..7762cffc0 100644 --- a/src/i18n/locales/it/common.json +++ b/src/i18n/locales/it/common.json @@ -45,7 +45,8 @@ "hide": "Nascondi OpenScreen", "hideOthers": "Nascondi gli altri", "unhide": "Mostra tutto", - "saveDiagnostics": "Salva dati diagnostici" + "saveDiagnostics": "Salva dati diagnostici", + "closeNotification": "Chiudi la notifica" }, "updates": { "available": "OpenScreen {{latestVersion}} è disponibile. Stai usando la versione {{currentVersion}}.", diff --git a/src/i18n/locales/ja-JP/common.json b/src/i18n/locales/ja-JP/common.json index a1375e0ed..af2e34144 100644 --- a/src/i18n/locales/ja-JP/common.json +++ b/src/i18n/locales/ja-JP/common.json @@ -45,7 +45,8 @@ "hide": "OpenScreenを隠す", "hideOthers": "ほかを隠す", "unhide": "すべて表示", - "saveDiagnostics": "診断情報を保存" + "saveDiagnostics": "診断情報を保存", + "closeNotification": "通知を閉じる" }, "updates": { "available": "OpenScreen {{latestVersion}} を利用できます。現在のバージョンは {{currentVersion}} です。", diff --git a/src/i18n/locales/ko-KR/common.json b/src/i18n/locales/ko-KR/common.json index c9ddd9ba7..3d2e9905e 100644 --- a/src/i18n/locales/ko-KR/common.json +++ b/src/i18n/locales/ko-KR/common.json @@ -45,7 +45,8 @@ "hide": "OpenScreen 숨기기", "hideOthers": "다른 항목 숨기기", "unhide": "모두 보기", - "saveDiagnostics": "진단 정보 저장" + "saveDiagnostics": "진단 정보 저장", + "closeNotification": "알림 닫기" }, "updates": { "available": "OpenScreen {{latestVersion}} 버전을 사용할 수 있습니다. 현재 버전은 {{currentVersion}}입니다.", diff --git a/src/i18n/locales/pt-BR/common.json b/src/i18n/locales/pt-BR/common.json index 15c673daf..b5be265c9 100644 --- a/src/i18n/locales/pt-BR/common.json +++ b/src/i18n/locales/pt-BR/common.json @@ -45,7 +45,8 @@ "hide": "Ocultar OpenScreen", "hideOthers": "Ocultar Outros", "unhide": "Mostrar Todos", - "saveDiagnostics": "Salvar Diagnósticos" + "saveDiagnostics": "Salvar Diagnósticos", + "closeNotification": "Fechar a notificação" }, "updates": { "available": "O OpenScreen {{latestVersion}} está disponível. Você está usando a versão {{currentVersion}}.", diff --git a/src/i18n/locales/ru/common.json b/src/i18n/locales/ru/common.json index 67411840b..e73234bb4 100644 --- a/src/i18n/locales/ru/common.json +++ b/src/i18n/locales/ru/common.json @@ -45,7 +45,8 @@ "hide": "Скрыть OpenScreen", "hideOthers": "Скрыть остальные", "unhide": "Показать все", - "saveDiagnostics": "Сохранить диагностику" + "saveDiagnostics": "Сохранить диагностику", + "closeNotification": "Закрыть уведомление" }, "updates": { "available": "Доступен OpenScreen {{latestVersion}}. Установлена версия {{currentVersion}}.", diff --git a/src/i18n/locales/tr/common.json b/src/i18n/locales/tr/common.json index a5bb28a32..3df4c0d5f 100644 --- a/src/i18n/locales/tr/common.json +++ b/src/i18n/locales/tr/common.json @@ -45,7 +45,8 @@ "hide": "OpenScreen’i Gizle", "hideOthers": "Diğerlerini Gizle", "unhide": "Tümünü Göster", - "saveDiagnostics": "Teşhis Verilerini Kaydet" + "saveDiagnostics": "Teşhis Verilerini Kaydet", + "closeNotification": "Bildirimi kapat" }, "updates": { "available": "OpenScreen {{latestVersion}} kullanılabilir. Mevcut sürümünüz {{currentVersion}}.", diff --git a/src/i18n/locales/vi/common.json b/src/i18n/locales/vi/common.json index 0d4cc878b..ba9a964c3 100644 --- a/src/i18n/locales/vi/common.json +++ b/src/i18n/locales/vi/common.json @@ -45,7 +45,8 @@ "hide": "Ẩn OpenScreen", "hideOthers": "Ẩn ứng dụng khác", "unhide": "Hiển thị tất cả", - "saveDiagnostics": "Lưu thông tin chẩn đoán" + "saveDiagnostics": "Lưu thông tin chẩn đoán", + "closeNotification": "Đóng thông báo" }, "updates": { "available": "Đã có OpenScreen {{latestVersion}}. Bạn đang dùng phiên bản {{currentVersion}}.", diff --git a/src/i18n/locales/zh-CN/common.json b/src/i18n/locales/zh-CN/common.json index e789cbd5b..e7facb4ed 100644 --- a/src/i18n/locales/zh-CN/common.json +++ b/src/i18n/locales/zh-CN/common.json @@ -45,7 +45,8 @@ "hide": "隐藏 OpenScreen", "hideOthers": "隐藏其他", "unhide": "显示全部", - "saveDiagnostics": "保存诊断信息" + "saveDiagnostics": "保存诊断信息", + "closeNotification": "关闭通知" }, "updates": { "available": "OpenScreen {{latestVersion}} 已发布。当前版本为 {{currentVersion}}。", diff --git a/src/i18n/locales/zh-TW/common.json b/src/i18n/locales/zh-TW/common.json index 8e3d0648c..7fc0eab1f 100644 --- a/src/i18n/locales/zh-TW/common.json +++ b/src/i18n/locales/zh-TW/common.json @@ -45,7 +45,8 @@ "hide": "隱藏 OpenScreen", "hideOthers": "隱藏其他", "unhide": "全部顯示", - "saveDiagnostics": "儲存診斷資料" + "saveDiagnostics": "儲存診斷資料", + "closeNotification": "關閉通知" }, "updates": { "available": "OpenScreen {{latestVersion}} 已推出。目前版本為 {{currentVersion}}。", diff --git a/src/index.css b/src/index.css index eb7cb8384..ac9686726 100644 --- a/src/index.css +++ b/src/index.css @@ -329,3 +329,53 @@ -ms-overflow-style: none; } } + +/* Sonner puts the toast close button on the START side — top-left in a left-to-right UI — + and overhanging the corner, half outside the toast. Nothing else in this app closes + there: every dialog, pane and inspector puts its cross INSIDE the top end corner, and a + dismiss control that moves depending on the surface is one the eye has to hunt for. + + Three things have to change together, which is why this is not a one-liner: + - the side, via sonner's own custom properties; + - the overhang, by dropping its translate — sonner's default pulls the button out of + the box by 35% in both axes; + - `top`, which sonner hardcodes to 0 rather than exposing as a variable, so it takes a + rule on the element itself. + + Flipped per direction rather than hard-coded to `right`, so the mirroring survives an + RTL locale (ar ships today; the app does not set `dir` yet, but this must not be the + thing that breaks when it does). + + The leading `html` is not decoration: sonner injects its stylesheet at runtime, after + this file, so an override of equal specificity would lose on source order. */ +html [data-sonner-toaster][dir="ltr"] { + --toast-close-button-start: unset; + --toast-close-button-end: 8px; + --toast-close-button-transform: none; +} + +html [data-sonner-toaster][dir="rtl"] { + --toast-close-button-start: 8px; + --toast-close-button-end: unset; + --toast-close-button-transform: none; +} + +/* Room for the button, so a long title does not run under it. Logical, not `padding-right`, + for the same mirroring reason as above. */ +html [data-sonner-toaster] [data-sonner-toast][data-styled="true"] { + padding-inline-end: 36px; +} + +/* Inside the box, the chrome sonner gives the button stops making sense: a bordered, + opaque chip reads as a badge stuck on the corner when it is overhanging, and as a second + surface floating on the toast when it is not. Plain icon, weight on hover only. */ +html [data-sonner-toaster] [data-sonner-toast][data-styled="true"] [data-close-button] { + top: 8px; + border-color: transparent; + background: transparent; + transition: background 0.12s ease-out, color 0.12s ease-out; +} + +html [data-sonner-toaster] [data-sonner-toast][data-styled="true"] [data-close-button]:hover { + background: rgb(255 255 255 / 10%); +} From b9db771e5b08acdb8a492e20102cbc7ced4a0fdb Mon Sep 17 00:00:00 2001 From: Nicolas Gonzalez Date: Wed, 16 Sep 2026 19:43:30 +0200 Subject: [PATCH 07/12] fix(inspector): keep the facet rail still while the panel opens beside it Seating the rail against the panel moved it in front of the panel in a wrap anchored by its right edge, so opening the 300px panel pushed the rail 300px left. The active facet was no longer under the pointer that had just clicked it: a second click to close the panel landed inside the panel instead, and selecting or deselecting a timeline region made the whole rail jump. The rail now keeps one spot on screen, at the stage's end edge, and the panel opens towards the start. The joined look stays as it was, mirrored: same height, one shared border, the rail's radius dropped on the seam side and the panel's on its own. Dropping the rail's border on its start side rather than its end is also what keeps its buttons at the same pixel open and closed. The DOM order does not change. The wrap lays out with `row-reverse`, so the rail still comes first for a keyboard or a screen reader: press a facet and the next Tab is in the controls it opened, instead of behind the rest of the rail. The visual order now runs the other way, but the rail is a vertical toolbar pinned to the edge, not a line read across, and chooser-then-content is the order tabbing needs. Anchor, flex direction, borders and radii are logical properties, so the pair mirrors to the stage's left once the app sets dir="rtl" for ar. Checked in a browser (Vite + Playwright, 1440x900): the first facet button stays at the same coordinates open and closed, in LTR and with dir="rtl" forced on . The tests pin the re-click-to-close path, the rail-before-panel DOM order, and the CSS contract behind the placement, which jsdom cannot lay out. Co-Authored-By: Claude Opus 5 (1M context) --- .../ai-edition/v4/EditorShellV4.module.css | 32 ++++++--- .../ai-edition/v4/FloatingInspector.test.tsx | 68 +++++++++++++++++++ .../ai-edition/v4/FloatingInspector.tsx | 15 ++-- 3 files changed, 102 insertions(+), 13 deletions(-) diff --git a/src/components/ai-edition/v4/EditorShellV4.module.css b/src/components/ai-edition/v4/EditorShellV4.module.css index bc4f3e7bb..581bb53b7 100644 --- a/src/components/ai-edition/v4/EditorShellV4.module.css +++ b/src/components/ai-edition/v4/EditorShellV4.module.css @@ -628,9 +628,18 @@ .inspectorWrap { position: absolute; top: 20px; - right: 20px; + /* Logical, so the inspector sits against the stage's END edge: the right in a + left-to-right UI, the left once the app sets dir="rtl" for ar. */ + inset-inline-end: 20px; bottom: 20px; display: flex; + /* The rail comes first in the DOM (see FloatingInspector) but lays out at the END + edge, with the panel opening towards the start. The wrap is anchored by that end + edge, so the rail keeps the same spot on screen whether the panel is open or not: + re-clicking the active facet closes the panel from where the pointer already is, + and selecting a timeline region does not make the rail jump by a panel's width. + `row-reverse` follows the writing direction, so this mirrors under RTL by itself. */ + flex-direction: row-reverse; align-items: flex-start; gap: 10px; z-index: 20; @@ -638,8 +647,11 @@ } /* Open, the rail and the panel are ONE object: same height, touching, sharing the edge between them. Closed, the rail is on its own again and goes back to being a floating - pill of its own height — stretching an empty rail to the full stage would be a tall - column of nothing over the video. Hence the attribute rather than a blanket rule. */ + pill of its own height; stretching an empty rail to the full stage would be a tall + column of nothing over the video. Hence the attribute rather than a blanket rule. + + The seam is on the rail's START side (the panel opens towards the start), written with + logical properties so it follows the mirroring above. */ .inspectorWrap[data-open] { align-items: stretch; gap: 0; @@ -647,14 +659,16 @@ .inspectorWrap[data-open] .facetRail { /* Buttons stay at the top of a rail that is now as tall as the panel. */ justify-content: flex-start; - border-top-right-radius: 0; - border-bottom-right-radius: 0; - /* The panel's own left border is the shared one; two of them would draw 2px. */ - border-right: 0; + border-start-start-radius: 0; + border-end-start-radius: 0; + /* The panel's own end border is the shared one; two of them would draw 2px. Dropping + it on the start side leaves the rail's end edge, and so its buttons, where they + were while it was closed. */ + border-inline-start: 0; } .inspectorWrap[data-open] .inspector { - border-top-left-radius: 0; - border-bottom-left-radius: 0; + border-start-end-radius: 0; + border-end-end-radius: 0; } .inspector { pointer-events: auto; diff --git a/src/components/ai-edition/v4/FloatingInspector.test.tsx b/src/components/ai-edition/v4/FloatingInspector.test.tsx index d21187686..1a3786022 100644 --- a/src/components/ai-edition/v4/FloatingInspector.test.tsx +++ b/src/components/ai-edition/v4/FloatingInspector.test.tsx @@ -1,5 +1,7 @@ // @vitest-environment jsdom import "@testing-library/jest-dom"; +import { readFileSync } from "node:fs"; +import path from "node:path"; import { fireEvent, render, screen } from "@testing-library/react"; import { describe, expect, it, vi } from "vitest"; @@ -123,6 +125,37 @@ describe("FloatingInspector", () => { expect(wrap()).toHaveAttribute("data-open", "true"); }); + // The rail stays where it is while the panel opens and closes, so the facet that is + // already active is still under the pointer and a second click on it closes the panel. + it("closes the panel when the active facet is clicked again", () => { + const onToggleOpen = vi.fn(); + const onFacetChange = vi.fn(); + render( + , + ); + fireEvent.click(screen.getByRole("button", { name: "settings.layout.title" })); + expect(onToggleOpen).toHaveBeenCalledTimes(1); + expect(onFacetChange).not.toHaveBeenCalled(); + }); + + // On screen the panel opens to the rail's start side, but in the DOM the rail comes + // first: press a facet, and the next Tab is in the controls it opened rather than + // behind the rest of the rail. + it("keeps the rail ahead of the panel in the DOM", () => { + render(); + const facetButton = screen.getByRole("button", { name: "settings.layout.title" }); + const pane = screen.getByTestId("layout-pane"); + expect( + facetButton.compareDocumentPosition(pane) & Node.DOCUMENT_POSITION_FOLLOWING, + ).toBeTruthy(); + }); + it("renders close button on AudioTrackPane when audio track is selected and deselects on click", () => { const clearSelection = vi.fn(); const tl = { @@ -137,3 +170,38 @@ describe("FloatingInspector", () => { expect(clearSelection).toHaveBeenCalledTimes(1); }); }); + +// Where the rail lands is CSS that jsdom does not lay out, and the failure it guards +// against is silent: anchor the wrap by the wrong edge, or lay it out in DOM order, and +// the rail jumps a panel's width every time the panel opens, so a second click on the +// active facet lands inside the panel instead of closing it. What can be pinned is the +// contract that keeps it still. +describe("inspector rail placement contract", () => { + const css = readFileSync(path.resolve(__dirname, "EditorShellV4.module.css"), "utf8"); + const block = (selector: string) => { + const start = css.indexOf(`${selector} {`); + expect(start, `${selector} rule`).toBeGreaterThanOrEqual(0); + return css.slice(start, css.indexOf("}", start)); + }; + // Comments mention properties by name; only declarations count. + const declarations = (selector: string) => block(selector).replace(/\/\*[\s\S]*?\*\//g, ""); + + it("anchors the wrap by its inline end and lays the rail out at that end", () => { + const wrap = declarations(".inspectorWrap"); + expect(wrap).toMatch(/inset-inline-end:\s*20px/); + expect(wrap).toMatch(/flex-direction:\s*row-reverse/); + // A physical anchor would stay on the right under dir="rtl". + expect(wrap).not.toMatch(/(^|\s)(right|left):/); + }); + + // The seam faces the panel, which is on the rail's start side in either direction. + it("joins the rail and the panel on the rail's start side, in logical terms", () => { + const rail = declarations(".inspectorWrap[data-open] .facetRail"); + expect(rail).toMatch(/border-inline-start:\s*0/); + expect(rail).toMatch(/border-start-start-radius:\s*0/); + expect(rail).toMatch(/border-end-start-radius:\s*0/); + const panel = declarations(".inspectorWrap[data-open] .inspector"); + expect(panel).toMatch(/border-start-end-radius:\s*0/); + expect(panel).toMatch(/border-end-end-radius:\s*0/); + }); +}); diff --git a/src/components/ai-edition/v4/FloatingInspector.tsx b/src/components/ai-edition/v4/FloatingInspector.tsx index 347fa32b1..4daf9df0b 100644 --- a/src/components/ai-edition/v4/FloatingInspector.tsx +++ b/src/components/ai-edition/v4/FloatingInspector.tsx @@ -121,10 +121,17 @@ export function FloatingInspector({ const effectiveOpen = open || selection !== null || audioTrackSelected; return (
- {/* The rail leads, the panel follows. It is the chooser and the panel is what it - chose, so meeting it first is the right reading order for a keyboard and a - screen reader alike -- which is why this is a DOM reorder and not - `row-reverse`, where the two would disagree. */} + {/* The rail leads in the DOM, the panel follows: the rail is the chooser and the + panel is what it chose, so a keyboard or screen-reader user presses a facet + and the next Tab lands in its controls, instead of having to back out through + the rail to reach them. + + On screen the rail sits at the END edge with the panel opening towards the + start (`row-reverse` on the wrap). That keeps the rail fixed while the panel + comes and goes, so re-clicking the active facet closes it from the same spot. + The visual order runs panel then rail, against the DOM, but the rail is a + vertical toolbar pinned to the stage edge rather than a line of text read + across, and chooser-then-content is the order that serves tabbing. */}
{FACETS.map(({ id, labelKey, icon: Icon }) => (