From 3b29865b9b6144a94cb67f6d498b6f310c1295af Mon Sep 17 00:00:00 2001 From: Timothy Date: Wed, 26 Aug 2026 18:25:21 +0200 Subject: [PATCH 1/7] =?UTF-8?q?feat(734):=20field-level=20progressive=20di?= =?UTF-8?q?sclosure=20=E2=80=94=20shared=20FieldHelp=20trigger=20+=20panel?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Settings fields carry consequences that are severe and non-obvious while the UI gives a bare label; the explanation existed only in decision records and source comments. Adds the three-level pattern from #734 as a shared component so it is identical everywhere rather than sprinkled per screen. - `FieldHelp` (web/src/components/fieldHelp.tsx): an Info-icon + {open && ( + + {detail} + {docsHref && ( + setPinned(false)} + > + {docsLabel} + + )} + + )} + + ); +} diff --git a/web/src/components/index.ts b/web/src/components/index.ts index ca8a6d0aa..fbf63f38b 100644 --- a/web/src/components/index.ts +++ b/web/src/components/index.ts @@ -3,6 +3,7 @@ import './components.css'; export * from './bugPreview'; export * from './dataDisplay'; export * from './feedback'; +export * from './fieldHelp'; export * from './forms'; export * from './navigation'; export * from './overlay'; diff --git a/web/src/screens/FFmpegProfilesScreen.tsx b/web/src/screens/FFmpegProfilesScreen.tsx index ba095bbcd..9752fa484 100644 --- a/web/src/screens/FFmpegProfilesScreen.tsx +++ b/web/src/screens/FFmpegProfilesScreen.tsx @@ -3,7 +3,18 @@ import { useCallback, useEffect, useRef, useState, type ReactNode } from 'react' import { ArrowLeft, Check, Copy, Plus, SlidersHorizontal, Trash2, TriangleAlert } from 'lucide-react'; import { navigateToPath } from '../routing'; import { usePrimaryAction } from '../primaryAction'; -import { Badge, Button, Card, Checkbox, ConfirmDialog, IconButton, Input, Select, Spinner } from '../components'; +import { + Badge, + Button, + Card, + Checkbox, + ConfirmDialog, + FieldHelp, + IconButton, + Input, + Select, + Spinner +} from '../components'; import { createFFmpegProfile, deleteFFmpegProfile, @@ -25,6 +36,31 @@ const BASE_PATH = '/app/ffmpeg-profiles'; // form must not offer one it would silently override (ersatztv#529) const MINIMUM_QSV_EXTRA_HARDWARE_FRAMES = 64; +// Level-2 explainer copy for the progressive-disclosure pattern (#734). Colocated with the field +// definitions it describes — see docs/spa-conventions.md §15. One short paragraph each; anything +// longer belongs behind a `docsHref` once the external docs exist. Level 1 (the one-sentence +// summary) stays on the prop at the call site. +const FIELD_HELP: Record = { + threadCount: + 'Caps the worker threads FFmpeg uses per transcode. 0 lets FFmpeg decide, which is usually right; a low fixed value keeps one channel from starving the others on a busy host, at the cost of falling behind realtime on heavy content.', + scalingBehavior: + 'Decides what happens when the source aspect ratio does not match the preferred resolution: scale and pad to fit, or crop the overflow. Padding never loses picture; cropping never shows bars.', + videoBitrate: + 'Target output bitrate. Too low and the encoder throws away detail on motion; too high and clients on slow links buffer. Buffer size is the companion setting — it bounds how far the encoder may deviate from this target.', + videoBufferSize: + 'How much bitrate deviation the encoder may bank before it has to correct. Roughly 2x the bitrate is the usual starting point. Very small values force a near-constant bitrate and hurt quality on scene changes.', + hardwareAcceleration: + 'Offloads decode and encode to the GPU. Whether an option actually works depends on the container having the device passed through and the FFmpeg build supporting it — a mismatch fails at playback time, not when you save.', + qsvExtraHardwareFrames: + `Extra surfaces in the QSV upload pool. Below ${MINIMUM_QSV_EXTRA_HARDWARE_FRAMES} the pool runs out of headroom on unthrottled reads and the channel serves nothing at all, so the server floors any smaller value rather than honouring it.`, + qsvPreferNativeDecoder: + 'Splits the pipeline: VA-API decodes, QSV encodes. The VA-API decoder tolerates imperfect streams that the QSV decoder rejects outright, and it is required for Dolby Vision, so this is the recommended setting on Intel.', + normalizeLoudnessMode: + 'Levels volume across content from different sources. Dynamic normalization adapts as it plays and needs no analysis pass; loudnorm targets a fixed loudness and is more predictable but reacts poorly to content whose levels swing wildly.', + targetLoudness: + 'The integrated loudness loudnorm aims for, in LUFS. Broadcast practice sits near -23; streaming services sit nearer -16. Louder targets leave less headroom and are more likely to clip peaks.' +}; + // `Complete<…>` so the draft must name every request member: the edit path PUTs this whole // object to a full-replace endpoint, where an unset member is written as its default rather // than left alone. `qsvPreferNativeDecoder` is optional in the schema and both draft builders @@ -287,20 +323,29 @@ function draftFromProfile(profile: FFmpegProfile): Draft { function Row({ children, control = 320, + detail, + docsHref, first = false, help, label }: { children: ReactNode; control?: number; + /** Level-2 explainer paragraph; renders the shared FieldHelp trigger beside the label (#734). */ + detail?: string; + docsHref?: string; first?: boolean; + /** Level-1 one-sentence summary. */ help?: string; label: string; }) { return (
-
{label}
+
+ {label} + {detail && } +
{help &&
{help}
}
{children}
@@ -676,7 +721,7 @@ function ProfileEditor({ mode }: { mode: { kind: 'add'; from: null | number } | value={draft.name ?? ''} /> - + set({ threadCount })} value={draft.threadCount} /> @@ -694,7 +739,7 @@ function ProfileEditor({ mode }: { mode: { kind: 'add'; from: null | number } | {draft.normalizeVideo && ( <> - + set({ hardwareAcceleration: event.target.value as HardwareAccelerationKind })} options={hwaccelKinds} @@ -790,7 +835,12 @@ function ProfileEditor({ mode }: { mode: { kind: 'add'; from: null | number } | )} {draft.hardwareAcceleration === 'Qsv' ? ( - + {/* below this the QSV upload pool has too little headroom and transcoding fails on any unthrottled read; the server floors it anyway (ersatztv#529) */} set({ audioSampleRate })} unit="kHz" value={draft.audioSampleRate} /> - + - + @@ -191,7 +257,7 @@ {normalizeVideo && ( - + - + - + - + - + ` instead of free-text Chronic natural-language parsing. | 2026-07-09 | [link](records/spa/datetime-local-input.md) | | `spa.deco-templates-table` | The deco-templates editor also renders its day/deco assignment as a table, extending (not replacing) the templates-editor-table convention. | 2026-07-09 | [link](records/spa/deco-templates-table.md) | | `spa.download-sample-gate` | The SPA disables both Download Media Sample and Download Results while a troubleshooting session is starting/running (Blazor only gated Download Results). | 2026-07-09 | [link](records/spa/download-sample-gate.md) | +| `spa.field-progressive-disclosure` | A consequential settings field explains itself through one shared `FieldHelp` icon trigger beside the field name — never the label itself, never a widened `Tooltip` — with the paragraph declared `as const` in the screen's own `FIELD_HELP` record and the panel portalled to `document.body`. | 2026-08-26 | [link](records/spa/field-progressive-disclosure.md) | | `spa.legacy-redirect-matcher` | `LegacyUiRedirects.TryGetRedirect` is a two-tier matcher — an exact `OrdinalIgnoreCase` `Map` (Tier 1) then an ordered segment-template pattern list (Tier 2, first-match-wins) — collision-free by construction, with a guard invariant that no rule may prefix-match `/api`, `/artwork`, `/docs`, `/openapi`, `/iptv`, `/app`, or `/media/sources`. | 2026-07-11 | [link](records/spa/legacy-redirect-matcher.md) | | `spa.library-pickers-resolve-by-search` | A picker over a media-library table (Episode/Song/Image/Movie/MusicVideo/TelevisionShow/TelevisionSeason/Artist/OtherVideo/RemoteStream) resolves its options by SEARCH — a debounced `SearchPicker` calling `searchLibraryPickerOptions`, which issues at most ONE `getLibraryBrowseItems` request per settled query, bounded to `LIBRARY_PICKER_RESULTS` (25) rows — CLAMPED inside the helper, not merely defaulted — and gated on `LIBRARY_PICKER_MIN_QUERY` (2) characters. It list-loads NOTHING on mount or on a type switch, so there is no truncation to surface and no truncation hint. The typed text is COMPILED (`titleContainsQuery` → `title:**`), never forwarded raw. The current selection renders from the OWNING RECORD, not from the result set (`selectedName` on a rerun collection / playlist item; a single by-id detail read — `getShow`/`getSeason`/`getArtist` — for a filler preset, which stores only the id), and an edit draft is INITIALIZED ONCE from the detail read — never seeded from the list row, never reconciled against a late response — with the form withheld until it lands, the editor failing CLOSED when the response carries no USABLE concurrency token — absent, empty and whitespace-only ETags are ONE case, normalized in one place, so a PUT without `If-Match` is unreachable, and a deadline plus a route back so a hung request cannot strand it. An id NEVER travels without its namespace: search results are cached against `(source, query)` and list-backed options carry the type they were loaded for, so no id from one type can be offered under another; and every id entering editor state — search result, list-backed option, or a selection restored from a detail read — passes ONE shared `isSelectionId` (int32) predicate at that boundary, an unbindable id being treated as ABSENT rather than coerced. Conflicts are detected at SAVE time via `If-Match` -> 412 -> Reload, and Reload simply drops the draft back to null and re-runs the same initialize-once load, so the form is unmounted while the replacement is in flight; an asynchronously-resolved name is keyed to the id it was resolved for and never overwrites a label naming a different id. The typeahead implements the full ARIA combobox keyboard contract, because it replaces a natively keyboard-operable `); + const trigger = renderHelp(); + const wrap = trigger.parentElement as HTMLElement; + const elsewhere = screen.getByTestId('elsewhere'); + elsewhere.focus(); + + fireEvent.mouseEnter(wrap); + expect(screen.getByRole('note')).toBeInTheDocument(); + + fireEvent.keyDown(document, { key: 'Escape' }); + + expect(screen.getByRole('note')).toBeInTheDocument(); + expect(document.activeElement).toBe(elsewhere); + }); + it('closes a pinned panel on Escape and returns focus to the trigger', () => { const trigger = renderHelp(); diff --git a/web/src/components/fieldHelp.tsx b/web/src/components/fieldHelp.tsx index 2664bda45..25f35f828 100644 --- a/web/src/components/fieldHelp.tsx +++ b/web/src/components/fieldHelp.tsx @@ -1,4 +1,13 @@ -import { useCallback, useEffect, useId, useRef, useState, type ReactNode } from 'react'; +import { + useCallback, + useEffect, + useId, + useLayoutEffect, + useRef, + useState, + type ReactNode +} from 'react'; +import { createPortal } from 'react-dom'; import { ExternalLink, Info } from 'lucide-react'; /** @@ -14,6 +23,12 @@ import { ExternalLink, Info } from 'lucide-react'; * `pointer-events: none`, opens on hover/focus only, and cannot contain a link. All four are * wrong for a paragraph with a click-through, and touch users never get it at all. The two * coexist deliberately; do not widen `Tooltip` into this role. + * + * Why the panel is PORTALLED to `document.body` rather than positioned inside the wrapper: + * `.ctv-card` sets `overflow: hidden`, which clips a positioned descendant no matter its + * `z-index`. An in-flow panel on the last row of a card was measured showing 12px of a 92px + * paragraph. A portal escapes every ancestor's clip, so the panel's readability stops depending + * on which row of which card adopted it. */ export interface FieldHelpProps { /** The field name this explains. Used to build the trigger's accessible name. */ @@ -23,56 +38,119 @@ export interface FieldHelpProps { /** Optional level-3 deep link. Renders a "Learn more" link inside the panel. */ docsHref?: string; docsLabel?: string; - placement?: 'bottom' | 'top'; } -export function FieldHelp({ - label, - detail, - docsHref, - docsLabel = 'Learn more', - placement = 'bottom' -}: FieldHelpProps) { - const panelId = useId(); - const wrapRef = useRef(null); - const triggerRef = useRef(null); +/** + * Offset between trigger and panel. `.ctv-field-help-panel::before` spans it with a transparent + * bridge (9px, deliberately taller); keep the two in step — see spa-conventions.md §15. + */ +const GAP = 7; +/** Mirrors `max-width` on `.ctv-field-help-panel`; used only to keep the panel inside the viewport. */ +const PANEL_MAX_WIDTH = 320; +const EDGE = 8; - // Three independent reasons the panel can be showing. `pinned` is the tap/click state and is - // the only one a touch user can produce, so it must not be derived from hover or focus. +interface PanelPosition { + left: number; + top?: number; + bottom?: number; + placement: 'top' | 'bottom'; +} + +export function FieldHelp({ label, detail, docsHref, docsLabel = 'Learn more' }: FieldHelpProps) { + const panelId = useId(); + const triggerRef = useRef(null); + const panelRef = useRef(null); + + // Four independent reasons the panel can be showing, none derived from another. `pinned` is the + // tap/click state and is the only one a touch user can produce. Trigger-hover and panel-hover are + // separate because the panel is portalled: it is no longer a DOM descendant of the wrapper, so + // the wrapper's mouseleave cannot stand in for "the pointer is still somewhere in this widget". const [pinned, setPinned] = useState(false); + const [hoveredTrigger, setHoveredTrigger] = useState(false); + const [hoveredPanel, setHoveredPanel] = useState(false); + const [focused, setFocused] = useState(false); + const open = pinned || hoveredTrigger || hoveredPanel || focused; + // Set immediately before the programmatic re-focus that follows a dismiss, so the trigger's own // focus handler does not read that focus as "the user tabbed here" and reopen what was just closed. const suppressFocusOpen = useRef(false); - const [hovered, setHovered] = useState(false); - const [focused, setFocused] = useState(false); - const open = pinned || hovered || focused; + + const [position, setPosition] = useState(null); const dismiss = useCallback(() => { setPinned(false); - setHovered(false); + setHoveredTrigger(false); + setHoveredPanel(false); setFocused(false); }, []); - // Escape and outside-pointer close an open panel. Both are registered on the document because - // an open panel can outlive focus (tap on touch, click-then-move-mouse-away on desktop), so a - // handler bound to the wrapper would never see the event. - useEffect(() => { + // Position the portalled panel against the trigger's viewport rect, flipping above when the panel + // would run off the bottom and there is room above. Runs in a layout effect so the flip lands + // before paint; re-runs on scroll/resize because a fixed panel does not travel with the page. + useLayoutEffect(() => { + // No reset on close: the panel is unmounted, and because this is a LAYOUT effect the next open + // recomputes before paint, so a stale position from the previous open is never painted. if (!open) { return; } + const update = () => { + const trigger = triggerRef.current; + if (!trigger) { + return; + } + const rect = trigger.getBoundingClientRect(); + const height = panelRef.current?.offsetHeight ?? 0; + const spaceBelow = window.innerHeight - rect.bottom - GAP; + const spaceAbove = rect.top - GAP; + const flip = height > 0 && spaceBelow < height && spaceAbove > spaceBelow; + const left = Math.max( + EDGE, + Math.min(rect.left, window.innerWidth - PANEL_MAX_WIDTH - EDGE) + ); + + setPosition( + flip + ? { left, bottom: window.innerHeight - rect.top + GAP, placement: 'top' } + : { left, top: rect.bottom + GAP, placement: 'bottom' } + ); + }; + + update(); + window.addEventListener('scroll', update, true); + window.addEventListener('resize', update); + return () => { + window.removeEventListener('scroll', update, true); + window.removeEventListener('resize', update); + }; + // `detail` participates because changing the copy changes the panel height, and therefore the flip. + }, [detail, docsHref, open]); + + // Escape and outside-pointer dismissal. Deliberately NOT armed for a hover-only panel: a + // document-level Escape handler that fires whenever a pointer happens to rest on an info icon + // would steal focus from whatever the user was actually trying to dismiss. + const dismissible = pinned || focused; + useEffect(() => { + if (!dismissible) { + return; + } + const onKeyDown = (event: KeyboardEvent) => { - if (event.key === 'Escape') { - dismiss(); - if (document.activeElement !== triggerRef.current) { - suppressFocusOpen.current = true; - triggerRef.current?.focus(); - } + if (event.key !== 'Escape') { + return; + } + dismiss(); + if (document.activeElement !== triggerRef.current) { + suppressFocusOpen.current = true; + triggerRef.current?.focus(); } }; const onPointerDown = (event: Event) => { const target = event.target; - if (target instanceof Node && wrapRef.current?.contains(target)) { + if ( + target instanceof Node && + (triggerRef.current?.contains(target) || panelRef.current?.contains(target)) + ) { return; } dismiss(); @@ -84,14 +162,46 @@ export function FieldHelp({ document.removeEventListener('keydown', onKeyDown); document.removeEventListener('pointerdown', onPointerDown); }; - }, [dismiss, open]); + }, [dismiss, dismissible]); + + const panel = open ? ( + setHoveredPanel(true)} + onMouseLeave={() => setHoveredPanel(false)} + > + {detail} + {docsHref && ( + setPinned(false)} + > + {docsLabel} + + )} + + ) : null; return ( setHovered(true)} - onMouseLeave={() => setHovered(false)} + onMouseEnter={() => setHoveredTrigger(true)} + onMouseLeave={() => setHoveredTrigger(false)} > - {open && ( - - {detail} - {docsHref && ( - setPinned(false)} - > - {docsLabel} - - )} - - )} + {panel && createPortal(panel, document.body)} ); } diff --git a/web/src/screens/FFmpegProfilesScreen.tsx b/web/src/screens/FFmpegProfilesScreen.tsx index 9752fa484..ef48f0aab 100644 --- a/web/src/screens/FFmpegProfilesScreen.tsx +++ b/web/src/screens/FFmpegProfilesScreen.tsx @@ -40,11 +40,11 @@ const MINIMUM_QSV_EXTRA_HARDWARE_FRAMES = 64; // definitions it describes — see docs/spa-conventions.md §15. One short paragraph each; anything // longer belongs behind a `docsHref` once the external docs exist. Level 1 (the one-sentence // summary) stays on the prop at the call site. -const FIELD_HELP: Record = { +const FIELD_HELP = { threadCount: 'Caps the worker threads FFmpeg uses per transcode. 0 lets FFmpeg decide, which is usually right; a low fixed value keeps one channel from starving the others on a busy host, at the cost of falling behind realtime on heavy content.', scalingBehavior: - 'Decides what happens when the source aspect ratio does not match the preferred resolution: scale and pad to fit, or crop the overflow. Padding never loses picture; cropping never shows bars.', + 'Decides what happens when the source aspect ratio does not match the preferred resolution. Scale and Pad keeps the whole picture and adds bars; Crop fills the frame and cuts whatever overflows; Stretch fills it by distorting the image, which is why it is rarely what you want.', videoBitrate: 'Target output bitrate. Too low and the encoder throws away detail on motion; too high and clients on slow links buffer. Buffer size is the companion setting — it bounds how far the encoder may deviate from this target.', videoBufferSize: @@ -52,14 +52,16 @@ const FIELD_HELP: Record = { hardwareAcceleration: 'Offloads decode and encode to the GPU. Whether an option actually works depends on the container having the device passed through and the FFmpeg build supporting it — a mismatch fails at playback time, not when you save.', qsvExtraHardwareFrames: - `Extra surfaces in the QSV upload pool. Below ${MINIMUM_QSV_EXTRA_HARDWARE_FRAMES} the pool runs out of headroom on unthrottled reads and the channel serves nothing at all, so the server floors any smaller value rather than honouring it.`, + `Extra surfaces in the QSV upload pool. A pool of 0 is measured to fail outright — the channel serves nothing at all — while values between 1 and ${MINIMUM_QSV_EXTRA_HARDWARE_FRAMES - 1} are untested rather than known-bad. The server raises anything smaller to ${MINIMUM_QSV_EXTRA_HARDWARE_FRAMES} and logs the override rather than trusting it.`, qsvPreferNativeDecoder: 'Splits the pipeline: VA-API decodes, QSV encodes. The VA-API decoder tolerates imperfect streams that the QSV decoder rejects outright, and it is required for Dolby Vision, so this is the recommended setting on Intel.', normalizeLoudnessMode: - 'Levels volume across content from different sources. Dynamic normalization adapts as it plays and needs no analysis pass; loudnorm targets a fixed loudness and is more predictable but reacts poorly to content whose levels swing wildly.', + 'Levels volume across content from different sources. Off leaves each item at its own level, so volume jumps between them; loudnorm retargets everything to the fixed integrated loudness below, which evens that out at the cost of an extra filter in the graph and a less faithful dynamic range.', targetLoudness: 'The integrated loudness loudnorm aims for, in LUFS. Broadcast practice sits near -23; streaming services sit nearer -16. Louder targets leave less headroom and are more likely to clip peaks.' -}; + // `as const` rather than `Record`: under an index signature `FIELD_HELP.typo` + // types as `string`, so a mistyped key renders a trigger with an empty panel and no build error. +} as const; // `Complete<…>` so the draft must name every request member: the edit path PUTs this whole // object to a full-replace endpoint, where an unset member is written as its default rather -- 2.47.3 From 14d1fbaabd2cc110782ac60879fac11f8247430c Mon Sep 17 00:00:00 2001 From: Timothy Date: Wed, 26 Aug 2026 19:14:57 +0200 Subject: [PATCH 6/7] fix(734): withdraw the hover bridge and say so; drop a fourth wrong copy claim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second cold review: BLOCKED on N1 and N4, plus three regressions the portal round introduced. N1 — the bridge only ever worked on a strictly vertical descent. The reviewer measured a diagonal walk toward the panel (down-and-right, the natural reach for a 320px panel sitting below-right) closing at step 4 of 12 — the same signature the bridge commit claimed to have fixed. A boundary scan puts the safe sideways exit at the bottom 1.25px of an 18px icon, about 7% of its height. The bridge is WITHDRAWN, not patched. Two rounds of live measurement of this component's hover model were both taken along the vertical axis — the one where it works — so a third patch would have been guessing at an axis again. §15 and the decision record now say what is true: hover is for reading the paragraph in place, and the panel's interactive content is reached by PINNING, which survives any pointer motion and is the only gesture a touch user has regardless. A mechanism needing a 1.25px caveat is not a mechanism. N4 — a fourth wrong copy claim survived the round that fixed the first three: "the server raises anything smaller to 64 and logs the override". Neither CreateFFmpegProfileHandler nor UpdateFFmpegProfileHandler has an ILogger; the Math.Max is silent. The warning lives in QsvPipelineBuilder at pipeline-build time, and since the save path normalises the value first, the log the copy promised cannot be produced by anything a user does on that screen. Non-blocking regressions from the portal round, also fixed: - N3: the panel left every dialog's stacking context on being portalled and painted BEHIND `.ctv-dialog-overlay` / `.ctv-slideover-scrim` (fixed, z-index 100). Raised to 110 and measured against an injected scrim: hit-test now returns the panel. - N6: the horizontal clamp used the 320px max-width constant instead of the panel's measured width, which would shove a narrower panel left of its own trigger near the right edge. - N7: the loudness paragraph pointed at "the fixed integrated loudness below", but the Target loudness row only renders once loudnorm is already selected — so in the state where the reader is deciding, there is nothing below. - N5: the hover test fired mouseEnter(panel) BEFORE mouseLeave(wrap), the reverse of the browser's own order, so it could only confirm its own arrangement. Replaced with two tests that assert what is actually claimed: a panel already entered stays open, and a pinned panel's link survives arbitrary pointer motion. N2 (the portal puts a panel-internal link at the end of the document tab order) is recorded in §15 and the record as a known limitation rather than fixed — no screen passes `docsHref`, and inventing custom Tab forwarding at round three is how the bridge happened. Re-measured live at this head: bridge absent from the shipped stylesheet, all 7 triggers fully visible at 320px, three auto-flipped to `top`, none out of viewport. Gate green: lint clean, build clean, 1220/1220, decisions-validate OK. refs #734 --- .../chicorytv-admin/FFmpegProfiles.jsx | 2 +- .../spa/field-progressive-disclosure.md | 27 ++++++++++++------ docs/spa-conventions.md | 26 ++++++++++++----- web/src/components/components.css | 25 +++-------------- web/src/components/fieldHelp.test.tsx | 28 +++++++++++++------ web/src/components/fieldHelp.tsx | 21 +++++++------- web/src/screens/FFmpegProfilesScreen.tsx | 4 +-- 7 files changed, 75 insertions(+), 58 deletions(-) diff --git a/design-system/templates/chicorytv-admin/FFmpegProfiles.jsx b/design-system/templates/chicorytv-admin/FFmpegProfiles.jsx index cb37af6b3..7ecf1e757 100644 --- a/design-system/templates/chicorytv-admin/FFmpegProfiles.jsx +++ b/design-system/templates/chicorytv-admin/FFmpegProfiles.jsx @@ -71,7 +71,7 @@ hardwareAcceleration: "Offloads decode and encode to the GPU. Whether an option actually works depends on the container having the device passed through and the FFmpeg build supporting it — a mismatch fails at playback time, not when you save.", normalizeLoudnessMode: - "Levels volume across content from different sources. Off leaves each item at its own level, so volume jumps between them; loudnorm retargets everything to the fixed integrated loudness below, which evens that out at the cost of an extra filter in the graph and a less faithful dynamic range.", + "Levels volume across content from different sources. Off leaves each item at its own level, so volume jumps between them; loudnorm retargets everything to one integrated loudness, which evens that out at the cost of an extra filter in the graph and a less faithful dynamic range.", }; // Mockup of the shared `FieldHelp` primitive (web/src/components/fieldHelp.tsx). The prototype diff --git a/docs/decisions/records/spa/field-progressive-disclosure.md b/docs/decisions/records/spa/field-progressive-disclosure.md index 6f9735a3a..8e9429de8 100644 --- a/docs/decisions/records/spa/field-progressive-disclosure.md +++ b/docs/decisions/records/spa/field-progressive-disclosure.md @@ -30,8 +30,12 @@ severe and non-obvious rather than backfilled across every screen. a card's last row was measured showing 12px of a 92px paragraph, and the Audio card's final row always carries a `detail`, so one field's explainer was destroyed in every state of that card. A layout effect positions it from the trigger's viewport rect, flips it above when it would run off - the bottom, and clamps it inside the horizontal edges. Rejected: raising `z-index` (irrelevant to a + the bottom, and clamps it inside its measured width. Rejected: raising `z-index` (irrelevant to a clip) and relaxing `.ctv-card`'s overflow (repo-wide blast radius for one component's benefit). + The portal costs two things, both accepted knowingly: the panel leaves any dialog's stacking + context, so it needs `z-index: 110` to clear the `100` scrims, and a focusable element inside it + lands at the end of the document tab order rather than beside its trigger (latent — no screen + passes `docsHref` yet). - **Copy lives in the screen's own `FIELD_HELP`, declared `as const`.** Not in the shared component (it would collect copy for screens it knows nothing about) and not from the API (copy shipping separately from the field it describes drifts). `as const` rather than `Record`, @@ -42,11 +46,14 @@ severe and non-obvious rather than backfilled across every screen. stated. It can hold neither a paragraph nor a link, and touch users never see it. **Do not widen `Tooltip` into this role**, and do not reach for `FieldHelp` where a two-word label would do. - **Three independent opening signals, none derived from another**: tap (pins — the only gesture a - touch user has), hover, keyboard focus. The trigger wrapper and the panel each own their hover - state, because the portal means the wrapper's `mouseleave` no longer stands in for "the pointer is - still in this widget". A transparent `::before` bridge spans the 7px offset where the pointer is - over neither element; without it the panel closes under the cursor — measured as disappearing four - steps into a twelve-step walk, which makes a `docsHref` link unreachable by mouse. + touch user has), hover, keyboard focus. **Hover is for reading in place; interacting with the + panel means pinning it first.** A transparent `::before` bridge across the 7px offset was tried + and withdrawn: it held for a strictly vertical descent onto the panel and failed for a diagonal + one, leaving a safe sideways exit of 1.25px on an 18px icon. Withdrawn rather than patched a third + time — the honest contract is cheaper to keep true than a mechanism whose correctness depends on + the axis the last person happened to measure along. Every live measurement of this component's + hover model, across two review rounds, had been taken along the vertical axis, the one where it + worked. - **`role="note"` does not announce anything.** It is not a live region; `aria-describedby` on the trigger, set only while the panel is open, is what reads the paragraph out. `aria-controls` and `aria-describedby` are both omitted while collapsed — a reference to an absent id is worse than @@ -57,5 +64,9 @@ severe and non-obvious rather than backfilled across every screen. cut compared loudnorm to a dynamic-normalization mode this fork does not have (`NormalizeLoudnessMode` is `{ Off, LoudNorm }`), described scaling as binary while omitting `Stretch`, and called `1..63` known-bad where `ffmpeg.qsv-extra-hw-frames-floor` says explicitly - that only `0` and `64` were measured. Copy describing options the user cannot choose is the exact - drift this convention exists to prevent. + that only `0` and `64` were measured. A fourth survived the round that fixed the first three: the + copy promised the server "logs the override", but neither save handler has an `ILogger` — that + warning lives in `QsvPipelineBuilder`, and because the save path normalises the value first, the + log it promised cannot be produced by anything a user does on that screen. Copy describing options + the user cannot choose, or behaviour the code does not have, is the exact drift this convention + exists to prevent — and one pass over it is demonstrably not enough. diff --git a/docs/spa-conventions.md b/docs/spa-conventions.md index 9db253a85..544af1db6 100644 --- a/docs/spa-conventions.md +++ b/docs/spa-conventions.md @@ -1013,6 +1013,12 @@ explainer was destroyed in every state of that card. `FieldHelp` therefore rende rect in a layout effect that also flips it above when it would run off the bottom and clamps it inside the horizontal edges. Readability then stops depending on which row of which card adopted it. +The portal has two consequences worth knowing before you adopt it on a new screen. The panel no +longer sits inside a dialog's or slide-over's stacking context, so it carries `z-index: 110` to +out-rank `.ctv-dialog-overlay` / `.ctv-slideover-scrim` (both fixed at 100) — at the original 60 it +painted *behind* the very surface whose field it was explaining. And a focusable element inside the +panel lands at the end of the document's tab order rather than next to its trigger. + ### Accessibility contract (not optional) Hover-only content is invisible to keyboard and touch users, so `FieldHelp` opens on **three @@ -1020,13 +1026,16 @@ independent signals**, none derived from another: - **click/tap** — pins the panel open; the only gesture a touch user has. It survives the pointer leaving, and toggles closed on a second press. -- **hover** — the trigger wrapper and the panel each own their hover state, because the portal means - the wrapper's `mouseleave` can no longer stand in for "the pointer is still somewhere in this - widget". Unlike `.ctv-tooltip` the panel is interactive: it is not `pointer-events: none`, so a - `docsHref` link is clickable. The panel's transparent `::before` bridge is load-bearing, not - decoration: across the 7px offset the pointer is over neither element, and without the bridge the - panel closes under the cursor — measured as disappearing four steps into a twelve-step walk. Do - not delete it, and re-measure the walk if you change the offset. +- **hover** — for **reading the paragraph in place**. Unlike `.ctv-tooltip` the panel is not + `pointer-events: none`, so a pointer that reaches it keeps it open; but **hover does not reliably + get you there.** Across the 7px offset the pointer is over neither element and the panel closes + under the cursor. A transparent `::before` bridge was tried and **withdrawn**: it held for a + strictly vertical descent and failed for a diagonal one, the natural reach toward a panel sitting + below and to the right — measured, the safe sideways exit was the bottom **1.25px of an 18px + icon**. Do not re-add it; a mechanism that needs a 1.25px caveat is not a mechanism. + **Interacting with the panel — which today means a `docsHref` link — is done by pinning it first** + (click/tap). That is the documented route, it survives any pointer motion, and it is the only one + a touch user has anyway. - **keyboard focus** — opens on focus, closes on blur. Plus: `aria-expanded` on the trigger, an accessible name of `More about `, **Escape** closes @@ -1063,6 +1072,9 @@ do. this convention exists to prevent. 3. Render `` inside the row's label element. 4. Reuse the `ctv-field-help*` classes in `components.css`; do not restyle per screen. +5. If the field needs a `docsHref`, remember the link is reached by pinning, and that the portal puts + it at the END of the document's tab order rather than immediately after the trigger — a known + limitation, tracked rather than fixed because no screen passes `docsHref` yet. Backfilling every screen is deliberately **not** this convention's job — adopt it where a field's consequences are severe and non-obvious, which is what makes the icon meaningful rather than diff --git a/web/src/components/components.css b/web/src/components/components.css index 24c18a9db..913b2fb79 100644 --- a/web/src/components/components.css +++ b/web/src/components/components.css @@ -1239,7 +1239,10 @@ not "simplify" this back into the wrapper. */ .ctv-field-help-panel { position: fixed; - z-index: 60; + /* Above `.ctv-dialog-overlay` / `.ctv-slideover-scrim` (both fixed at 100). Being portalled to + body means this panel no longer sits inside a dialog's stacking context, so it must out-rank + the scrim explicitly or it paints behind the surface whose field it is explaining. */ + z-index: 110; display: flex; flex-direction: column; gap: 6px; @@ -1260,26 +1263,6 @@ text-align: left; } -/* The panel sits 7px clear of the trigger. A pointer walking down into it crosses that gap, where - it is over neither element — measured: the panel disappeared four steps into a twelve-step walk, - which makes a `docsHref` link unreachable by mouse. This transparent bridge extends the panel's - own hover area across the gap, so the pointer enters the panel in the same event batch that it - leaves the trigger. Keep it taller than the gap, and re-measure the walk if you change either. */ -.ctv-field-help-panel::before { - content: ''; - position: absolute; - left: 0; - right: 0; - height: 9px; -} - -.ctv-field-help-panel-bottom::before { - top: -9px; -} - -.ctv-field-help-panel-top::before { - bottom: -9px; -} .ctv-field-help-detail { display: block; diff --git a/web/src/components/fieldHelp.test.tsx b/web/src/components/fieldHelp.test.tsx index eedfdb8e4..b746a18e5 100644 --- a/web/src/components/fieldHelp.test.tsx +++ b/web/src/components/fieldHelp.test.tsx @@ -105,25 +105,37 @@ describe('FieldHelp', () => { expect(screen.getByRole('note')).toHaveTextContent(DETAIL); }); - // The portal means the wrapper's mouseleave can no longer stand in for "the pointer is still - // somewhere in this widget" — the panel must own its own hover state or a docsHref link is - // unreachable by mouse. - it('stays open when the pointer moves from the trigger onto the panel, and closes after both', () => { + // NOT asserted here: that the pointer can travel from the trigger onto the panel on hover alone. + // A `::before` bridge across the 7px gap was tried and withdrawn — it held for a strictly vertical + // descent and failed for a diagonal one (the safe sideways corridor measured 1.25px of an 18px + // icon), so the panel's interactive content is reached by PINNING, which is what §15 now says. A + // jsdom test of that transit would also have to fire mouseEnter(panel) before mouseLeave(wrap) — + // the reverse of the browser's own order — and so could only ever confirm its own arrangement. + it('keeps a panel open while the pointer is on the panel itself, and closes when it leaves', () => { const trigger = renderHelp({ docsHref: 'https://example.invalid/docs' }); const wrap = trigger.parentElement as HTMLElement; fireEvent.mouseEnter(wrap); const panel = screen.getByRole('note'); - fireEvent.mouseEnter(panel); - fireEvent.mouseLeave(wrap); - expect(screen.getByRole('note')).toBeInTheDocument(); - expect(screen.getByRole('link', { name: /Learn more/ })).toBeInTheDocument(); fireEvent.mouseLeave(panel); expect(screen.queryByRole('note')).toBeNull(); }); + // Pinning is the documented route to the level-3 link: it survives any pointer motion, and it is + // the only gesture a touch user has. + it('keeps the docs link reachable once the panel is pinned, through arbitrary pointer motion', () => { + const trigger = renderHelp({ docsHref: 'https://example.invalid/docs' }); + const wrap = trigger.parentElement as HTMLElement; + + fireEvent.click(trigger); + fireEvent.mouseEnter(wrap); + fireEvent.mouseLeave(wrap); + + expect(screen.getByRole('link', { name: /Learn more/ })).toBeInTheDocument(); + }); + // A document-level Escape handler armed by hover alone would steal focus from whatever the user // was actually dismissing, whenever a pointer happened to be resting on an info icon. it('leaves Escape alone while the panel is open on hover only', () => { diff --git a/web/src/components/fieldHelp.tsx b/web/src/components/fieldHelp.tsx index 25f35f828..6f170ba20 100644 --- a/web/src/components/fieldHelp.tsx +++ b/web/src/components/fieldHelp.tsx @@ -40,10 +40,7 @@ export interface FieldHelpProps { docsLabel?: string; } -/** - * Offset between trigger and panel. `.ctv-field-help-panel::before` spans it with a transparent - * bridge (9px, deliberately taller); keep the two in step — see spa-conventions.md §15. - */ +/** Offset between trigger and panel. */ const GAP = 7; /** Mirrors `max-width` on `.ctv-field-help-panel`; used only to keep the panel inside the viewport. */ const PANEL_MAX_WIDTH = 320; @@ -62,9 +59,11 @@ export function FieldHelp({ label, detail, docsHref, docsLabel = 'Learn more' }: const panelRef = useRef(null); // Four independent reasons the panel can be showing, none derived from another. `pinned` is the - // tap/click state and is the only one a touch user can produce. Trigger-hover and panel-hover are - // separate because the panel is portalled: it is no longer a DOM descendant of the wrapper, so - // the wrapper's mouseleave cannot stand in for "the pointer is still somewhere in this widget". + // tap/click state, the only one a touch user can produce, and the ONLY one under which the + // panel's own interactive content (a `docsHref` link) is meant to be reachable — see the hover + // note in spa-conventions.md §15. Trigger-hover and panel-hover are tracked separately because + // the portal means the wrapper's mouseleave cannot stand in for "the pointer is still in this + // widget"; panel-hover keeps an already-entered panel open, it does not make the gap crossable. const [pinned, setPinned] = useState(false); const [hoveredTrigger, setHoveredTrigger] = useState(false); const [hoveredPanel, setHoveredPanel] = useState(false); @@ -104,10 +103,10 @@ export function FieldHelp({ label, detail, docsHref, docsLabel = 'Learn more' }: const spaceBelow = window.innerHeight - rect.bottom - GAP; const spaceAbove = rect.top - GAP; const flip = height > 0 && spaceBelow < height && spaceAbove > spaceBelow; - const left = Math.max( - EDGE, - Math.min(rect.left, window.innerWidth - PANEL_MAX_WIDTH - EDGE) - ); + // Clamp against the panel's MEASURED width once it exists: using the max-width constant for a + // panel narrower than it would shove the panel left of its own trigger near the right edge. + const width = panelRef.current?.offsetWidth || PANEL_MAX_WIDTH; + const left = Math.max(EDGE, Math.min(rect.left, window.innerWidth - width - EDGE)); setPosition( flip diff --git a/web/src/screens/FFmpegProfilesScreen.tsx b/web/src/screens/FFmpegProfilesScreen.tsx index ef48f0aab..aaf69a34e 100644 --- a/web/src/screens/FFmpegProfilesScreen.tsx +++ b/web/src/screens/FFmpegProfilesScreen.tsx @@ -52,11 +52,11 @@ const FIELD_HELP = { hardwareAcceleration: 'Offloads decode and encode to the GPU. Whether an option actually works depends on the container having the device passed through and the FFmpeg build supporting it — a mismatch fails at playback time, not when you save.', qsvExtraHardwareFrames: - `Extra surfaces in the QSV upload pool. A pool of 0 is measured to fail outright — the channel serves nothing at all — while values between 1 and ${MINIMUM_QSV_EXTRA_HARDWARE_FRAMES - 1} are untested rather than known-bad. The server raises anything smaller to ${MINIMUM_QSV_EXTRA_HARDWARE_FRAMES} and logs the override rather than trusting it.`, + `Extra surfaces in the QSV upload pool. A pool of 0 is measured to fail outright — the channel serves nothing at all — while values between 1 and ${MINIMUM_QSV_EXTRA_HARDWARE_FRAMES - 1} are untested rather than known-bad. Rather than trust them, the server raises anything smaller to ${MINIMUM_QSV_EXTRA_HARDWARE_FRAMES} as it saves.`, qsvPreferNativeDecoder: 'Splits the pipeline: VA-API decodes, QSV encodes. The VA-API decoder tolerates imperfect streams that the QSV decoder rejects outright, and it is required for Dolby Vision, so this is the recommended setting on Intel.', normalizeLoudnessMode: - 'Levels volume across content from different sources. Off leaves each item at its own level, so volume jumps between them; loudnorm retargets everything to the fixed integrated loudness below, which evens that out at the cost of an extra filter in the graph and a less faithful dynamic range.', + 'Levels volume across content from different sources. Off leaves each item at its own level, so volume jumps between them; loudnorm retargets everything to one integrated loudness, which evens that out at the cost of an extra filter in the graph and a less faithful dynamic range.', targetLoudness: 'The integrated loudness loudnorm aims for, in LUFS. Broadcast practice sits near -23; streaming services sit nearer -16. Louder targets leave less headroom and are more likely to clip peaks.' // `as const` rather than `Record`: under an index signature `FIELD_HELP.typo` -- 2.47.3 From f3d003b8acee7e04b167e01856ba38de543f72e0 Mon Sep 17 00:00:00 2001 From: Timothy Date: Wed, 26 Aug 2026 19:25:43 +0200 Subject: [PATCH 7/7] fix(734): a fifth wrong copy claim, and the N5 replacement was vacuous too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third cold review: BLOCKED on N8, with N9/N10 non-blocking. N8 — the hardware-acceleration paragraph told the operator to worry about whether the FFmpeg build supports a kind. It does not have to: `GetSupportedHardwareAccelerationKindsHandler` adds a kind only when `FFmpegCapabilities.HasHardwareAcceleration` is already true, so an unsupported kind never reaches the dropdown and cannot fail at playback. On this host the dropdown offers exactly ["None","VideoToolbox"]. The device-passthrough half was right, and is now the whole claim: nothing checks at save time whether the device is present and passed through, and that is what fails at playback. That is the FIFTH wrong copy claim across three review rounds, in nine paragraphs — three fixed in round two, a fourth found in round three, this one found after I asked for a fifth. The decision record now says so plainly, because the pattern is the lesson: prose about behaviour is not cheaper to get right than code, and one pass over it is demonstrably not enough. N9 — the test I wrote to replace the vacuously-arranged one was itself vacuous. Deleting the panel's onMouseEnter/onMouseLeave left 14/14 green, for two reasons the reviewer's mutation established: there was no assertion between the panel enter and leave, so the "keeps open" half was never checked; and React propagates portal events through the React tree, so mouseLeave(panel) also fires the WRAPPER's onMouseLeave — with `hoveredTrigger` never cleared, the closing assertion passed on the wrapper's signal alone. The test now leaves the wrapper first, so `hoveredPanel` is the only thing holding the panel open, and asserts the panel is still present at that point. Re-run with the reviewer's exact mutation: it now kills exactly that test, and the restored control is 14/14. N10 — `.ctv-field-help-panel-top`/`-bottom` were emptied by the bridge's removal and are now pure state hooks. Commented as such: do not hang an offset off them, because that is how the bridge started. Follow-ups filed rather than bolted on at round four: - #839 — panel placement (unclipped, auto-flip) is verified by live measurement, not by a committed test; jsdom has no layout and cannot see it. - #840 — the portal puts a panel-internal `docsHref` link at the end of the tab order. Trigger: the first screen that passes `docsHref`. Both linked from §15's adopting checklist and from the decision record, so an adopter meets them where it matters. Gate: lint clean, build clean, 1220/1220, decisions-validate OK, mirror 6/6 byte-identical (checker asserts it read all six before reporting). refs #734 --- .../templates/chicorytv-admin/FFmpegProfiles.jsx | 2 +- .../records/spa/field-progressive-disclosure.md | 12 ++++++++++-- docs/spa-conventions.md | 7 ++++++- web/src/components/components.css | 5 +++++ web/src/components/fieldHelp.test.tsx | 13 ++++++++++--- web/src/screens/FFmpegProfilesScreen.tsx | 2 +- 6 files changed, 33 insertions(+), 8 deletions(-) diff --git a/design-system/templates/chicorytv-admin/FFmpegProfiles.jsx b/design-system/templates/chicorytv-admin/FFmpegProfiles.jsx index 7ecf1e757..26b4f54fb 100644 --- a/design-system/templates/chicorytv-admin/FFmpegProfiles.jsx +++ b/design-system/templates/chicorytv-admin/FFmpegProfiles.jsx @@ -69,7 +69,7 @@ videoBufferSize: "How much bitrate deviation the encoder may bank before it has to correct. Roughly 2x the bitrate is the usual starting point. Very small values force a near-constant bitrate and hurt quality on scene changes.", hardwareAcceleration: - "Offloads decode and encode to the GPU. Whether an option actually works depends on the container having the device passed through and the FFmpeg build supporting it — a mismatch fails at playback time, not when you save.", + "Offloads decode and encode to the GPU. The list only offers what this FFmpeg build supports, so an unsupported kind never appears here. What nothing checks when you save is whether the device itself is present and passed through to the container — that is the mismatch that fails at playback time.", normalizeLoudnessMode: "Levels volume across content from different sources. Off leaves each item at its own level, so volume jumps between them; loudnorm retargets everything to one integrated loudness, which evens that out at the cost of an extra filter in the graph and a less faithful dynamic range.", }; diff --git a/docs/decisions/records/spa/field-progressive-disclosure.md b/docs/decisions/records/spa/field-progressive-disclosure.md index 8e9429de8..991a6ca0d 100644 --- a/docs/decisions/records/spa/field-progressive-disclosure.md +++ b/docs/decisions/records/spa/field-progressive-disclosure.md @@ -35,7 +35,8 @@ severe and non-obvious rather than backfilled across every screen. The portal costs two things, both accepted knowingly: the panel leaves any dialog's stacking context, so it needs `z-index: 110` to clear the `100` scrims, and a focusable element inside it lands at the end of the document tab order rather than beside its trigger (latent — no screen - passes `docsHref` yet). + passes `docsHref` yet; tracked in #840, triggered by the first screen that does). Panel placement + itself is verified by live measurement rather than a committed test — #839. - **Copy lives in the screen's own `FIELD_HELP`, declared `as const`.** Not in the shared component (it would collect copy for screens it knows nothing about) and not from the API (copy shipping separately from the field it describes drifts). `as const` rather than `Record`, @@ -69,4 +70,11 @@ severe and non-obvious rather than backfilled across every screen. warning lives in `QsvPipelineBuilder`, and because the save path normalises the value first, the log it promised cannot be produced by anything a user does on that screen. Copy describing options the user cannot choose, or behaviour the code does not have, is the exact drift this convention - exists to prevent — and one pass over it is demonstrably not enough. + exists to prevent. A fifth then survived the round that fixed the fourth: the hardware-acceleration + paragraph told the operator to worry about whether the FFmpeg build supports a kind, but + `GetSupportedHardwareAccelerationKindsHandler` only offers kinds for which + `FFmpegCapabilities.HasHardwareAcceleration` is already true, so an unsupported kind never reaches + the dropdown — the surviving failure is the device not being passed through, which nothing checks + at save time. **Five wrong claims across three review rounds, in nine paragraphs.** Prose about + behaviour is not cheaper to get right than code; check each sentence against the enum, the handler + or the record it describes, and expect more than one pass. diff --git a/docs/spa-conventions.md b/docs/spa-conventions.md index 544af1db6..67228abd1 100644 --- a/docs/spa-conventions.md +++ b/docs/spa-conventions.md @@ -1074,7 +1074,12 @@ do. 4. Reuse the `ctv-field-help*` classes in `components.css`; do not restyle per screen. 5. If the field needs a `docsHref`, remember the link is reached by pinning, and that the portal puts it at the END of the document's tab order rather than immediately after the trigger — a known - limitation, tracked rather than fixed because no screen passes `docsHref` yet. + limitation tracked in **#840**, whose trigger is exactly this: the first screen to pass + `docsHref`. Pick it up rather than working around it locally. + +Panel placement (not clipped, flips when it would run off the bottom) is currently verified by live +measurement, not by a committed test — **#839** tracks pinning it in the browser harness. Re-measure +if you change the offset, the placement logic or `.ctv-card`'s overflow. Backfilling every screen is deliberately **not** this convention's job — adopt it where a field's consequences are severe and non-obvious, which is what makes the icon meaningful rather than diff --git a/web/src/components/components.css b/web/src/components/components.css index 913b2fb79..437715f59 100644 --- a/web/src/components/components.css +++ b/web/src/components/components.css @@ -1264,6 +1264,11 @@ } +/* `.ctv-field-help-panel-top` / `-bottom` intentionally carry no rules: placement is inline (the + layout effect in fieldHelp.tsx computes it), and these remain purely as a state hook for tests + and live measurement. Do not hang an offset off them — that is how the withdrawn hover bridge + started. See spa-conventions.md §15. */ + .ctv-field-help-detail { display: block; } diff --git a/web/src/components/fieldHelp.test.tsx b/web/src/components/fieldHelp.test.tsx index b746a18e5..44693d710 100644 --- a/web/src/components/fieldHelp.test.tsx +++ b/web/src/components/fieldHelp.test.tsx @@ -116,10 +116,17 @@ describe('FieldHelp', () => { const wrap = trigger.parentElement as HTMLElement; fireEvent.mouseEnter(wrap); - const panel = screen.getByRole('note'); - fireEvent.mouseEnter(panel); + fireEvent.mouseEnter(screen.getByRole('note')); - fireEvent.mouseLeave(panel); + // Leave the WRAPPER first, so `hoveredPanel` is the only signal still holding the panel open. + // Without this the wrapper's own hover keeps it up and the assertions below pass with the + // panel's handlers deleted — React propagates portal events through the React tree, so + // mouseLeave(panel) also fires the wrapper's onMouseLeave. + fireEvent.mouseLeave(wrap); + expect(screen.getByRole('note')).toBeInTheDocument(); + expect(screen.getByRole('link', { name: /Learn more/ })).toBeInTheDocument(); + + fireEvent.mouseLeave(screen.getByRole('note')); expect(screen.queryByRole('note')).toBeNull(); }); diff --git a/web/src/screens/FFmpegProfilesScreen.tsx b/web/src/screens/FFmpegProfilesScreen.tsx index aaf69a34e..5dfb3e796 100644 --- a/web/src/screens/FFmpegProfilesScreen.tsx +++ b/web/src/screens/FFmpegProfilesScreen.tsx @@ -50,7 +50,7 @@ const FIELD_HELP = { videoBufferSize: 'How much bitrate deviation the encoder may bank before it has to correct. Roughly 2x the bitrate is the usual starting point. Very small values force a near-constant bitrate and hurt quality on scene changes.', hardwareAcceleration: - 'Offloads decode and encode to the GPU. Whether an option actually works depends on the container having the device passed through and the FFmpeg build supporting it — a mismatch fails at playback time, not when you save.', + 'Offloads decode and encode to the GPU. The list only offers what this FFmpeg build supports, so an unsupported kind never appears here. What nothing checks when you save is whether the device itself is present and passed through to the container — that is the mismatch that fails at playback time.', qsvExtraHardwareFrames: `Extra surfaces in the QSV upload pool. A pool of 0 is measured to fail outright — the channel serves nothing at all — while values between 1 and ${MINIMUM_QSV_EXTRA_HARDWARE_FRAMES - 1} are untested rather than known-bad. Rather than trust them, the server raises anything smaller to ${MINIMUM_QSV_EXTRA_HARDWARE_FRAMES} as it saves.`, qsvPreferNativeDecoder: -- 2.47.3