feat(web): edit an entry's section — type it, drag it, or capture it from the player - #306
Open
Sawtaytoes wants to merge 8 commits into
Open
feat(web): edit an entry's section — type it, drag it, or capture it from the player#306Sawtaytoes wants to merge 8 commits into
Sawtaytoes wants to merge 8 commits into
Conversation
…here `lib/section.ts` is the pure half of the section editor: it reads `start.position_ms` and `end.position_ms` off an entry as one `Section`, and it prints the three tag readings and the panel summary. Two rules run through the whole file and each has already cost something somewhere: * `0` is a real offset — a section that begins on the first frame — so every test is `!= null` and never truthiness. `Number(null)` is `0`, which is how a cleared window reads as a section at offset zero; the server's own `hasSection` was written the wrong way first for exactly this. * An open end is the ABSENCE of a choice, not `0` and not the runtime. Four states, all of them real. `withUnit` / `withPositionMs` split the shared `start` mapping in two. `PATCH …/start` replaces that mapping whole, and the entry sheet edits WHICH unit plays and WHERE IN IT in separate rows, so each writer has to carry the other's field through. Without it the start picker deleted a section every time somebody changed the episode, and nothing on screen would have said so. `startLabel` now answers "" for a start that names no unit. A film section is exactly that — a position and nothing else, because a film has no season and no episode — and the tag used to read "Start set", which names nothing. `nowPlayingPositionMs` is the browser's ONE converter from the now-playing payload's seconds to the milliseconds everything else speaks; `finished.ts nowPlayingMs()` is the server's. The Now-playing bar's scrubber and the section editor's capture buttons ask the same question, and two extrapolators that drift would put a mark somewhere the bar never showed. `tileFace.clock` and `NowPlayingBar.toClock` are now `formatTimecode` from `@charcuterie/ui` — two of the five hand-rolled printers the library counted across the fleet. The one visible change is a padded minute below an hour. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A queue's registry row carries `provider_kind`, not what that backend can actually do, so the browser has to join the two. `GET /api/providers` already answers `plays_sections` per provider — reported without instantiating one, so it works for a backend whose token was never configured. One fetch for the life of the page, in a module store like `overlays.ts`: the answer outlives a view, several unrelated components ask for it, and it is not part of the payload `revalidate()` swaps in and out. A per-tile request would put an uncached read back on a page that took a lot of work to get to zero. `usePlaysSections` answers FALSE while the list is in flight and false for a kind this build has not heard of. No control beats a control nothing serves. The check is the declared capability and never `kind === "plex"` — a UI that branches on a backend's name has to be edited again for every future backend. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The control the section feature was missing. `start.position_ms` and `end.position_ms` have been stored, served and played since #302 and #304; this is where a person sets them. A TAG on the tile, reading differently in each of the three states worth showing and showing nothing at all when there is no section — the rule every tag here follows. Beside the start tag rather than instead of it: they answer two different questions, WHICH unit plays and WHERE IN IT, and an episode entry can carry both. A ROW in the entry sheet, shaped like the start-point row: a summary, a Choose…/Change… that opens the modal, and a clear right there in the row. A MODAL with all three ways in, over one pair of numbers: * type it — a `TimecodeInput` in `isRange` mode, bounded by the runtime; * drag it — a `RangeSlider` over `0..duration`, `onChange` to paint and `onChangeEnd` to commit; * capture it — "Start here" / "End here", reading the live playback position. So a drag moves the fields and a typed value moves the thumbs. `valueMs` SEEDS, so the fields are re-keyed on the other writers and never on their own value — keying on the value would remount the control under a live caret. Four judgements worth naming, three of which are traps: * AN UNKNOWN RUNTIME DROPS THE BAR, and must. The wire sends `0` for "not known", and `durationMs={0}` on a `TimecodeInput` clamps every mark to zero. So it becomes `undefined`, the field takes no upper bound, the slider is replaced by a line saying why, and the two routes that need no scale still work. * DRAGGING ONE THUMB MUST NOT CLOSE THE OTHER END. An open end has to be drawn somewhere, so a `RangeSlider` reports `start: 0` while you drag the end of a start-open section. Each end is adopted only when it left where it was drawn. * THE CAPTURE BUTTONS ARE DISABLED, NOT ABSENT, and say which of three reasons is in force. This is a transient state, not a missing capability: the buttons come alive the moment this entry is on screen, and a control that is simply gone teaches nobody that playing the item would bring it back. * WHICH WRITE GOES FIRST IS NOT A STYLE QUESTION. `start` and `end` are two routes and the pair rule lives on the writers, so moving a window later and writing the start first lands it against the stored end and is refused. `end` first is safe whenever the new end sits after the stored start; when it does not, the window moved earlier and `start` first is safe. Only the end that actually moved is written. A refused window comes back as `{ok: false, error}` at HTTP 200, so `api()` resolves and nothing throws — the commit reads the body, or it would report "Saved" over a file it did not change. The modal names the consequence a reader hits first: a windowed entry on `watch_history: provider` never completes and replays its section every sitting, because Plex judges a part-played item as unwatched. Deliberate, and changed one step away in the entry sheet. The whole thing is gated on `plays_sections`, so a reading queue and a board-game queue show no section control at all — not a disabled one. `#sectionmodal` is named in every `app.css` modal-chrome list. `.subhint`, `.field` and `.idnote` are each scoped to a modal id in this file, so a new modal that is not named renders unstyled while looking styled in the source. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two gaps the section editor leaves open on its own. THE FILTER LIED. `hasOverrides()` read `Boolean(it.start)`, and an entry that only says where to STOP carries no `start` at all — so "Has overrides" hid it while claiming to show every entry that deviates from its queue. It reads the section now, both ends. A "Plays a section only" filter joins the toolbar's state list, offered only where a section can exist. "IN THIS QUEUE" WAS A DEAD END. That refusal is right for the ordinary case — an accidental second copy of a film in a watch queue is a bug, and the badge is what caught it — but the demo reel this feature exists for plays three windows of one film, which is three lines. So the badge keeps its place and gains an explicit "Add another" beside it, which is the one call site that sends `allow_duplicate: true`. It paints after the answer rather than before it, and that is forced rather than chosen: the server MINTS an `id` for the new line and that id is what keys it, so the browser cannot name the tile before it asks. The same round trip is what makes the next step work — a second line exists to carry a second section, so the entry sheet opens straight onto it rather than leaving somebody to work out which of two identical tiles is new. Offered only where a section can be played, because that is the whole reason to want a second line. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Twenty-seven checks over the tag, the panel row, the modal and the write. Every one is either one of the four optionality states or one of the two ways `null` gets confused with `0`, which is the whole class of bug this feature can have. The tiles are synthetic — this repo is public, so the gate and its screenshots show invented films and never a real library. The PATCHes are real: they continue through to the server the harness spawned, so the YAML on disk is the record of what the app wrote. Browser, but no Plex, so it runs on every pull request.
Three defects the screenshots found, none of which a unit test could have. 1. "Plays Plays the whole item." The default state returned a whole sentence while the other three returned a fragment, and the modal puts its own verb in front — "Will play" for a draft, "Plays" for what is stored. `sectionSummary` now returns a fragment in all four states and both call sites supply the verb. 2. The tag was cut off in the Narrow View. `Section 01:01:00–01:…` names which end was lost and nothing about where the section stops, and the numbers are the whole meaning of this tag. It wraps instead, through `Badge`'s own `overflow` prop. Every other tag keeps truncating, which is right when the text is a word. 3. Two `hh:mm:ss.mmm` fields side by side did not fit a 390px modal, so the value read `01:01:00.00`. A smaller type size in the Narrow View, not a layout change: the two-across arrangement belongs to `TimecodeInput`. The browser gate also polls for the cleared tag rather than sleeping. A clear is two writes and a refresh, and a fixed wait passed most of the time.
The dragged bar, a mark captured from a live player, the "Add another" door, the light scheme and the Narrow View — nine screenshots, each driven to its state by a real pointer or a real click rather than seeded. `Locator.press` joins the hand-written Playwright slice: `Enter` is what commits a `TimecodeInput`, which is the only field in the app where typing alone deliberately does not write. AGENTS.md gains the new gate and records `.addanother` as a third DOM handle that carries no rule, beside `.playbtn`.
Eighteen captures, each driven to its state in a real browser. The films are invented and the queue is the harness fixture — this repo is public, so no screenshot may show the real library. The two "before" images are shot on the merge base. They are what made the case: an entry that already carried `start.position_ms` wore a tag reading "Start set", which names a setting without naming the setting, and its entry sheet had no way to reach the value at all. A pull request outlives the session that opened it, so these live in the branch rather than behind a temporary share.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Builds the part a person actually touches.
start.position_msandend.position_mshavebeen stored (#302) and played (#304) since last week; until now nothing on screen said so.
Design and decisions: #299. The duplicate-line id this leans on: #300.
Before
An entry that already carried a section wore a tag reading "Start set" — a label that
names a setting without naming the setting — and its entry sheet offered no way to reach the
value at all.
After
The tag reads differently in each state, and says nothing when there is none
All four states of the decision's table are reachable and clearable: neither, start only,
end only, both. An entry with no section wears no tag, which is the rule every tag here
follows.
The entry sheet gains a Section row
A summary against the runtime, a Change… that closes the panel and opens the editor, and
the clear right there in the row.
The editor — three routes over one value
Typed is
TimecodeInputinisRangemode, bounded by the runtime. It owns its own echoand its own refusals, and a refused value stays exactly as typed.
Dragged is
RangeSliderover0..duration.onChangepaints andonChangeEndcommits,so a drag does not remount the fields dozens of times.
Captured reads the live playback position. The buttons are disabled rather than absent —
this is a passing state, not a missing capability — and the note under them names which of
three reasons is in force: nothing is playing, something else is playing, or no position has
been reported yet. When it can capture it shows the position it would take.
Two decisions worth calling out
An unknown runtime (
duration: 0) drops the bar rather than clamping to zero. A sliderneeds a scale, and inventing one would draw a proportion that is not true of the item. Passing
durationMs={0}to aTimecodeInputwould clamp every typed mark to zero, so0becomesundefinedand the field takes no upper bound. Typing and capture still work; the modal sayswhy the bar is gone.
A backend that cannot play a section shows no control at all — absent, never disabled. The
gate is the provider's declared
plays_sections, neverkind === 'plex'. A reading queue anda board-game queue get no tag and no row.
A second section of the same film is reachable
"In this queue" is the right refusal for the ordinary case — an accidental second copy of a
film is a bug, and that badge is what caught it. A demo reel that plays three windows of one
film is the case where a second line is the point, so the refusal keeps its badge and gains a
door: Add another sends
allow_duplicate: true, paints the line the server minted, andopens its settings straight away.
Light scheme, and the Narrow View
The tag wraps rather than truncating, because its meaning is entirely in two timecodes and
Section 01:01:00–01:…says only which end was lost. Every other tag keeps truncating, whichis right when the text is a word. The two
hh:mm:ss.mmmfields take a smaller type size at390px so the value reads in full — a type-size change, not a layout change, because the
two-across arrangement belongs to
TimecodeInput.A data-loss bug found on the way
PATCH …/items/<key>/startreplaces the whole mapping, andStartModal.readForm()returnedonly the unit fields. Changing an entry's episode would have deleted its section's
position_mswith nothing on screen to notice.withUnitandwithPositionMssplit theshared mapping so each writer carries the other's field through, and
section.test.tspinsboth directions.
Two more that came out of the same seam:
{ok: false, error}at HTTP 200, so nothing throws.commitSectionreads the body, or it would report "Saved" over a file it did not change.
startfirst lands it against the storedendand isrefused.
isEndFirst()picks the safe order, and only the end that actually moved is written.Housekeeping
tileFace.clockandNowPlayingBar.toClockwere two hand-rolledcopies in this repo; both are gone, replaced by
formatTimecodefrom the library.everything else speaks milliseconds.
nowPlaying.nowPlayingPositionMs()is the browser'ssingle extrapolator, matching
finished.ts nowPlayingMs()on the server — the scrubber andthe capture buttons ask the same question, and two that drifted would put a mark somewhere
the bar never showed.
hasOverrides()and the toolbar state filter know about a section, or the "has overrides"filter lies. The filter gains a "Plays a section only" option, offered only where a section
can be played.
Server
No server change was needed, and no field was missing.
queueTilealready sends bothstartandend, andProviderInfo.plays_sectionswas already on/api/providers. The onlyjoin the browser had to make itself was
RegistrySet.provider_kind→ the provider list →plays_sections, which is what the newstate/capabilities.tsstore does, in one request forthe life of the page.
Gates
e2e/section-ui-test.tsis a browser gate with no Plex, so it runs on every pull request.Its PATCHes continue through to the server the harness spawned, so the YAML on disk is the
record of what the app wrote.
Already failing on the merge base and untouched here — all five report
[store] YAML import failed: unable to open database file, identically on630701f:topup-test,session-profile-gate-test,rotation-length-test,sse-test,binding-token-test.tile-menu-testis flaky on the merge base too (3 fails in 4 runs there,2 in 3 here) — its optimistic lane paint has to survive an SSE-triggered refresh inside a
1200 ms window.
Not done here
The
TimecodeInputecho restates the start field whenever it holds text, even when the endis the field being edited, because the component derives it from
texts[activeEndpoint]andthat defaults to
"start". It is the library's behaviour and reads as a stray timecode underthe fields. Left alone deliberately: a fix belongs in Charcuterie, not in a local
reimplementation of a component that already owns its echo.
🤖 Generated with Claude Code