VersionControl:PRs/rabbithole/9: Difference between revisions
From PickiPedia: A knowledge base of bluegrass, old time psychedelic jams, and other public domain music
Jump to navigationJump to search
cryptograss/rabbithole#9, as GitHub has it |
cryptograss/rabbithole#9, as GitHub has it |
||
| (2 intermediate revisions by the same user not shown) | |||
| Line 2: | Line 2: | ||
|repo=rabbithole | |repo=rabbithole | ||
|number=9 | |number=9 | ||
|title=Player: | |title=Player: one ensemble element, rendered into for every song | ||
|url=https://github.com/cryptograss/rabbithole/pull/9 | |url=https://github.com/cryptograss/rabbithole/pull/9 | ||
|author=magent-cryptograss | |author=magent-cryptograss | ||
|state= | |state=merged | ||
|base=main | |base=main | ||
|branch=fix/ensemble-twice | |branch=fix/ensemble-twice | ||
|opened=2026-11-05 05:05:02 | |opened=2026-11-05 05:05:02 | ||
|pushed=2026-11-05 | |pushed=2026-11-05 19:51:17 | ||
|merged= | |merged=2026-11-05 19:55:45 | ||
|closed= | |closed= | ||
|worked_on_by=magent, skyler, justin | |worked_on_by=magent, skyler, justin | ||
| Line 16: | Line 16: | ||
|deploy=pickipedia | |deploy=pickipedia | ||
}} | }} | ||
<pre class="pr-body">Skyler saw every musician card twice in Firefox. Two fixes before this one removed the duplicate or guarded the timer; Justin asked why the ensemble isn't simply one container whose contents are replaced. It is now. | |||
**What changed (`src/player/rabbithole-player.js` only):** | |||
- The ensemble element and its Packery grid are created once, when Webamp's window first exists (`createEnsembleAfterMainWindow`). `this.ensembleEl` holds it for the life of the player. | |||
- One card builder, `renderEnsembleCards(container)`, used by both modes. It replaces the container's contents with a card per musician in the current track data. | |||
- `loadNewSong` calls `renderSong()`: cards, parts strip, Packery `reloadItems()`, state at 0:00. Nothing is removed, nothing is scheduled. A song loaded before the element exists is picked up by the build, which reads `trackData` when it runs. | |||
- Gone, because the rebuild they existed for is gone: `isLoadingNewSong`; the 100-try poll for cards that had just been appended synchronously; the `innerHTML` rewrite in `updateMusicianCard` (so a card's children, including the rabbit icon and any click handler, persist; the two hand re-adds of the rabbit go with it); a new MutationObserver per song load (one now, disconnected in `cleanup()`); and the unused `oldPositions` measurement that threw when `#ensemble-display` was absent. | |||
- `currentSolo` is normalised to `null`, so "no solo" to "no solo" no longer counts as a solo change on the first tick. | |||
**Checked on pickipedia.xyz** with this branch's player routed in: | |||
- first load: 1 ensemble, 5 cards, 21 parts, Packery-positioned | |||
- `loadNewSong` twice 20ms apart: still 1 ensemble, 5 cards, same element (was 2 and 10) | |||
- a real switch to Victory through the pane's Listen button: same element, 6 cards, 17 parts | |||
- play into Kyle Tuttle's solo: lead class on Kyle, pane follows to Kyle Tuttle | |||
- no page errors other than Webamp's own ReadableStream one, which appears on main too | |||
**Tests:** 4 new in `tests/player/ensemble.test.js` with a toy DOM (render into the element, two loads leave one, a load before the element waits for the build, a card update keeps its children). 89/89 pass. | |||
This is the first of the three structural changes in [26350517 Rabbithole player audit](https://pickipedia.xyz/wiki/26350517_Rabbithole_player_audit). Card-click delegation (audit item 6) waits for #10 to merge, since it touches the same lines. | |||
Deploy: pickipedia | |||
Worked on by: magent, skyler, justin, in #rabbithole</pre> | |||
Latest revision as of 19:06, 9 November 2026
| State | 🟣 merged 2026-11-05 19:55:45 |
|---|---|
| Worked on by | magent, skyler, justin |
| Moods | #rabbithole |
| Deploy | pickipedia |
| Branch | fix/ensemble-twice → main, by magent-cryptograss
|
| Opened · last pushed | 2026-11-05 05:05:02 · 2026-11-05 19:51:17 |
Skyler saw every musician card twice in Firefox. Two fixes before this one removed the duplicate or guarded the timer; Justin asked why the ensemble isn't simply one container whose contents are replaced. It is now. **What changed (`src/player/rabbithole-player.js` only):** - The ensemble element and its Packery grid are created once, when Webamp's window first exists (`createEnsembleAfterMainWindow`). `this.ensembleEl` holds it for the life of the player. - One card builder, `renderEnsembleCards(container)`, used by both modes. It replaces the container's contents with a card per musician in the current track data. - `loadNewSong` calls `renderSong()`: cards, parts strip, Packery `reloadItems()`, state at 0:00. Nothing is removed, nothing is scheduled. A song loaded before the element exists is picked up by the build, which reads `trackData` when it runs. - Gone, because the rebuild they existed for is gone: `isLoadingNewSong`; the 100-try poll for cards that had just been appended synchronously; the `innerHTML` rewrite in `updateMusicianCard` (so a card's children, including the rabbit icon and any click handler, persist; the two hand re-adds of the rabbit go with it); a new MutationObserver per song load (one now, disconnected in `cleanup()`); and the unused `oldPositions` measurement that threw when `#ensemble-display` was absent. - `currentSolo` is normalised to `null`, so "no solo" to "no solo" no longer counts as a solo change on the first tick. **Checked on pickipedia.xyz** with this branch's player routed in: - first load: 1 ensemble, 5 cards, 21 parts, Packery-positioned - `loadNewSong` twice 20ms apart: still 1 ensemble, 5 cards, same element (was 2 and 10) - a real switch to Victory through the pane's Listen button: same element, 6 cards, 17 parts - play into Kyle Tuttle's solo: lead class on Kyle, pane follows to Kyle Tuttle - no page errors other than Webamp's own ReadableStream one, which appears on main too **Tests:** 4 new in `tests/player/ensemble.test.js` with a toy DOM (render into the element, two loads leave one, a load before the element waits for the build, a card update keeps its children). 89/89 pass. This is the first of the three structural changes in [26350517 Rabbithole player audit](https://pickipedia.xyz/wiki/26350517_Rabbithole_player_audit). Card-click delegation (audit item 6) waits for #10 to merge, since it touches the same lines. Deploy: pickipedia Worked on by: magent, skyler, justin, in #rabbithole