Skip to content

refactor: address review findings #51-54, #59#92

Merged
silverbucket merged 1 commit into
masterfrom
t3code/cleanup-review-findings
Apr 28, 2026
Merged

refactor: address review findings #51-54, #59#92
silverbucket merged 1 commit into
masterfrom
t3code/cleanup-review-findings

Conversation

@silverbucket
Copy link
Copy Markdown
Owner

Summary

Five small, independent cleanups from the comprehensive review.

  • App.svelte: dieColor / customDieColor derives are duplicates; else-branch is dead #54App.svelte: collapse dieColor / customDieColor derives into one. The else branch on the CSS-variable effect was dead code (the --accent* properties are never set when customDieColor is null in the first place).
  • iOS PWA scroll-nudge re-runs on every account swap #59App.svelte: latch the iOS-PWA scroll-nudge so it only fires the first time initialSyncComplete flips to true. Account swaps re-flip it true → false → true, so each swap was triggering another (harmless but wasted) nudge.
  • Dead code: setlistViewVersion / {#key} forced re-mount #51RollScreen.svelte + stores/app.svelte.js: drop the {#key store.setlistViewVersion} wrapper and the matching setlistViewVersion state. Svelte 5's reactivity already updates the keyed {#each} cleanly when generatedSetlist changes — no need to tear down and rebuild the whole result subtree on every roll. Audit notes:
    • SetlistSongCard's expanded state now persists across rolls when the same song happens to repeat (keyed by song.id). Reasonable UX.
    • .result-section's pop-in animation now only runs the first time the section mounts (i.e., the first roll after an empty state). The dice button's landed bounce + confetti still gives feedback on every subsequent roll.
  • Drop or inline scopedKey now that accountSlot is the API #53accounts.js: make scopedKey module-private. accountSlot(address).key(base) is the public API. Tests rewritten to exercise the same behavior through accountSlot.
  • Bloat: hex helpers in utils.js share validate→parse pattern #52utils.js: extract a parseHex(hex) → [r, g, b] helper that internalizes the validate-and-fallback pattern, drop the recursive fallback in each formatter, and add a toHexByte helper for darkenHex.

Closes #51, #52, #53, #54, #59 once merged.

Test plan

  • npm run lint — clean
  • npm test — 276/276 pass
  • npm run build — succeeds
  • npm run test:e2e — 136/136 pass

@silverbucket silverbucket self-assigned this Apr 27, 2026
- App.svelte: collapse dieColor/customDieColor into one derived; the
  else branch was dead code (#54)
- App.svelte: latch the iOS PWA scroll-nudge so it fires once per
  session, not on every account swap (#59)
- RollScreen + store: drop the {#key} block + setlistViewVersion;
  Svelte 5 reactivity already handles fresh-roll updates without a
  forced subtree remount (#51)
- accounts.js: make scopedKey module-private; accountSlot is the
  public API now (#53)
- utils.js: extract parseHex shared helper, drop the recursive
  fallback in each formatter (#52)
@silverbucket silverbucket force-pushed the t3code/cleanup-review-findings branch from 1168dd8 to efb56bc Compare April 28, 2026 22:04
@silverbucket silverbucket merged commit ace0374 into master Apr 28, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dead code: setlistViewVersion / {#key} forced re-mount

1 participant