feat(734): field-level progressive disclosure — shared FieldHelp trigger + panel (#841)
Build ErsatzTV Image / CI toolchain image resolves (push) Successful in 7s
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 15s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 8m57s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m32s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 6m18s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 5m27s
Build ErsatzTV Image / CI toolchain image resolves (push) Successful in 7s
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 15s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 8m57s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m32s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 6m18s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 5m27s
Implements the three-level field-help pattern from #734 as a shared component: field name + optional one-sentence summary → a one-short-paragraph panel behind a consistent Info-icon trigger → a future external-docs deep link (`docsHref`, built and typed; no screen passes one yet). Adopted on FFmpegProfilesScreen (9 fields), documented as docs/spa-conventions.md §15 with decision record `spa.field-progressive-disclosure`, and mirrored into the design-system prototype. The panel is portalled to document.body: `.ctv-card` sets `overflow: hidden`, which clips a positioned descendant whatever its z-index, and one field's explainer rendered 12px of a 92px paragraph in every state of the Audio card. A `::before` hover bridge was added and then WITHDRAWN — it held for a vertical descent onto the panel and failed for a diagonal one, leaving a safe sideways exit of 1.25px on an 18px icon. Hover reads the paragraph in place; the panel's interactive content is reached by pinning. Four cold adversarial review rounds; the first three returned BLOCKED. They found five wrong copy claims across nine paragraphs and two vacuous tests in a row for the same mechanism. Deferred with owners: #839 (placement verified by hand, not by a test) and #840 (the portal puts a docsHref link at the end of the tab order). fixes #734 Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
This commit was merged in pull request #841.
This commit is contained in:
@@ -56,7 +56,71 @@
|
||||
{ label: "loudnorm", value: "LoudNorm" },
|
||||
];
|
||||
|
||||
function Row({ label, help, control = 320, first = false, children }) {
|
||||
// Level-2 explainer copy for the field-level progressive-disclosure pattern (ersatztv#734).
|
||||
// Mirrors web/src/screens/FFmpegProfilesScreen.tsx's FIELD_HELP record; see
|
||||
// docs/spa-conventions.md §15 for the contract this shape is mirroring.
|
||||
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 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:
|
||||
"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. 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.",
|
||||
};
|
||||
|
||||
// Mockup of the shared `FieldHelp` primitive (web/src/components/fieldHelp.tsx). The prototype
|
||||
// reproduces the two opening signals that read in a static review (tap-pins and hover); the shipped
|
||||
// component additionally handles keyboard focus, Escape, outside-press dismissal, the hover
|
||||
// bridge, and portalling the panel out of the card's clip.
|
||||
function FieldHelp({ label, detail }) {
|
||||
const [pinned, setPinned] = React.useState(false);
|
||||
const [hovered, setHovered] = React.useState(false);
|
||||
const open = pinned || hovered;
|
||||
return (
|
||||
<span
|
||||
style={{ position: "relative", display: "inline-flex", verticalAlign: "middle", marginLeft: 5 }}
|
||||
onMouseEnter={() => setHovered(true)}
|
||||
onMouseLeave={() => setHovered(false)}
|
||||
>
|
||||
<button
|
||||
type="button"
|
||||
aria-label={`More about ${label}`}
|
||||
aria-expanded={open}
|
||||
onClick={(e) => { e.preventDefault(); setPinned((v) => !v); }}
|
||||
style={{
|
||||
display: "inline-flex", alignItems: "center", justifyContent: "center",
|
||||
width: 18, height: 18, border: "none", borderRadius: "var(--radius-xs)",
|
||||
background: "none", padding: 0, color: "var(--text-disabled)", cursor: "help",
|
||||
}}
|
||||
>
|
||||
<Ico.Info aria-hidden="true" size={13} />
|
||||
</button>
|
||||
{open && (
|
||||
<span
|
||||
role="note"
|
||||
style={{
|
||||
position: "absolute", zIndex: 60, left: 0, top: "calc(100% + 7px)",
|
||||
width: "max-content", maxWidth: 320,
|
||||
border: "1px solid var(--border-control)", borderRadius: "var(--radius-sm)",
|
||||
background: "var(--ctv-surface-3)", boxShadow: "var(--shadow-pop)",
|
||||
padding: "9px 11px", color: "var(--text-primary)",
|
||||
font: "var(--text-xs)/1.5 var(--font-sans)", whiteSpace: "normal", textAlign: "left",
|
||||
}}
|
||||
>
|
||||
{detail}
|
||||
</span>
|
||||
)}
|
||||
</span>
|
||||
);
|
||||
}
|
||||
|
||||
function Row({ label, help, detail, control = 320, first = false, children }) {
|
||||
return (
|
||||
<div
|
||||
style={{
|
||||
@@ -68,7 +132,10 @@
|
||||
}}
|
||||
>
|
||||
<div style={{ flex: "1 1 auto", minWidth: 0, paddingTop: 5 }}>
|
||||
<div style={{ font: "var(--weight-medium) var(--text-sm)/1.3 var(--font-sans)", color: "var(--text-primary)" }}>{label}</div>
|
||||
<div style={{ font: "var(--weight-medium) var(--text-sm)/1.3 var(--font-sans)", color: "var(--text-primary)" }}>
|
||||
{label}
|
||||
{detail && <FieldHelp detail={detail} label={label} />}
|
||||
</div>
|
||||
{help && <div style={{ marginTop: 3, font: "var(--text-xs)/1.4 var(--font-sans)", color: "var(--text-secondary)", maxWidth: 460 }}>{help}</div>}
|
||||
</div>
|
||||
<div style={{ flex: `0 0 ${control}px` }}>{children}</div>
|
||||
@@ -177,7 +244,7 @@
|
||||
<Row first label="Name" control={360}>
|
||||
<Input size="sm" value={isAdd ? "" : profile.name} placeholder="e.g. 1080p30 Software" />
|
||||
</Row>
|
||||
<Row label="Thread count" control={200}>
|
||||
<Row label="Thread count" control={200} detail={FIELD_HELP.threadCount} help="0 lets FFmpeg pick a thread count.">
|
||||
<Input size="sm" type="number" trailing="threads" value="0" />
|
||||
</Row>
|
||||
<Row label="Normalize audio" control={200}>
|
||||
@@ -191,7 +258,7 @@
|
||||
</Row>
|
||||
{normalizeVideo && (
|
||||
<React.Fragment>
|
||||
<Row label="Scaling behavior" control={360}>
|
||||
<Row label="Scaling behavior" control={360} detail={FIELD_HELP.scalingBehavior} help="What to do when the source aspect ratio does not match.">
|
||||
<Select value="ScaleAndPad" options={SCALING_OPTIONS} />
|
||||
</Row>
|
||||
<Row control={360} label="Pad mode" help="Hardware padding only applies with VAAPI; otherwise software padding is used.">
|
||||
@@ -219,13 +286,13 @@
|
||||
<Row label="Bit depth" control={360}>
|
||||
<Select value="EightBit" options={BIT_DEPTH_OPTIONS} />
|
||||
</Row>
|
||||
<Row label="Bitrate" control={220}>
|
||||
<Row label="Bitrate" control={220} detail={FIELD_HELP.videoBitrate}>
|
||||
<Input size="sm" type="number" trailing="kBit/s" value="2000" />
|
||||
</Row>
|
||||
<Row label="Buffer size" control={220}>
|
||||
<Row label="Buffer size" control={220} detail={FIELD_HELP.videoBufferSize} help="Usually about twice the bitrate.">
|
||||
<Input size="sm" type="number" trailing="kBit" value="4000" />
|
||||
</Row>
|
||||
<Row label="Hardware acceleration" control={360}>
|
||||
<Row label="Hardware acceleration" control={360} detail={FIELD_HELP.hardwareAcceleration} help="Requires the device to be passed through to the container.">
|
||||
<Select value={hwaccel} options={HWACCEL_OPTIONS} onChange={(e) => setHwaccel(e.target.value)} />
|
||||
</Row>
|
||||
{hwaccel === "Vaapi" && (
|
||||
@@ -290,7 +357,7 @@
|
||||
<Row label="Sample rate" control={220}>
|
||||
<Input size="sm" type="number" trailing="kHz" value="48" />
|
||||
</Row>
|
||||
<Row label="Normalize loudness" control={360}>
|
||||
<Row label="Normalize loudness" control={360} detail={FIELD_HELP.normalizeLoudnessMode}>
|
||||
<Select value={loudness} options={LOUDNESS_OPTIONS} onChange={(e) => setLoudness(e.target.value)} />
|
||||
</Row>
|
||||
{loudness === "LoudNorm" && (
|
||||
|
||||
@@ -20,6 +20,7 @@ doc below, or that changes which sections a task signal points to.**
|
||||
| Named-issue pickup | Skip queue selection; go straight to focused retrieval — see "Knowledge retrieval" below, then the issue body |
|
||||
| Adding/changing a `/api/*` endpoint | `docs/api-conventions.md` checklist + `docs/endpoint-index.md` |
|
||||
| Adding a ChicoryTV SPA screen | `docs/spa-conventions.md` |
|
||||
| Explaining a consequential settings field in the SPA (summary → hover/tap panel → docs link) | `docs/spa-conventions.md` §15 — use the shared `FieldHelp` trigger and put the copy in the screen's own `FIELD_HELP` record; the icon, the gesture and the a11y contract are fixed |
|
||||
| Scheduling / playout engine work | `docs/domain-model.md` + decisions catalog rows keyed `sched.*` (`docs/decisions/README.md`) |
|
||||
| Adding or changing a paged list handler (a page plus a `TotalCount`) | Resolve `api.paged-count-matches-page-query` via `docs/decisions/README.md` — for an EF-backed filtered list, count the SAME query you page, with includes appended to the page chain only; where the count and the page are separate methods, a test pins their agreement. Then `api.paging-zero-based` for the `pageNum`/`pageSize` contract |
|
||||
| Concurrency / optimistic-locking work | `docs/api-conventions.md` §7a/b/c + `docs/decisions/optimistic-concurrency.md` |
|
||||
|
||||
@@ -184,6 +184,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera
|
||||
| `spa.datetime-local-input` | The channel-mode date/time input uses a native `<input type="datetime-local">` 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:*<escaped>*`), 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 `<select>`. The other half of the superseded record is UNCHANGED: bounded-by-construction admin lists (collections, multi-collections, smart collections, playlists) still page to completeness via `loadAllPages` and still report `complete`/`hint: incomplete`. Server-side caps are not raised — this is a web-only change. | 2026-07-26 | [link](records/spa/library-pickers-resolve-by-search.md) |
|
||||
| `spa.logs-page-size-local` | The Logs page rows-per-page preference is stored in `window.localStorage` (`ctv-logs-page-size`), not a server `ConfigElement`. | 2026-07-11 | [link](records/spa/logs-page-size-local.md) |
|
||||
|
||||
@@ -0,0 +1,80 @@
|
||||
---
|
||||
key: spa.field-progressive-disclosure
|
||||
title: '2026-08-26 — Field-level progressive disclosure: one shared `FieldHelp` trigger, copy in the screen, panel portalled out of the card clip (#734)'
|
||||
status: active
|
||||
since: '2026-08-26'
|
||||
supersedes: none
|
||||
superseded-by: none
|
||||
rule: 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`.
|
||||
signals: 'field help, explainer popup, info icon, tooltip vs panel, settings copy · paths: `web/src/components/fieldHelp.tsx`, `web/src/screens/FFmpegProfilesScreen.tsx` · issues: #734'
|
||||
mechanics: spa-conventions.md §15; `ctv-field-help*` in components.css
|
||||
---
|
||||
|
||||
Settings fields — FFmpeg profiles above all — carry consequences that are severe and non-obvious
|
||||
while the UI gives a bare label. The information existed only in decision records and source
|
||||
comments, invisible to the person changing the value. Three levels: field name plus an optional
|
||||
one-sentence summary, a one-short-paragraph panel behind a trigger, and a future external-docs deep
|
||||
link (`docsHref`, built and typed, with no live target yet — do not invent a URL to fill it).
|
||||
|
||||
The value is in the pattern being **identical everywhere**; applied ad-hoc it is visual noise. Hence
|
||||
one shared component rather than per-screen popovers, adopted where a field's consequences are
|
||||
severe and non-obvious rather than backfilled across every screen.
|
||||
|
||||
- **The trigger is an icon in a real `<button>`, never the field name.** A settings label is
|
||||
frequently a `<label>` bound to its control, so a label-wide trigger competes with click-to-focus.
|
||||
The click handler calls `preventDefault()` as belt-and-braces for the nesting; per the HTML spec a
|
||||
label's activation behaviour is skipped for events targeted at interactive content, and Chromium
|
||||
was verified to behave as specified, so this guards a case no engine is currently known to hit.
|
||||
- **The panel is portalled to `document.body` with `position: fixed`.** `.ctv-card` sets
|
||||
`overflow: hidden`, which clips a positioned descendant whatever its `z-index`; an in-flow panel on
|
||||
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 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; 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<string, string>`,
|
||||
under which a mistyped key types as `string` and renders a trigger with an empty panel and no
|
||||
build error.
|
||||
- **`FieldHelp` and `Tooltip` coexist deliberately.** `Tooltip` is a single-line, `nowrap`,
|
||||
`pointer-events: none`, hover/focus-only label for a control whose purpose is not otherwise
|
||||
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. **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
|
||||
none. Escape and outside-press dismissal are armed only when pinned or focused, never on hover
|
||||
alone, or a document-level Escape would steal focus from whatever the user was actually dismissing
|
||||
whenever a pointer happened to rest on an info icon.
|
||||
- **The copy is checked against the option list and the server rule, not recollection.** The first
|
||||
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. 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. 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.
|
||||
@@ -953,3 +953,134 @@ list filter/count off the rollup `status`, never a filter per individual fault:
|
||||
emits plain strings backed by a `const string` class (`ChannelHealthStatus`, `ChannelFault`), not a
|
||||
generated enum, so the SPA's local TS union (`'Healthy' | 'Problems' | 'Unknown'`, and the fault
|
||||
union) is kept in sync by hand when the server adds a value.
|
||||
|
||||
## 15. Field-level progressive disclosure (`FieldHelp`, #734)
|
||||
|
||||
Settings fields — FFmpeg profiles above all — carry consequences that are severe and non-obvious,
|
||||
while the UI gives a bare label. The information exists (decision records, source comments) but is
|
||||
invisible to the person changing the value. The pattern below puts it one interaction away without
|
||||
cluttering the form, and **its value is in being identical everywhere**: one icon, one gesture, one
|
||||
place users learn to look. Applied ad-hoc per screen it is just visual noise, so adopt this shape or
|
||||
none.
|
||||
|
||||
Decision record: `docs/decisions/records/spa/field-progressive-disclosure.md`
|
||||
(`spa.field-progressive-disclosure`).
|
||||
|
||||
### The three levels
|
||||
|
||||
| Level | What | Where it lives | Length limit |
|
||||
| --- | --- | --- | --- |
|
||||
| 1 | Field name, optionally a one-sentence summary | the screen's own row markup (`.ctv-settings-row-label` / `.ctv-settings-row-help`) | **one short sentence**, hard limit |
|
||||
| 2 | Explainer panel behind a trigger next to the name | `FieldHelp` (`web/src/components/fieldHelp.tsx`) | **one short paragraph**, hard limit |
|
||||
| 3 | Deep link to external docs | `FieldHelp`'s `docsHref` prop | n/a — external |
|
||||
|
||||
Level 3 has **no live target yet**; the affordance is built and typed so a screen can adopt it the
|
||||
day the docs exist, and no screen passes `docsHref` today. Do not invent a URL to fill it.
|
||||
|
||||
### The trigger is an icon, never the label
|
||||
|
||||
`FieldHelp` renders a `lucide-react` `Info` icon inside a real `<button type="button">`, placed
|
||||
immediately after the field name. **Do not make the field name itself the trigger** — a settings
|
||||
label is frequently a `<label>` bound to its control (`components/forms.tsx` wraps the input in
|
||||
one), so a label-wide trigger competes with click-to-focus. The trigger's click handler calls
|
||||
`preventDefault()` as belt-and-braces for that nesting: per the HTML spec a label's activation
|
||||
behaviour is skipped for events targeted at *interactive content*, and a `<button>` is interactive
|
||||
content, so this guards a case no engine is currently known to hit — Chromium was verified to
|
||||
behave as specified. It costs nothing and does not depend on every engine agreeing.
|
||||
`fieldHelp.test.tsx` asserts the mechanism (`defaultPrevented`) rather than "the input did not
|
||||
focus", because jsdom does not implement label activation at all and the obvious assertion would
|
||||
pass vacuously.
|
||||
|
||||
### Copy lives with the field definition
|
||||
|
||||
Put the level-2 paragraphs in a module-level `FIELD_HELP` object (declared `as const`, **not**
|
||||
`Record<string, string>` — under an index signature a mistyped key types as `string` and renders a
|
||||
trigger with an empty panel and no build error) in the **screen module**, next to the field
|
||||
definitions it describes (see `FFmpegProfilesScreen.tsx`), and pass
|
||||
`detail={FIELD_HELP.someField}` at the call site. Not in the shared component (it would collect
|
||||
copy for screens it knows nothing about) and **not from the API** — copy that ships separately from
|
||||
the field it describes drifts, and drifted copy is worse than none. Where a bound comes from a
|
||||
constant the form already uses, interpolate it (`MINIMUM_QSV_EXTRA_HARDWARE_FRAMES`) rather than
|
||||
restating the number.
|
||||
|
||||
### The panel is portalled — do not put it back in the wrapper
|
||||
|
||||
`.ctv-card` sets `overflow: hidden`, and an `overflow: hidden` ancestor clips a positioned
|
||||
descendant **whatever its `z-index`**. An in-flow panel on a card's last row was measured showing
|
||||
12px of a 92px paragraph — the Audio card's final row always carries a `detail`, so one field's
|
||||
explainer was destroyed in every state of that card. `FieldHelp` therefore renders the panel through
|
||||
`createPortal` into `document.body` with `position: fixed`, positioned from the trigger's viewport
|
||||
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
|
||||
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** — 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 <field>`, **Escape** closes
|
||||
and returns focus to the trigger, and an outside pointer press dismisses. Two details are easy to
|
||||
get wrong and are pinned by tests:
|
||||
|
||||
- **`role="note"` is not a live region** and nothing announces the panel when it appears. What
|
||||
actually reads the paragraph out is `aria-describedby` on the *trigger*, set only while the panel
|
||||
is open. Do not treat the role as satisfying the announcement requirement — it does not.
|
||||
- **`aria-controls`/`aria-describedby` are set only while the panel exists.** A reference to an id
|
||||
that is not in the document is worse than no reference at all.
|
||||
|
||||
Escape and outside-press are armed only when the panel is pinned or focused, never when it is merely
|
||||
hovered: 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 dismissing. Escape's re-focus
|
||||
deliberately does not re-open the panel — that regression is pinned by a test.
|
||||
|
||||
### `FieldHelp` vs `Tooltip`
|
||||
|
||||
They coexist on purpose. `Tooltip` (`components/feedback.tsx`) is a single-line, `nowrap`,
|
||||
`pointer-events: none`, hover/focus-only label for a control whose purpose is not otherwise stated —
|
||||
an icon button, say. It cannot hold a paragraph, cannot hold a link, and touch users never see it.
|
||||
**Do not widen `Tooltip` into this role**, and do not use `FieldHelp` where a two-word label would
|
||||
do.
|
||||
|
||||
### Adopting it on a new screen
|
||||
|
||||
1. Give the row a level-1 summary if one short sentence genuinely helps; skip it otherwise.
|
||||
2. Add the paragraph to that screen's `FIELD_HELP` record. **Check it against the option list and
|
||||
the server-side rule, not against your recollection**: the first cut of this screen's copy
|
||||
compared loudnorm to a dynamic-normalization mode this fork does not have, described scaling as a
|
||||
binary choice while omitting `Stretch`, and asserted a range as known-bad that its own decision
|
||||
record calls untested. Copy that describes options the user cannot choose is exactly the drift
|
||||
this convention exists to prevent.
|
||||
3. Render `<FieldHelp detail={…} label={fieldName} />` 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 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
|
||||
decorative. `FFmpegProfilesScreen.tsx` is the reference implementation (nine fields).
|
||||
|
||||
@@ -39,6 +39,7 @@
|
||||
.ctv-tag-remove:focus-visible,
|
||||
.ctv-toast-close:focus-visible,
|
||||
.ctv-nav-item:focus-visible,
|
||||
.ctv-field-help-trigger:focus-visible,
|
||||
.ctv-tab:focus-visible {
|
||||
outline: 0;
|
||||
box-shadow: var(--ring-focus, 0 0 0 3px var(--focus-ring));
|
||||
@@ -1200,3 +1201,88 @@
|
||||
animation: none;
|
||||
}
|
||||
}
|
||||
/* ---- Field-level progressive disclosure (#734) ----
|
||||
Level 2 of the pattern: a consistent trigger next to a field name that opens a one-paragraph
|
||||
explainer, optionally deep-linking to external docs. Distinct from `.ctv-tooltip`, which is a
|
||||
single-line, pointer-events:none label — see components/fieldHelp.tsx for why they coexist. */
|
||||
.ctv-field-help {
|
||||
position: relative;
|
||||
display: inline-flex;
|
||||
vertical-align: middle;
|
||||
margin-left: 5px;
|
||||
}
|
||||
|
||||
.ctv-field-help-trigger {
|
||||
display: inline-flex;
|
||||
align-items: center;
|
||||
justify-content: center;
|
||||
/* No global box-sizing reset in this SPA (#377): keep the hit target padding-free and sized. */
|
||||
width: 18px;
|
||||
height: 18px;
|
||||
border: none;
|
||||
border-radius: var(--radius-xs);
|
||||
background: none;
|
||||
padding: 0;
|
||||
color: var(--text-disabled);
|
||||
cursor: help;
|
||||
}
|
||||
|
||||
.ctv-field-help-trigger:hover,
|
||||
.ctv-field-help-trigger:focus-visible {
|
||||
color: var(--text-secondary);
|
||||
}
|
||||
|
||||
/* The panel is PORTALLED to document.body and positioned in viewport coordinates by
|
||||
fieldHelp.tsx. `.ctv-card` sets `overflow: hidden`, which clips a positioned descendant whatever
|
||||
its z-index — an in-flow panel on a card's last row was measured showing 12px of a 92px
|
||||
paragraph. Being at body level is what makes the copy readable from any row of any card, so do
|
||||
not "simplify" this back into the wrapper. */
|
||||
.ctv-field-help-panel {
|
||||
position: fixed;
|
||||
/* 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;
|
||||
/* The SPA is authored under the default content-box (#377); without this the 11px side padding
|
||||
is added to max-width and the panel renders 344px, not 320px. */
|
||||
box-sizing: border-box;
|
||||
width: max-content;
|
||||
max-width: 320px;
|
||||
border: 1px solid var(--border-control);
|
||||
border-radius: var(--radius-sm);
|
||||
background: var(--ctv-surface-3);
|
||||
box-shadow: var(--shadow-pop);
|
||||
padding: 9px 11px;
|
||||
color: var(--text-primary);
|
||||
font: var(--text-xs) / 1.5 var(--font-sans);
|
||||
/* Unlike .ctv-tooltip this panel IS interactive — it can carry a docs link. */
|
||||
white-space: normal;
|
||||
text-align: left;
|
||||
}
|
||||
|
||||
|
||||
/* `.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;
|
||||
}
|
||||
|
||||
.ctv-field-help-docs {
|
||||
display: inline-flex;
|
||||
align-items: center;
|
||||
gap: 4px;
|
||||
align-self: flex-start;
|
||||
color: var(--ctv-accent);
|
||||
font-weight: var(--weight-medium);
|
||||
text-decoration: none;
|
||||
}
|
||||
|
||||
.ctv-field-help-docs:hover {
|
||||
text-decoration: underline;
|
||||
}
|
||||
|
||||
@@ -0,0 +1,201 @@
|
||||
import { cleanup, fireEvent, render, screen } from '@testing-library/react';
|
||||
import { afterEach, describe, expect, it } from 'vitest';
|
||||
|
||||
import { FieldHelp } from '.';
|
||||
|
||||
const DETAIL = 'A pool of 0 is measured to fail outright; 1 to 63 are untested rather than known-bad.';
|
||||
|
||||
function renderHelp(props: Partial<Parameters<typeof FieldHelp>[0]> = {}) {
|
||||
render(<FieldHelp detail={DETAIL} label="QSV extra hardware frames" {...props} />);
|
||||
return screen.getByRole('button', { name: 'More about QSV extra hardware frames' });
|
||||
}
|
||||
|
||||
describe('FieldHelp', () => {
|
||||
afterEach(cleanup);
|
||||
|
||||
it('starts collapsed with no dangling references to a panel that does not exist', () => {
|
||||
const trigger = renderHelp();
|
||||
|
||||
expect(trigger).toHaveAttribute('aria-expanded', 'false');
|
||||
// An aria-controls/aria-describedby pointing at a missing id is worse than none at all.
|
||||
expect(trigger).not.toHaveAttribute('aria-controls');
|
||||
expect(trigger).not.toHaveAttribute('aria-describedby');
|
||||
expect(screen.queryByRole('note')).toBeNull();
|
||||
});
|
||||
|
||||
// role="note" is not a live region, so nothing announces the panel on appearance.
|
||||
// aria-describedby on the focused trigger is what actually gets the paragraph read out.
|
||||
it('describes the trigger with the panel while it is open', () => {
|
||||
const trigger = renderHelp();
|
||||
|
||||
fireEvent.focus(trigger);
|
||||
const panel = screen.getByRole('note');
|
||||
expect(trigger.getAttribute('aria-describedby')).toBe(panel.id);
|
||||
expect(trigger.getAttribute('aria-controls')).toBe(panel.id);
|
||||
});
|
||||
|
||||
// The panel is portalled out of the wrapper because .ctv-card clips with overflow:hidden.
|
||||
it('renders the panel outside the trigger wrapper, as a direct child of document.body', () => {
|
||||
const trigger = renderHelp();
|
||||
const wrap = trigger.parentElement as HTMLElement;
|
||||
|
||||
fireEvent.click(trigger);
|
||||
const panel = screen.getByRole('note');
|
||||
|
||||
expect(wrap.contains(panel)).toBe(false);
|
||||
expect(panel.parentElement).toBe(document.body);
|
||||
});
|
||||
|
||||
// Tap is the only opening gesture available to a touch user, so it must work on its own —
|
||||
// this is the assertion that a hover-only implementation would fail.
|
||||
it('toggles open and closed on click/tap', () => {
|
||||
const trigger = renderHelp();
|
||||
|
||||
fireEvent.click(trigger);
|
||||
expect(trigger).toHaveAttribute('aria-expanded', 'true');
|
||||
expect(screen.getByRole('note')).toHaveTextContent(DETAIL);
|
||||
expect(trigger.getAttribute('aria-controls')).toBe(screen.getByRole('note').id);
|
||||
|
||||
fireEvent.click(trigger);
|
||||
expect(screen.queryByRole('note')).toBeNull();
|
||||
});
|
||||
|
||||
// The trigger can sit inside a <label> (components/forms.tsx wraps its control in one). Per the
|
||||
// HTML spec a label's activation behaviour is skipped for events targeted at interactive content,
|
||||
// and Chromium was verified to do exactly that, so preventDefault here guards a case no engine is
|
||||
// currently known to hit — it is belt-and-braces, not a fix for an observed bug. The assertion is
|
||||
// on the mechanism rather than on "the input did not focus": jsdom does not implement label
|
||||
// activation at all (a positive control left focus on <body>), so the obvious assertion would
|
||||
// pass vacuously. fireEvent.click returns false exactly when the handler called preventDefault.
|
||||
it('prevents the default click action, belt-and-braces against a wrapping label', () => {
|
||||
const trigger = renderHelp();
|
||||
|
||||
expect(fireEvent.click(trigger)).toBe(false);
|
||||
});
|
||||
|
||||
it('opens on keyboard focus and closes on blur', () => {
|
||||
const trigger = renderHelp();
|
||||
|
||||
fireEvent.focus(trigger);
|
||||
expect(screen.getByRole('note')).toHaveTextContent(DETAIL);
|
||||
|
||||
fireEvent.blur(trigger);
|
||||
expect(screen.queryByRole('note')).toBeNull();
|
||||
});
|
||||
|
||||
it('opens on hover and closes when the pointer leaves', () => {
|
||||
const trigger = renderHelp();
|
||||
const wrap = trigger.parentElement as HTMLElement;
|
||||
|
||||
fireEvent.mouseEnter(wrap);
|
||||
expect(screen.getByRole('note')).toHaveTextContent(DETAIL);
|
||||
|
||||
fireEvent.mouseLeave(wrap);
|
||||
expect(screen.queryByRole('note')).toBeNull();
|
||||
});
|
||||
|
||||
it('keeps a pinned panel open after the pointer leaves', () => {
|
||||
const trigger = renderHelp();
|
||||
const wrap = trigger.parentElement as HTMLElement;
|
||||
|
||||
fireEvent.click(trigger);
|
||||
fireEvent.mouseEnter(wrap);
|
||||
fireEvent.mouseLeave(wrap);
|
||||
|
||||
expect(screen.getByRole('note')).toHaveTextContent(DETAIL);
|
||||
});
|
||||
|
||||
// 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);
|
||||
fireEvent.mouseEnter(screen.getByRole('note'));
|
||||
|
||||
// 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();
|
||||
});
|
||||
|
||||
// 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', () => {
|
||||
render(<input data-testid="elsewhere" />);
|
||||
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();
|
||||
|
||||
fireEvent.click(trigger);
|
||||
expect(screen.getByRole('note')).toBeInTheDocument();
|
||||
|
||||
fireEvent.keyDown(document, { key: 'Escape' });
|
||||
|
||||
expect(screen.queryByRole('note')).toBeNull();
|
||||
expect(document.activeElement).toBe(trigger);
|
||||
});
|
||||
|
||||
it('closes a pinned panel on an outside pointer press but not an inside one', () => {
|
||||
const trigger = renderHelp({ docsHref: 'https://example.invalid/docs' });
|
||||
|
||||
fireEvent.click(trigger);
|
||||
fireEvent.pointerDown(screen.getByRole('link', { name: /Learn more/ }));
|
||||
expect(screen.getByRole('note')).toBeInTheDocument();
|
||||
|
||||
fireEvent.pointerDown(document.body);
|
||||
expect(screen.queryByRole('note')).toBeNull();
|
||||
});
|
||||
|
||||
it('renders the level-3 docs affordance only when a target is supplied', () => {
|
||||
const trigger = renderHelp();
|
||||
fireEvent.click(trigger);
|
||||
expect(screen.queryByRole('link')).toBeNull();
|
||||
cleanup();
|
||||
|
||||
const withDocs = renderHelp({ docsHref: 'https://example.invalid/docs', docsLabel: 'Full reference' });
|
||||
fireEvent.click(withDocs);
|
||||
|
||||
const link = screen.getByRole('link', { name: /Full reference/ });
|
||||
expect(link).toHaveAttribute('href', 'https://example.invalid/docs');
|
||||
expect(link).toHaveAttribute('target', '_blank');
|
||||
expect(link).toHaveAttribute('rel', 'noreferrer');
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,242 @@
|
||||
import {
|
||||
useCallback,
|
||||
useEffect,
|
||||
useId,
|
||||
useLayoutEffect,
|
||||
useRef,
|
||||
useState,
|
||||
type ReactNode
|
||||
} from 'react';
|
||||
import { createPortal } from 'react-dom';
|
||||
import { ExternalLink, Info } from 'lucide-react';
|
||||
|
||||
/**
|
||||
* Field-level progressive disclosure (#734).
|
||||
*
|
||||
* Level 1 (the field name plus an optional one-sentence summary) is rendered by the calling
|
||||
* screen's own row/label markup — see `.ctv-settings-row-help`. This component is level 2: a
|
||||
* consistently-shaped trigger that opens a one-short-paragraph explainer, with an optional
|
||||
* level-3 deep link to external docs.
|
||||
*
|
||||
* Why not `Tooltip` (components/feedback.tsx): that primitive is a single-line label for a
|
||||
* control whose purpose is not otherwise stated — it is `white-space: nowrap`, has
|
||||
* `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. */
|
||||
label: string;
|
||||
/** One short paragraph. Longer treatments belong behind `docsHref`, not here. */
|
||||
detail: ReactNode;
|
||||
/** Optional level-3 deep link. Renders a "Learn more" link inside the panel. */
|
||||
docsHref?: string;
|
||||
docsLabel?: string;
|
||||
}
|
||||
|
||||
/** 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;
|
||||
const EDGE = 8;
|
||||
|
||||
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<HTMLButtonElement | null>(null);
|
||||
const panelRef = useRef<HTMLSpanElement | null>(null);
|
||||
|
||||
// Four independent reasons the panel can be showing, none derived from another. `pinned` is the
|
||||
// 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);
|
||||
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 [position, setPosition] = useState<PanelPosition | null>(null);
|
||||
|
||||
const dismiss = useCallback(() => {
|
||||
setPinned(false);
|
||||
setHoveredTrigger(false);
|
||||
setHoveredPanel(false);
|
||||
setFocused(false);
|
||||
}, []);
|
||||
|
||||
// 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;
|
||||
// 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
|
||||
? { 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') {
|
||||
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 &&
|
||||
(triggerRef.current?.contains(target) || panelRef.current?.contains(target))
|
||||
) {
|
||||
return;
|
||||
}
|
||||
dismiss();
|
||||
};
|
||||
|
||||
document.addEventListener('keydown', onKeyDown);
|
||||
document.addEventListener('pointerdown', onPointerDown);
|
||||
return () => {
|
||||
document.removeEventListener('keydown', onKeyDown);
|
||||
document.removeEventListener('pointerdown', onPointerDown);
|
||||
};
|
||||
}, [dismiss, dismissible]);
|
||||
|
||||
const panel = open ? (
|
||||
<span
|
||||
ref={panelRef}
|
||||
id={panelId}
|
||||
role="note"
|
||||
className={`ctv-field-help-panel ctv-field-help-panel-${position?.placement ?? 'bottom'}`}
|
||||
style={{
|
||||
left: position?.left ?? 0,
|
||||
top: position?.top,
|
||||
bottom: position?.bottom,
|
||||
// Until the layout effect has measured, keep the panel out of sight rather than flashing it
|
||||
// at the wrong place; `position` is set synchronously before paint.
|
||||
visibility: position ? 'visible' : 'hidden'
|
||||
}}
|
||||
onMouseEnter={() => setHoveredPanel(true)}
|
||||
onMouseLeave={() => setHoveredPanel(false)}
|
||||
>
|
||||
<span className="ctv-field-help-detail">{detail}</span>
|
||||
{docsHref && (
|
||||
<a
|
||||
className="ctv-field-help-docs"
|
||||
href={docsHref}
|
||||
target="_blank"
|
||||
rel="noreferrer"
|
||||
onClick={() => setPinned(false)}
|
||||
>
|
||||
{docsLabel}
|
||||
<ExternalLink aria-hidden="true" size={12} />
|
||||
</a>
|
||||
)}
|
||||
</span>
|
||||
) : null;
|
||||
|
||||
return (
|
||||
<span
|
||||
className="ctv-field-help"
|
||||
onMouseEnter={() => setHoveredTrigger(true)}
|
||||
onMouseLeave={() => setHoveredTrigger(false)}
|
||||
>
|
||||
<button
|
||||
ref={triggerRef}
|
||||
type="button"
|
||||
className="ctv-field-help-trigger"
|
||||
aria-label={`More about ${label}`}
|
||||
aria-expanded={open}
|
||||
// Only while the panel exists: an `aria-controls`/`aria-describedby` pointing at a missing
|
||||
// id is worse than none. `aria-describedby` is what actually gets the paragraph ANNOUNCED —
|
||||
// `role="note"` is not a live region and nothing reads it on appearance.
|
||||
aria-controls={open ? panelId : undefined}
|
||||
aria-describedby={open ? panelId : undefined}
|
||||
// Belt-and-braces: this trigger can render inside a `<label>` (components/forms.tsx wraps
|
||||
// its control in one). Per the HTML spec a label's activation behaviour is skipped for
|
||||
// events targeted at interactive content, and Chromium was verified to do exactly that, so
|
||||
// this guards a case no engine is currently known to hit — it costs nothing and does not
|
||||
// depend on every engine agreeing.
|
||||
onClick={(event) => {
|
||||
event.preventDefault();
|
||||
setPinned((value) => !value);
|
||||
}}
|
||||
onFocus={() => {
|
||||
if (suppressFocusOpen.current) {
|
||||
suppressFocusOpen.current = false;
|
||||
return;
|
||||
}
|
||||
setFocused(true);
|
||||
}}
|
||||
onBlur={() => {
|
||||
suppressFocusOpen.current = false;
|
||||
setFocused(false);
|
||||
}}
|
||||
>
|
||||
<Info aria-hidden="true" size={13} />
|
||||
</button>
|
||||
{panel && createPortal(panel, document.body)}
|
||||
</span>
|
||||
);
|
||||
}
|
||||
@@ -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';
|
||||
|
||||
@@ -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,33 @@ 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 <Row help> prop at the call site.
|
||||
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 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:
|
||||
'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. 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:
|
||||
'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 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<string, string>`: 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
|
||||
// than left alone. `qsvPreferNativeDecoder` is optional in the schema and both draft builders
|
||||
@@ -287,20 +325,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 (
|
||||
<div className="ctv-settings-row" style={first ? { borderTop: 'none' } : undefined}>
|
||||
<div className="ctv-settings-row-main">
|
||||
<div className="ctv-settings-row-label">{label}</div>
|
||||
<div className="ctv-settings-row-label">
|
||||
{label}
|
||||
{detail && <FieldHelp detail={detail} docsHref={docsHref} label={label} />}
|
||||
</div>
|
||||
{help && <div className="ctv-settings-row-help">{help}</div>}
|
||||
</div>
|
||||
<div className="ctv-settings-row-control" style={{ flex: `0 0 ${control}px` }}>{children}</div>
|
||||
@@ -676,7 +723,7 @@ function ProfileEditor({ mode }: { mode: { kind: 'add'; from: null | number } |
|
||||
value={draft.name ?? ''}
|
||||
/>
|
||||
</Row>
|
||||
<Row control={200} label="Thread count">
|
||||
<Row control={200} detail={FIELD_HELP.threadCount} help="0 lets FFmpeg pick a thread count." label="Thread count">
|
||||
<NumberField onChange={(threadCount) => set({ threadCount })} value={draft.threadCount} />
|
||||
</Row>
|
||||
<Row control={200} label="Normalize audio">
|
||||
@@ -694,7 +741,7 @@ function ProfileEditor({ mode }: { mode: { kind: 'add'; from: null | number } |
|
||||
</Row>
|
||||
{draft.normalizeVideo && (
|
||||
<>
|
||||
<Row control={360} label="Scaling behavior">
|
||||
<Row control={360} detail={FIELD_HELP.scalingBehavior} help="What to do when the source aspect ratio does not match." label="Scaling behavior">
|
||||
<Select
|
||||
onChange={(event) => set({ scalingBehavior: event.target.value as Draft['scalingBehavior'] })}
|
||||
options={SCALING_OPTIONS}
|
||||
@@ -753,13 +800,13 @@ function ProfileEditor({ mode }: { mode: { kind: 'add'; from: null | number } |
|
||||
value={draft.bitDepth}
|
||||
/>
|
||||
</Row>
|
||||
<Row control={220} label="Bitrate">
|
||||
<Row control={220} detail={FIELD_HELP.videoBitrate} label="Bitrate">
|
||||
<NumberField onChange={(videoBitrate) => set({ videoBitrate })} unit="kBit/s" value={draft.videoBitrate} />
|
||||
</Row>
|
||||
<Row control={220} label="Buffer size">
|
||||
<Row control={220} detail={FIELD_HELP.videoBufferSize} help="Usually about twice the bitrate." label="Buffer size">
|
||||
<NumberField onChange={(videoBufferSize) => set({ videoBufferSize })} unit="kBit" value={draft.videoBufferSize} />
|
||||
</Row>
|
||||
<Row control={360} label="Hardware acceleration">
|
||||
<Row control={360} detail={FIELD_HELP.hardwareAcceleration} help="Requires the device to be passed through to the container." label="Hardware acceleration">
|
||||
<Select
|
||||
onChange={(event) => set({ hardwareAcceleration: event.target.value as HardwareAccelerationKind })}
|
||||
options={hwaccelKinds}
|
||||
@@ -790,7 +837,12 @@ function ProfileEditor({ mode }: { mode: { kind: 'add'; from: null | number } |
|
||||
</Row>
|
||||
)}
|
||||
{draft.hardwareAcceleration === 'Qsv' ? (
|
||||
<Row control={200} label="QSV extra hardware frames">
|
||||
<Row
|
||||
control={200}
|
||||
detail={FIELD_HELP.qsvExtraHardwareFrames}
|
||||
help={`Values below ${MINIMUM_QSV_EXTRA_HARDWARE_FRAMES} are raised to ${MINIMUM_QSV_EXTRA_HARDWARE_FRAMES} by the server.`}
|
||||
label="QSV extra hardware frames"
|
||||
>
|
||||
{/* below this the QSV upload pool has too little headroom and transcoding fails on
|
||||
any unthrottled read; the server floors it anyway (ersatztv#529) */}
|
||||
<NumberField
|
||||
@@ -811,7 +863,8 @@ function ProfileEditor({ mode }: { mode: { kind: 'add'; from: null | number } |
|
||||
{draft.hardwareAcceleration === 'Qsv' && (
|
||||
<Row
|
||||
control={200}
|
||||
help="Decode with the more error-tolerant VA-API decoder instead of QSV, while still encoding with QSV. Recommended on Intel — handles imperfect streams and is required for Dolby Vision."
|
||||
detail={FIELD_HELP.qsvPreferNativeDecoder}
|
||||
help="Decode with VA-API while still encoding with QSV."
|
||||
label="Prefer native decoder"
|
||||
>
|
||||
<Checkbox
|
||||
@@ -860,7 +913,7 @@ function ProfileEditor({ mode }: { mode: { kind: 'add'; from: null | number } |
|
||||
<Row control={220} label="Sample rate">
|
||||
<NumberField onChange={(audioSampleRate) => set({ audioSampleRate })} unit="kHz" value={draft.audioSampleRate} />
|
||||
</Row>
|
||||
<Row control={360} label="Normalize loudness">
|
||||
<Row control={360} detail={FIELD_HELP.normalizeLoudnessMode} label="Normalize loudness">
|
||||
<Select
|
||||
onChange={(event) => set({ normalizeLoudnessMode: event.target.value as Draft['normalizeLoudnessMode'] })}
|
||||
options={LOUDNESS_OPTIONS}
|
||||
@@ -868,7 +921,7 @@ function ProfileEditor({ mode }: { mode: { kind: 'add'; from: null | number } |
|
||||
/>
|
||||
</Row>
|
||||
{draft.normalizeLoudnessMode === 'LoudNorm' && (
|
||||
<Row control={220} label="Target loudness">
|
||||
<Row control={220} detail={FIELD_HELP.targetLoudness} label="Target loudness">
|
||||
<NumberField
|
||||
onChange={(targetLoudness) => set({ targetLoudness })}
|
||||
unit="LUFS"
|
||||
|
||||
Reference in New Issue
Block a user