Commit Graph
3 Commits
Author SHA1 Message Date
peter 41b652f8fd fix(ui): a dialog opens focused where it said it would, and the wizard is a dialog
Deep-review round 5, over round 4's own fixes.

- Round 4 gave `ui/Modal` a persistent portal host so a fullscreen transition would stop tearing
  the dialog down. The host was attached in a `useLayoutEffect` — one commit phase AFTER React
  runs `autoFocus` in `commitMount`, and `focus()` on a disconnected node is a no-op. Every
  autoFocus in every dialog was dead: Mod+K opened the command palette with the focus on the ✕,
  so typing searched nothing and Enter closed it; the same for the download rename field and
  every non-danger confirm. The host is attached in a `useInsertionEffect` now, a whole phase
  earlier, and the whole re-parent — including the focus save/restore across the move, which
  `appendChild` needs because it is remove-then-insert — lives in one `useFullscreenPortalHost`.
  `ui/AnchoredPopover` had a second copy of that re-parent WITHOUT the focus restore, and a
  comment claiming focus survived it; both now come from the hook.

- `ui/Toaster` classified a toast raised in the SAME commit as a host takeover among the ones it
  inherits, because it read the store live at the takeover render. A dialog that notifies and
  closes in one handler (UploadDialog) therefore put its toast up with no role, into a region
  still `aria-live="off"` — announced by neither path. "Inherited" is now what the outgoing host
  last had on screen, and a toast that is NOT inherited is announced even inside the unarmed
  window.

- The onboarding wizard renders its own shell, and Escape ordering was all of the dialog contract
  it had: no role, no label, no initial focus, no trap, no restore. A keyboard user tabbed out of
  the first-login wizard into the app behind the scrim. The focus half of the contract is now one
  shared `ui/dialogFocus` that both `ui/Modal` and the wizard use.

Every fix has a regression test, each mutation-checked (6 mutations, all red on the assertion
they are named after).
2026-09-12 03:41:37 +02:00
peter 2756c21dbd feat(popover): fullscreen-aware portal + migrate the hover surfaces (R8 S4a)
The platform's FloatingPortal targeted <body>, which is painted UNDERNEATH whatever
holds the fullscreen lock. That is why the player's in-bar menus were either
non-portaled (and clipped) or, for add-to-playlist / download, disabled outright in
fullscreen. PopoverSurface now portals into `document.fullscreenElement` while one
exists, via a new reactive `useFullscreenElement()` on the existing WebKit-aware
fullscreen lib.

Two floating-ui details this had to work around: a null `root` means "the container
does not exist yet" (no portal node is created at all), so the no-fullscreen case must
pass undefined; and the portal node is built once and ignores a later root change, so
the portal is keyed on the fullscreen state to remount on a transition.

Migrated the three hand-rolled hover engines onto the platform:
- Tooltip: kept as a passive caption via a new `passthrough` prop on PopoverSurface,
  so it cannot swallow a click aimed at a control it overlaps during the close delay.
  Gains aria-describedby wiring it never had.
- AboutCell: the 160 ms hide-timer hover bridge is now safePolygon; the close-on-page-
  scroll is now hide-and-follow, so the card reappears when the row scrolls back.
- TagManager: same bridge removal; the scroll-fade moves to an inner scroller so the
  mask rides the scrolling box rather than the glass panel.

Verified in-browser: tooltip flip + centering + pointer-transparency, About card flip-
above with the measured height, tag popover right-start anchoring, and the portal
round-trip body -> fullscreen element -> body on a live fullscreenchange.
2026-07-30 23:49:29 +02:00
peter bdf7c03b59 fix(r5): address the eighth /code-review high round — 7 findings
The worst one was, again, the previous round's own fix.

- Round 7 stopped awaiting the HLS sweep so startup could serve immediately — which
  also made it run CONCURRENTLY with playback, and `sweep_orphans` snapshotted the
  live-session set BEFORE listing the root and rmtree'd outside the lock. Since
  `start_session` reuses a directory path for the same (key, offset, tag), a stale
  name can become a live session at any moment. Two guards now: nothing whose mtime
  is newer than this process's start is ever deleted, and the live check + rmtree
  happen together under the lock, one directory at a time.
- The sweep task is awaited after cancel, so shutdown can't leave it pending.
- `_rate_limiter` used a truthiness test on its timestamp, so a `now()` of exactly
  0.0 read as "never ran" — unreachable in production, but `now` exists to be
  injected and a test clock starting at 0 is the obvious choice.
- The RSS total-outage error now carries the first underlying error: "all N channels
  failed" is also what ONE dead feed id looks like on a small instance, and a red
  card the user can't act on is its own kind of dishonest.
- The WebKit fullscreen handling moved into `lib/fullscreen` and PlexPlayer uses it
  too — it had the same unprefixed-only reads in three places, including an Escape
  branch that would have closed the whole player instead of leaving fullscreen.
- The `--max` CSS comment claimed `inset: 0` did the sizing; Tailwind's `w-full` is
  still on the element and wins. Comment now states the real mechanism.

Verified in real Chrome (the pane can't composite or do native fullscreen):
- F fills the 1920x889 viewport; the exit pill renders top-right on the letterbox
  band and overlaps NO YouTube control — the collision this round suspected does not
  reproduce in this embed (its own buttons sit bottom-left / bottom-right).
- Four real ArrowDown presses while maximised: persisted volume 100 -> 80, with the
  level flash visible on screen.
- A real click on the exit pill un-maximises and returns focus to the card.
- lib/fullscreen driven from a real click: enter (innerHeight 889 -> 1024), the
  change listener fires exactly once per transition, exit returns to 889.

Backend suite 150 -> 158; the new sweep tests are mutation-checked, and they read
their cutoff from the filesystem rather than a clock (the container VM and the
bind-mounted host disagree by enough to make a clock-based assertion flaky).
2026-07-24 04:44:20 +02:00