fix: combined card layout improvements#12
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
- Swap streak positions: longest streak (gold trophy) on left, current streak (orange fire) on right - Full-width language progress bar in the title area - Language items aligned with stats rows (same 25px spacing, 14px font) - Proper ring/icon sizing matching the standalone streak card - Remove all divider lines for cleaner look - Add gold trophy ring + icon for longest streak - Reduce card width to 650px - Add preview script for local iteration Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
compscidr
force-pushed
the
fix/combined-card-layout
branch
from
56c515c to
ed7a8c3
Compare
There was a problem hiding this comment.
Pull request overview
This PR refines the combined stats card SVG layout (streak + stats + languages) to a cleaner, more compact presentation, and adds a small local preview script to iterate on the SVG output.
Changes:
- Reduced default combined card width and rebalanced column geometry.
- Reworked streak visuals (trophy/fire rings), language list spacing/typography, and removed divider lines.
- Moved the languages progress bar into the title area via post-render SVG string injection; added
scripts/preview-combined.jsfor local preview output.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/cards/combined.js |
Adjusts combined card dimensions/layout, streak rendering, languages list styling, and injects a full-width language progress bar into the title area. |
scripts/preview-combined.js |
Adds a local script to render the combined card with mock data and write an SVG for quick iteration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .map((lang, i) => { | ||
| const percent = ((lang.size / totalSize) * 100).toFixed(2); | ||
| const y = i * 20; | ||
| const y = i * 25; |
There was a problem hiding this comment.
Language rows are now laid out with y = i * 25, but the card height logic still uses a 20px-per-row assumption (langListHeight = topLangs.length * 20 + 30). For higher langs_count values this can under-estimate contentHeight and clip the language list; update the height calculation to match the new 25px row spacing (and adjust/remove the old progress-bar padding if it's no longer part of the body).
| const y = i * 25; | |
| const y = i * 20; |
| // Inject progress bar into the title area by inserting before the final </svg>. | ||
| const lastSvgClose = cardSvg.lastIndexOf("</svg>"); | ||
| return ( | ||
| cardSvg.slice(0, lastSvgClose) + | ||
| progressBarSection + | ||
| cardSvg.slice(lastSvgClose) | ||
| ); |
There was a problem hiding this comment.
Progress bar injection relies on string-slicing cardSvg using lastIndexOf("</svg>") without handling the case where the substring isn't found. If lastSvgClose is -1 (e.g., Card.render output changes), slice(0, -1) will corrupt the SVG output. Add a guard/fallback (or avoid string-based injection by rendering the progress bar within the card body / Card API).
| font: 700 12px 'Segoe UI', Ubuntu, Sans-Serif; | ||
| fill: ${streakColor}; | ||
| } | ||
| .streak-label-longest { | ||
| font: 700 12px 'Segoe UI', Ubuntu, Sans-Serif; |
There was a problem hiding this comment.
With the introduction of .streak-label-longest and use of .streak-label-current, the older .streak-label rule (a few lines above) is no longer referenced in this SVG. Consider removing it (or reusing it) to avoid dead CSS and ambiguity about which label class should be used.
| ${languagesSection} | ||
| </svg> | ||
| `); | ||
|
|
||
| // Inject progress bar into the title area by inserting before the final </svg>. | ||
| const lastSvgClose = cardSvg.lastIndexOf("</svg>"); | ||
| return ( | ||
| cardSvg.slice(0, lastSvgClose) + | ||
| progressBarSection + | ||
| cardSvg.slice(lastSvgClose) | ||
| ); |
There was a problem hiding this comment.
renderCombinedCard has substantial layout logic changes (streak rings, language list spacing, title-area progress bar injection), but there are currently no automated tests/snapshots covering the combined card output. Adding a render snapshot test (similar to other render*Card tests) would help catch layout regressions and validate the new progress-bar injection behavior.
Summary
Major layout polish for the combined card:
Also adds
scripts/preview-combined.jsfor local SVG iteration with mock data.Test plan
🤖 Generated with Claude Code