Stop the release-notes blink on variant clicks#24
Conversation
Two related fixes in js/views/device.js: - The variant click handler had no same-variant guard, so clicking the already-active variant still ran variant = ...; epoch++; and re-rendered everything, including a wasted GitHub API fetch. Now it no-ops when the clicked variant is already selected. - renderReleaseNotes() blanked #release-slot up front before its async fetch resolved, so the box went empty for a network round-trip on every render (the visible flash). The old notes now stay in place until the fetch resolves and swaps them in; the existing epoch guard still prevents stale writes. Added two Playwright tests in tests/installer.spec.js that fail without these fixes: same-variant clicks make no extra release-notes fetch, and #release-slot never goes empty while switching variants. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
WalkthroughThe device view now skips redundant same-variant refreshes and keeps existing release notes visible during asynchronous variant updates. Two Playwright tests verify request counts and prevent the release-notes slot from becoming empty. ChangesVariant release-notes behavior
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
js/views/device.js (1)
251-256: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winPrevent redundant re-renders on same-channel clicks.
Just like the newly added early return for same-variant clicks, you can also prevent unnecessary DOM rebuilds and network fetches when the user clicks the already active channel.
♻️ Proposed refactor
if (chanSeg) chanSeg.addEventListener('click', (e) => { const b = e.target.closest('button[data-channel]'); if (!b) return; + if (b.dataset.channel === channel) return; channel = b.dataset.channel; variant = Object.keys(device.firmware[channel])[0]; epoch++;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@js/views/device.js` around lines 251 - 256, Update the click handler for chanSeg to return immediately when the clicked button’s data-channel matches the currently active channel, before changing channel, selecting the firmware variant, or incrementing epoch. Preserve the existing behavior for clicks that select a different channel.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@js/views/device.js`:
- Around line 251-256: Update the click handler for chanSeg to return
immediately when the clicked button’s data-channel matches the currently active
channel, before changing channel, selecting the firmware variant, or
incrementing epoch. Preserve the existing behavior for clicks that select a
different channel.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 035640e7-2f6e-4a7e-9acd-6e8623b8e36a
📒 Files selected for processing (2)
js/views/device.jstests/installer.spec.js
Fixes the real cause of the variant-click flicker (follow-up to PR #23, which fixed a separate install-button churn but not the visible blip).
Instrumenting every render slot on the live page showed the blip is the "What's new" release-notes box: on every variant click, including clicking the already-selected variant, renderReleaseNotes did
slot.innerHTML = ''and then re-fetched from the GitHub API, so the box went empty for a network round-trip and refilled. Two causes:The fix:
Verified by re-instrumenting the fixed build: a same-variant click now produces zero DOM mutations anywhere (previously it emptied and refilled the box), and switching variants swaps the notes in a single in-place mutation that never leaves the box empty. New Playwright tests assert both: no extra GitHub API call on a same-variant click, and the release-notes slot never becomes empty during a switch. Full suite 26/26, validators green.
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests