feat: pre-dry-run — hover prompt, SVG mapping fix, fixture rebuild#141
feat: pre-dry-run — hover prompt, SVG mapping fix, fixture rebuild#141
Conversation
) Three changes for experiment readiness: 1. PROMPT.md: add [hover] section with :hover CSS generation instructions, update rules to allow hover effects when [hover]: data is provided 2. implement.ts: copy vectors/mapping.json alongside SVGs — was missing, causing all SVG inline to fail (mapping lookup fell back to legacy ID-based naming which doesn't match instance-nested IDs) 3. Rebuild desktop-product-detail fixture with interactionDestinations (5 hover destinations resolved) Exit criteria verified: - [component:] annotations: 91 ✓ - /* var: */ token references: 129 ✓ - svg: inline: 9 ✓ (was 0 before mapping fix) - url(images/): 4 ✓ - [hover:] blocks: 5 ✓ - PROMPT.md hover section: present ✓ Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (74)
📒 Files selected for processing (5)
📝 WalkthroughWalkthroughUpdated PROMPT.md to allow generating CSS hover rules only when the design tree includes explicit Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cli/commands/implement.ts (1)
88-93: 🧹 Nitpick | 🔵 TrivialFix is correct; minor log message inconsistency.
The filter change correctly includes
mapping.jsonalongside SVG files, which is necessary fordesign-tree.tsto resolve node IDs to SVG filenames (as shown in the context snippets).However, the log message at line 93 now counts
mapping.json(if present) but labels the count as "SVGs copied", which could be slightly misleading.💡 Optional: Adjust log message for accuracy
- console.log(` vectors/: ${vecFiles.length} SVGs copied`); + const svgCount = vecFiles.filter(f => f.endsWith(".svg")).length; + console.log(` vectors/: ${svgCount} SVGs copied`);Alternatively, use a more generic message like
"${vecFiles.length} files copied"to match the pattern used for images at line 106.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/commands/implement.ts` around lines 88 - 93, The log message after copying vector assets is now inaccurate because vecFiles can include mapping.json as well as .svg files; update the console output in implement.ts (the block that builds vecFiles from vectorDir and copies to vecOutputDir) to use a generic label like "files copied" or otherwise reflect mixed file types (e.g., `${vecFiles.length} files copied`) so the count matches the actual files copied.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/skills/design-to-code/PROMPT.md:
- Around line 78-95: Add a second example showing child-element hover overrides:
extend the existing [hover]: documentation to include a nested example using a
parent component (e.g., Card) with a child element (e.g., Icon) and an explicit
`[hover]: Icon: ...` line, and show the generated descendant selector `:hover`
CSS (e.g., `.card .icon { ... }` and `.card:hover .icon { ... }`) so readers see
how `[hover]: Icon:` maps to the parent `:hover` + child selector pattern; keep
wording consistent with the Button example and explicitly state that only
provided hover data is used.
- Line 94: The wording "via parent selector" is ambiguous; update the sentence
that references child hover styles (the example "[hover]: Icon: fill: `#FFFFFF`")
to explicitly state that the hover state should be applied using a parent hover
selector that targets the child element (i.e., the pattern where the parent is
hovered and the child receives the hovered styles, rather than using a CSS
parent-only construct like :has()). Replace "via parent selector" with a clear
phrase such as "using a parent:hover → child selector (apply the styles on
.parent:hover .child so the child changes when the parent is hovered)" and
ensure the example maps to that pattern.
- Around line 80-91: Update the two fenced code blocks shown — the design-tree
block starting with "Button (INSTANCE, 120x40) [component: Button]" and the
generated CSS block that begins ".button { background: `#2C2C2C`; }" — to include
explicit language identifiers: add a marker (e.g., ```text or ```design-tree) to
the first block and ```css to the second block so both fences specify a language
for proper syntax highlighting and to satisfy MD040.
---
Outside diff comments:
In `@src/cli/commands/implement.ts`:
- Around line 88-93: The log message after copying vector assets is now
inaccurate because vecFiles can include mapping.json as well as .svg files;
update the console output in implement.ts (the block that builds vecFiles from
vectorDir and copies to vecOutputDir) to use a generic label like "files copied"
or otherwise reflect mixed file types (e.g., `${vecFiles.length} files copied`)
so the count matches the actual files copied.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: f95daefb-ff06-48d8-a689-6280818254cd
⛔ Files ignored due to path filters (1)
fixtures/desktop-product-detail/data.jsonis excluded by!fixtures/**
📒 Files selected for processing (2)
.claude/skills/design-to-code/PROMPT.mdsrc/cli/commands/implement.ts
… IDs
- Add child hover example: .button:hover .icon { fill: #FFFFFF }
- Clarify "parent selector" → ".parent:hover .child" pattern
- Add text/css language identifiers to code blocks
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…tinction Fixture rebuild: - All 24 fixtures rebuilt from latest Figma source - interactionDestinations included (2-6 per fixture) - Screenshots renamed to viewport convention (375/768/1200/1920) - Tablet (768) and large desktop (1920) screenshots from Figma API Design-tree: content-image vs background-image: - Leaf nodes (no children) → content-image: url(...) + object-fit - Nodes with children → background-image: url(...) + background-size - Enables AI to use <img> for content images, CSS background for backgrounds PROMPT.md: updated image section with two distinct types Strip functions: content-image protected from color stripping Tests: updated for content-image/background-image distinction Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
Three changes preparing for ablation experiment (#130):
1. PROMPT.md — hover state instructions
### Hover Statessection:[hover]:data →:hoverCSS[hover]:data is provided2. implement.ts — SVG mapping.json copy fix
vectors/mapping.jsonwas not being copied to output directorymapping.jsonin copy filter → 9 SVGs now inline correctly3. Fixture rebuild — desktop-product-detail
interactionDestinations(5 hover destinations)Exit criteria (all pass)
[component:]annotations/* var: */token refssvg:inlineurl(images/)[hover:]blocksCloses #139
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
New Features