feat(734): field-level progressive disclosure — shared FieldHelp trigger + panel #841

Merged

timothy merged 7 commits from feat/734-field-progressive-disclosure into main 2026-08-26 20:05:54 +02:00

Conversation 1 Commits 7 Files Changed 10

+882 -19

timothy commented 2026-08-26 19:32:02 +02:00

Owner

Copy Link

Copy Source

Implements the three-level field-help pattern from #734 as a shared component, adopted on one reference screen.

Levels: field name + optional one-sentence summary → a one-short-paragraph panel behind a consistent icon trigger → a future external-docs deep link (docsHref, built and typed; no screen passes one — the docs do not exist yet).

• web/src/components/fieldHelp.tsx — an Info-icon <button> beside the field name, opening a role="note" panel portalled to document.body. Three independent opening signals: tap (pins), hover, keyboard focus.
• Reference adoption on FFmpegProfilesScreen (9 consequential fields). Copy lives in a screen-local FIELD_HELP declared as const, next to the field definitions.
• docs/spa-conventions.md §15 + decision record spa.field-progressive-disclosure + the docs/README.md task-signal row + the design-system prototype mirrored per docs/design-sync.md step 7.
• Deliberately not an extension of Tooltip: that primitive is single-line, nowrap, pointer-events: none and hover/focus-only, so it can hold neither a paragraph nor a link and touch users never see it.

What the reviews changed

Four cold adversarial rounds; the first three returned BLOCKED. Worth reading as the substance of the PR:

• The panel was invisible where it mattered. .ctv-card sets overflow: hidden, which clips a positioned descendant whatever its z-index. "Target loudness" rendered 12px of a 92px paragraph — and the Audio card's last row always carries help, so one field's explainer was destroyed in every state of that card. Fixed by portalling; re-measured across all 7 triggers, every panel now fully visible, three auto-flipping to top.
• A hover bridge was added and then WITHDRAWN. A ::before spanning the 7px trigger→panel gap fixed a vertical mouse walk and failed a diagonal one, leaving a safe sideways exit of 1.25px on an 18px icon. Both of my live measurements had been taken along the axis where it works. Rather than patch it a third time, the mechanism is gone and the docs say what is true: hover reads the paragraph in place; the panel's link is reached by pinning.
• Five wrong copy claims, across three rounds, in nine paragraphs. Loudnorm was compared to a dynamic-normalization mode this fork does not have; scaling was described as binary while omitting Stretch; 1..63 QSV frames were called known-bad where the decision record says only 0 and 64 were measured; the copy promised a save-time log neither handler can emit; and the hwaccel text warned about build support that GetSupportedHardwareAccelerationKindsHandler already filters on. All verified against the enum/handler/record before rewriting.
• Two vacuous tests in a row for the same mechanism — one arranged in the only event order that cannot fail, its replacement passing with the mechanism deleted (React propagates portal events through the React tree, so the panel closed on the wrapper's signal). Both now mutation-proved.

Verification

• npm run lint clean · npm run build clean · 1220/1220 web tests · 956 python guard tests · decisions_validate --base origin/main --head HEAD OK · H12 audit clean.
• Verified on the committed tree in a separate worktree, and re-run after rebasing onto 35affecd2.
• Live-E2E via scripts/e2e-local.sh: not required (pure-SPA, no write-path handler changed), but run anyway because the CSS and layout are invisible to jsdom — that is how the clipping blocker was measured and confirmed fixed.
• Mirror drift checked by a script that asserts it read all six shared strings before reporting clean (an earlier version matched zero keys and reported success).

Deferred, with owners

• #839 — panel placement is verified by live measurement, not a committed test. jsdom has no layout; web/e2e/ has one describe.serial spec that consumes the one-shot setup claim, so a second spec needs a session mechanism that does not exist yet.
• #840 — the portal puts a panel-internal docsHref link at the end of the tab order. Trigger: the first screen that passes docsHref.

Both are linked from §15's adopting checklist and the decision record.

fixes #734

Implements the three-level field-help pattern from #734 as a shared component, adopted on one reference screen. **Levels**: field name + optional one-sentence summary → a one-short-paragraph panel behind a consistent icon trigger → a future external-docs deep link (`docsHref`, built and typed; **no screen passes one** — the docs do not exist yet). - `web/src/components/fieldHelp.tsx` — an `Info`-icon `<button>` beside the field name, opening a `role="note"` panel **portalled to `document.body`**. Three independent opening signals: tap (pins), hover, keyboard focus. - Reference adoption on `FFmpegProfilesScreen` (9 consequential fields). Copy lives in a screen-local `FIELD_HELP` declared `as const`, next to the field definitions. - `docs/spa-conventions.md` §15 + decision record `spa.field-progressive-disclosure` + the `docs/README.md` task-signal row + the design-system prototype mirrored per `docs/design-sync.md` step 7. - Deliberately **not** an extension of `Tooltip`: that primitive is single-line, `nowrap`, `pointer-events: none` and hover/focus-only, so it can hold neither a paragraph nor a link and touch users never see it. ## What the reviews changed Four cold adversarial rounds; the first three returned BLOCKED. Worth reading as the substance of the PR: - **The panel was invisible where it mattered.** `.ctv-card` sets `overflow: hidden`, which clips a positioned descendant whatever its `z-index`. "Target loudness" rendered **12px of a 92px paragraph** — and the Audio card's last row *always* carries help, so one field's explainer was destroyed in every state of that card. Fixed by portalling; re-measured across all 7 triggers, every panel now fully visible, three auto-flipping to `top`. - **A hover bridge was added and then WITHDRAWN.** A `::before` spanning the 7px trigger→panel gap fixed a vertical mouse walk and failed a diagonal one, leaving a safe sideways exit of **1.25px on an 18px icon**. Both of my live measurements had been taken along the axis where it works. Rather than patch it a third time, the mechanism is gone and the docs say what is true: hover reads the paragraph in place; the panel's link is reached by pinning. - **Five wrong copy claims, across three rounds, in nine paragraphs.** Loudnorm was compared to a dynamic-normalization mode this fork does not have; scaling was described as binary while omitting `Stretch`; `1..63` QSV frames were called known-bad where the decision record says only `0` and `64` were measured; the copy promised a save-time log neither handler can emit; and the hwaccel text warned about build support that `GetSupportedHardwareAccelerationKindsHandler` already filters on. All verified against the enum/handler/record before rewriting. - **Two vacuous tests in a row for the same mechanism** — one arranged in the only event order that cannot fail, its replacement passing with the mechanism deleted (React propagates portal events through the *React* tree, so the panel closed on the wrapper's signal). Both now mutation-proved. ## Verification - `npm run lint` clean · `npm run build` clean · **1220/1220** web tests · 956 python guard tests · `decisions_validate --base origin/main --head HEAD` OK · H12 audit clean. - Verified on the **committed** tree in a separate worktree, and re-run after rebasing onto `35affecd2`. - Live-E2E via `scripts/e2e-local.sh`: not required (pure-SPA, no write-path handler changed), but run anyway because the CSS and layout are invisible to jsdom — that is how the clipping blocker was measured and confirmed fixed. - Mirror drift checked by a script that asserts it read all six shared strings before reporting clean (an earlier version matched zero keys and reported success). ## Deferred, with owners - **#839** — panel placement is verified by live measurement, not a committed test. jsdom has no layout; `web/e2e/` has one `describe.serial` spec that consumes the one-shot setup claim, so a second spec needs a session mechanism that does not exist yet. - **#840** — the portal puts a panel-internal `docsHref` link at the end of the tab order. Trigger: the first screen that passes `docsHref`. Both are linked from §15's adopting checklist and the decision record. fixes #734

timothy added 7 commits 2026-08-26 19:32:03 +02:00

feat(734): field-level progressive disclosure — shared FieldHelp trigger + panel ... 3b29865b9b

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 <button> beside a
  field name that opens a one-paragraph `role="note"` panel, with an optional
  level-3 `docsHref` deep link. Opens on three independent signals — tap (the
  only gesture a touch user has, and it pins), hover (wrapper-scoped so the panel
  stays reachable), and keyboard focus. Escape closes and returns focus without
  re-opening; an outside pointer press dismisses a pinned panel.
- Deliberately NOT an extension of `Tooltip`: that primitive is single-line,
  `nowrap`, `pointer-events: none` and hover/focus-only, so it can hold neither a
  paragraph nor a link and touch users never see it.
- Reference adoption on FFmpegProfilesScreen (9 consequential fields). `Row`
  gains `detail`/`docsHref`; copy lives in a module-level `FIELD_HELP` record
  next to the field definitions, interpolating existing bound constants rather
  than restating them. Two over-long level-1 summaries are cut to one sentence
  with the rest moved to level 2.
- No screen passes `docsHref` — the external docs do not exist yet; the
  affordance is built and typed, and exercised by the unit test.

The Escape-then-refocus regression (re-focusing the trigger re-opened what was
just closed) was found by its test before the fix and is pinned by it.

The label-activation assertion is on `defaultPrevented`, not on "the input did
not focus": a positive control confirmed jsdom does not implement label
activation at all, so the obvious assertion would pass vacuously.

fixes #734

fix(734): bridge the trigger→panel gap so a hovered panel stays reachable ... 6f6520f560

Measured live (dotnet host + built SPA, twelve-step mouse walk from the trigger
down into the panel): the panel disappeared at step four. `mouseleave` on the
wrapper does not fire while the pointer is over a descendant however it is
positioned, but the 7px offset between trigger and panel belongs to the ROW, not
to the wrapper — so the pointer leaves the wrapper in transit and the panel
closes under the cursor. That makes the level-3 `docsHref` link unreachable by
mouse, which is the whole point of the panel being interactive.

A transparent `::before` on the panel spans the offset, making the path
continuously a descendant. Re-measured after the fix: all twelve steps keep the
panel open, and a negative control (pointer moved 500px clear) still dismisses
it, so the bridge did not simply disable the close path.

refs #734

fix(734): the docs link's colour token never existed ... 64c030d2da

`var(--text-accent, var(--text-primary))` always took the fallback: the SPA has
no `--text-accent` token — the accent is `--ctv-accent`, defined per theme
(#e08a3c dark / #5b7cfa light) in the built stylesheet. So the level-3 "Learn
more" link rendered in body colour and read as prose rather than a link until
hovered. No screen passes `docsHref` yet, so nothing shipped wrong; the next
adopter would have inherited it.

refs #734

design(734): mirror the FieldHelp pattern into the FFmpegProfiles prototype ... d059647226

docs/design-sync.md step 7 — design-system/ changes land with the feature branch
so the repo and Claude Design stay mirrored. Adds a mockup `FieldHelp` (the two
opening signals that read in a static review: tap-pins and hover) plus the same
FIELD_HELP copy on the six rows the prototype actually renders. The shipped
component additionally handles Escape, outside-press dismissal and the hover
bridge; the prototype is a reference, never shipped code.

Parse-checked with the TypeScript JSX parser. The DesignSync push back to Claude
Design is main-session-only and needs the user's approval, so it is not done here.

refs #734

fix(734): portal the panel out of the card clip; correct three wrong copy claims ... f0da323a41

Cold adversarial review returned BLOCKED on four findings. All four fixed, plus
the reviewer's medium/low items.

BLOCKER — the panel was clipped away near a card bottom. `.ctv-card` sets
`overflow: hidden`, which clips a positioned descendant whatever its z-index, so
`z-index: 60` bought nothing. Measured on the reference screen: "Target
loudness" showed 12px of a 92px paragraph and "Scaling behavior" sat at exactly
0px of slack. Not an edge case — the Audio card's last row always carries a
`detail`, so one field's explainer was destroyed in every state of that card.

The panel is now portalled to `document.body` with `position: fixed`, placed
from the trigger's viewport rect in a layout effect that flips it above when it
would run off the bottom and clamps it inside the horizontal edges. Re-measured
across all seven rendered triggers: every panel fully visible, three flipped to
`top` automatically, none out of viewport. The portal cost the wrapper's
`mouseleave` as a proxy for "pointer still in this widget", so the panel now
owns its own hover state; the walk was re-measured on a flipped panel
(`[1]x12`, negative control 0).

Copy — three claims were wrong about the software they describe, which is the
exact drift §15 exists to prevent:
- loudnorm was compared to a dynamic-normalization mode this fork does not have
  (`NormalizeLoudnessMode` is `{ Off, LoudNorm }`).
- scaling was described as a binary choice, silently dropping `Stretch` — the
  only option that distorts the picture.
- `1..63` extra hardware frames were asserted known-bad, where
  `ffmpeg.qsv-extra-hw-frames-floor` states in terms that only `0` and `64` were
  measured and the rest are untested. The level-1 summary was already correct.
All three are corrected in the app and in the design mirror; a checker that
proves it read every key confirms the six shared strings stay byte-identical.

Also from the review:
- The `preventDefault` justification was false: per spec a label's activation
  behaviour is skipped for interactive-content targets, verified in Chromium.
  The call stays as belt-and-braces; the doc, the code comment and the test name
  no longer claim it fixes an observed bug.
- `role="note"` is not a live region and announced nothing. `aria-describedby`
  on the trigger, set only while open, is what reads the paragraph out.
- `aria-controls`/`aria-describedby` are omitted while collapsed rather than
  pointing at an id that is not in the document.
- Escape/outside-press are armed only when pinned or focused; armed on hover, a
  document-level Escape stole focus whenever a pointer rested on an info icon.
- The trigger joined the shared `--ring-focus` list instead of the UA default.
- `box-sizing: border-box` — the panel rendered 344px against a 320px max-width.
- `FIELD_HELP` is `as const`, so a mistyped key is a build error rather than a
  trigger with an empty panel.
- Added the missing decision record `spa.field-progressive-disclosure` and
  regenerated the catalog; §§8-13 each have one and §15 was the outlier.

Four new tests, each mutation-proved to fail for its own reason: portalling,
panel-owned hover, hover-scoped Escape, and collapsed-state aria. Restored
control green.

refs #734

fix(734): withdraw the hover bridge and say so; drop a fourth wrong copy claim ... 14d1fbaabd

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

fix(734): a fifth wrong copy claim, and the N5 replacement was vacuous too ... f3d003b8ac

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

timothy commented 2026-08-26 19:32:24 +02:00

Author

Owner

Copy Link

Copy Source

Review-verdict: MERGEABLE @ f3d003b

Four cold adversarial rounds (Opus, worktree-isolated, review-only). Rounds 1-3 BLOCKED: card-clip made one field's panel unreadable (12px of 92px); a hover bridge added then WITHDRAWN after a diagonal walk refuted it; five wrong copy claims across nine paragraphs; two vacuous tests for the same mechanism. All closed and independently re-verified at this head; the round-4 precondition (stale #839 body instructing a re-add of the withdrawn bridge) is corrected.

Review-verdict: MERGEABLE @ f3d003b Four cold adversarial rounds (Opus, worktree-isolated, review-only). Rounds 1-3 BLOCKED: card-clip made one field's panel unreadable (12px of 92px); a hover bridge added then WITHDRAWN after a diagonal walk refuted it; five wrong copy claims across nine paragraphs; two vacuous tests for the same mechanism. All closed and independently re-verified at this head; the round-4 precondition (stale #839 body instructing a re-add of the withdrawn bridge) is corrected.

timothy referenced this pull request 2026-08-26 19:40:09 +02:00

One shared verdict vocabulary instead of two shell copies plus a parity test #788

timothy merged commit 40a3232d9e into main 2026-08-26 20:05:54 +02:00

timothy deleted branch feat/734-field-progressive-disclosure 2026-08-26 20:05:56 +02:00

timothy referenced this pull request 2026-08-26 20:06:39 +02:00

SPA: field-level progressive disclosure — short summary → hover popup → (future) deep-dive docs link #734

timothy referenced this pull request 2026-08-26 21:00:33 +02:00

Validate FFmpeg profile numeric fields to sane ranges instead of silently transforming them, and expose readrate pacing as a bounded field #735

timothy referenced this issue from a commit 2026-08-26 22:36:01 +02:00

fix(788): prove the sentinel is load-bearing, and correct three claims

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

ad-hoc

One-off / ad-hoc work not tracked by a dedicated issue

api

REST API / HTTP endpoints

bug

Something isn't working

ci-cd

Build, test, deploy pipeline

content

Channel content / schedules / playlists

dependencies

Dependency updates (Renovate)

enhancement

New feature or improvement

frontend

ChicoryTV React SPA frontend

in-progress

Claimed by an active session — do not pick up

jellyfin

Jellyfin tuner / IPTV integration

parked

Excluded from automatic queue pickup; work only when explicitly selected

priority: high

priority: low

priority: medium

review

Adversarial review finding

security

Security / vulnerability fix

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

renovate timothy (Timothy)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: timothy/ersatztv#841