refactor: redesign CPU State + Execution debugger cards#184
Merged
Conversation
Extract the inline Overview-tab cards into prop-driven presentational components (CpuStateCard, ExecutionCard) and restyle per a Claude Design-assisted mockup, mapping every value to existing design tokens: - CPU State: segmented A/X/Y/SP register rail, a clickable PC pill (AddressLink), a labeled flag-chip strip with clear ON/clear/unused states, and a de-emphasized purple cycle counter. - Execution: divider-separated rows with a status dot on IRQ/NMI. Keeps the existing debugInfo data contract and helpers unchanged; flag register stays labeled "Flags" (honors AC-7 guard, no "Status" overload). Adds behavioral tests for both cards. Version bump 4.51.3 -> 4.51.4. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The first pass left Execution looking unchanged: values stayed at normal weight and both card titles kept the original 16px/medium size, so only CPU State (with its bold register rail) read as redesigned. Match the measured mockup: card titles -> 18px semibold (text-base), data values -> semibold, row labels -> 14px (text-xs). Keeps the flag label "FLAGS" (AC-7 guard) and all tokens. Lock the value emphasis in ExecutionCard tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR refactors the debugger panel by extracting CPU state and execution display from inline DebuggerLayout markup into two new memoized components: ChangesDebugger Panel Refactoring
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/components/CpuStateCard.tsx (1)
68-82: ⚡ Quick winConsider
bg-surface-overlayfor register rail cells.The register cells use
bg-surface-secondary/40for translucency. For consistency with the design token system, consider usingbg-surface-overlayfor translucent surfaces within the card. Based on learnings,bg-surface-overlayis preferred for overlay surfaces that require transparency.♻️ Alternative approach
<div key={reg.label} - className={`flex flex-col items-center gap-xs py-sm bg-surface-secondary/40 ${ + className={`flex flex-col items-center gap-xs py-sm bg-surface-overlay ${ i > 0 ? 'border-l border-border-primary' : '' }`} >🤖 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 `@src/components/CpuStateCard.tsx` around lines 68 - 82, The register cells in CpuStateCard use the tailwind class "bg-surface-secondary/40" which should be replaced with the design-token overlay class "bg-surface-overlay" for consistent translucent surface styling; update the JSX inside the REGISTERS.map render (the div with className containing bg-surface-secondary/40) to use bg-surface-overlay instead, keeping the rest of the classes and behavior (including the key using reg.label and the displayed value via getDebugValueOrDefault(cpu?.[reg.field], reg.def)) unchanged.
🤖 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.
Inline comments:
In `@src/components/CpuStateCard.tsx`:
- Line 47: Replace the opaque background token with the translucent overlay
token in the CpuStateCard component: locate the root div in CpuStateCard (the
element with className including "bg-surface-primary") and change that token to
"bg-surface-overlay" so the card uses the translucent panel background
consistent with other panels.
In `@src/components/ExecutionCard.tsx`:
- Line 29: In ExecutionCard (the component rendering the outer div with
className "bg-surface-primary rounded-lg p-md border border-border-primary"),
replace the opaque background token "bg-surface-primary" with the translucent
token "bg-surface-overlay" (keeping the other utility classes intact) so the
card uses the overlay surface style consistent with other panels.
---
Nitpick comments:
In `@src/components/CpuStateCard.tsx`:
- Around line 68-82: The register cells in CpuStateCard use the tailwind class
"bg-surface-secondary/40" which should be replaced with the design-token overlay
class "bg-surface-overlay" for consistent translucent surface styling; update
the JSX inside the REGISTERS.map render (the div with className containing
bg-surface-secondary/40) to use bg-surface-overlay instead, keeping the rest of
the classes and behavior (including the key using reg.label and the displayed
value via getDebugValueOrDefault(cpu?.[reg.field], reg.def)) unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ebc0a1ff-fef6-451d-b3ec-d773661ad717
📒 Files selected for processing (6)
src/components/CpuStateCard.tsxsrc/components/DebuggerLayout.tsxsrc/components/ExecutionCard.tsxsrc/components/__tests__/CpuStateCard-cpu-state-panel-redesign.vitest.test.tsxsrc/components/__tests__/ExecutionCard-cpu-state-panel-redesign.vitest.test.tsxsrc/version.ts
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.
Context
Redesign of the CPU State and Execution cards in the debugger Overview tab,
using a Claude Design–assisted mockup as the visual target. The original cards were
visually flat (labels, values, and the cycle counter all at similar weight), so nothing
guided the eye. This gives them a clear hierarchy while keeping the data contract and
design-token system intact.
What changed
DebuggerLayout.tsxintoprop-driven presentational components —
CpuStateCardandExecutionCard— which takedebugInfoand are unit-testable in isolation. Net code is roughly flat.PC pill (reuses
AddressLinkwith context menu / run-to-cursor), a labeledflag-chip strip with distinct ON / clear / unused (
-,B) states, and ade-emphasized purple cycle counter.
semibold title and emphasized (semibold) values so it reads as redesigned, not just
re-bordered.
Design decisions
the AC-7 guard test (
DebuggerLayout-debugger-exec-bar.vitest.test.tsx) deliberatelyforbids reusing "Status" as a label here. Honored the existing decision rather than
reverting it.
text-base/semibold (closest token to the mockup's 20px); the changeis scoped to these two cards — sibling cards (Memory Map, Stack, …) keep their current
title size. Happy to harmonize those in a follow-up if desired.
Verification
yarn test:ci(lint + type-check + vitest): 785 passed, 1 skipped (the WASMbenchmark that skips in Node, as documented).
success, unused→disabled,IRQ/NMI ACTIVE→
warning, register-rail values, PC renders as a button, value emphasis).typography hierarchy all match).
4.51.3→4.51.4(refactor → patch).Engine parity
UI-only change — no CPU/engine code touched, JS↔WASM parity unaffected.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Chores