diff --git a/.planning/NOTES.md b/.planning/NOTES.md index b7475e3..db33651 100644 --- a/.planning/NOTES.md +++ b/.planning/NOTES.md @@ -2264,3 +2264,65 @@ Six more things worth keeping: question. Same shape as `getCoverUrl()`, `track-index.ts` and `page-header` — when a rule is written per call site, the call sites do not disagree, they are all incomplete in the same way. + +## A decision phase earns its keep by finding it was not a decision + +Plan 009 phases 2 and 3: the badge becomes a button where it can act. + +The generalisation: **the questions worth taking a phase over are the +ones the code can answer, and you cannot tell which those are without +asking them.** Phase 2 was written as three judgement calls. Two turned +out not to be: + +- "An artist badge would commit a user to a whole discography" — + describing a badge that **does not exist**. `top-results-row` renders + `nothing` for an artist and no other site passes + `entity-type="artist"` to the component at all. Artist subscription + already had a labelled Follow button. +- "Should a track inside a requested album show something different" — + evaporated. It read as noise only while a plus on a track meant + nothing; once it means *want just this one*, the mixed row is the + interface working. + +The third — whether a track can be requested at all — went the other +way and is the more useful lesson. **`EntityRecording` reads like a +placeholder and is load-bearing.** It would have cost nothing to rule +tracks out as unsupported, and `Reconciler.tracklistFor` has an +explicit branch for them whose comment explains that a one-entry +expected tracklist is what lets filename matching score a single-track +download at all. A feature removed by assumption leaves no trace that +it was ever there. + +Five more things worth keeping: + +- **A test that passes on the neutered build is not a test, and the + vacuous ones are the negative assertions.** "Keeps its click off the + card it sits on" asserted that nothing bubbled — free when there is + no button, since `?.click()` on null is a silent no-op. It passed on + the neutered build while its seven neighbours failed. It asserts the + click *did the thing it was swallowed for* as well now. Same family + as `overflow: hidden` permitting programmatic scrolling, and the tell + was identical: **it could not fail.** +- **A measured coordinate is stale before it is used.** The e2e gesture + read a bounding box the moment the search settled; cover art is still + arriving then and a card that grows moves the badge, so the click + landed on the card and opened the album — reported as *a failure to + file a request*, which is a different bug. A Playwright locator + re-resolves and waits for the element to stop moving. Prefer one to + `mouse.click(x, y)` whenever the thing being clicked is in a list + that is still loading, which is most lists here. +- **A fix moves its own assertions, and that is not churn.** Phase 1's + spec asserted the badge announced "… is queued for download". A + *control* is named after what activating it does, so two commits + later it is "Cancel the request for …". Naming a thing after its + state is correct right up until it grows an action. +- **An opt-in makes a redundancy visible.** The badge could have known + which pages have a "Want this" button; instead a call site passes + `request-mbid` or does not, so `explore-album-details`'s header + declines in its own template. The rule is greppable and the component + has no list of exceptions to go stale. +- **Verify a control with the gesture, not with the event.** A synthetic + `MouseEvent` does not prove hit-testing, and a `.click()` on a shadow + child does not prove the icon inside it is `pointer-events: none`. + Both were checked with a real mouse (`mousemove`/`mousedown`/ + `mouseup`) and a real Tab/Enter before either was believed. diff --git a/.planning/plans/active/009-the-badge-that-cannot-act.md b/.planning/plans/active/009-the-badge-that-cannot-act.md index dda57a9..97a287d 100644 --- a/.planning/plans/active/009-the-badge-that-cannot-act.md +++ b/.planning/plans/active/009-the-badge-that-cannot-act.md @@ -1,6 +1,6 @@ # 009 — The badge that cannot act, and the state it already had -**Status:** active — Phase 1 shipped; Phase 2 is a decision, not written yet. +**Status:** complete — all three phases shipped. **Branch:** main **Created:** 2026-08-13 **Follows:** 008-the-last-audit @@ -165,27 +165,112 @@ Six things, and the first is the plan's own framing. ## Phase 2 — what a badge click means, per entity -*(decision, before code — not started)* +*(Decided 2026-08-13, before any code.)* -What Phase 1 leaves for it, now as observations rather than guesses: +**A badge is a button where it is the only way to act, and what it +toggles is a request — never a download.** -- On the album page the badge and the "Want this" button now say the - same thing twice, four centimetres apart. That is an argument for the - badge being **read-only there** and clickable only where there is no - button — or for the button going. -- A requested album shows an amber hourglass while every track in its - tracklist shows a plus, which is correct per the rule and reads as - busy. Worth deciding whether a track inside a requested album should - render *nothing* rather than a plus. -- An artist badge would mean a discography subscription, which is the - heaviest commitment in the download subsystem behind the smallest - control in the app. +Two of the three questions were answered by the code rather than by a +judgement, which is the point of asking them before writing anything. + +**There is no artist badge, and there never was.** The worry that one +20 px circle would commit a user to a whole discography does not apply: +`top-results-row` renders `nothing` for an artist, and no other site +passes `entity-type="artist"` to this component at all. Artist +subscription already has a home — `explore-artist-details`'s +`renderFollowAction()`, a labelled button with the scope beside it, +which is where a commitment that never completes belongs. + +**A track badge is honoured end to end.** `EntityRecording` is not a +placeholder in the request model: `Reconciler.tracklistFor` has a +deliberate branch for it ("A track request is its own tracklist") whose +comment explains that the single expected title is what lets filename +matching score a one-song download at all. So a track badge promises +something the backend can keep, and it is a button too. + +That also disposes of the second observation. An hourglass on an album +over a row of plusses read as noise while a plus meant nothing; once a +plus on a track means *want just this one*, the mixed row is the +interface working. No special case, and none of the four surfaces needs +to know what contains what. + +**The album detail header keeps its badge read-only.** "Want this" sits +directly below it saying the same thing in words. The rule is not "a +badge is decorative on detail pages" — it is that a call site **opts in +by supplying the MBID to act on**, so a redundancy is visible in the +template rather than hidden in the component. + +**And it is a request, not an acquisition.** The old copy said "Add … +to library", which 007 called the button's promise written into the +copy — and it would still be a lie, because clicking adds a row to the +request list and nothing to the library. The name is the action, in the +words the rest of the app already uses: **"Want …"**, and **"Cancel the +request for …"** when it is already wanted. No confirmation: the action +is one click to undo, which is the whole test for whether a dialog is +owed. --- ## Phase 3 — the button -*(scope depends on Phase 2)* +Ships what Phase 2 decided: `request-mbid` as the opt-in, a `