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: