[#1043] Fix chart label overlap, scale, and accent color#1047
[#1043] Fix chart label overlap, scale, and accent color#1047realproject7 merged 2 commits intomainfrom
Conversation
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
realproject7
left a comment
There was a problem hiding this comment.
re2 review: APPROVE — Log scale correctly spreads milestones using log10(K–M). Accent color #8B4513 matches current --accent in globals.css (CLAUDE.md is outdated — still says #00ff88). Padding prevents edge clipping. Scale labels updated to match log range.
Minor nit (non-blocking): ACCENT constant duplicates the CSS token value. Could use var(--accent) in SVG fill/stroke attributes for single-source-of-truth, but not a blocker.
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The accent color is correctly matched to --accent, but the chart label layout still does not satisfy the issue acceptance criteria. The new log scale moves the original left-side crowding to the right side and leaves the final label centered on the chart edge.
Findings
- [blocking]
$50Mand$100Mdesktop milestone labels remain too close together and the$100Mlabel is still positioned out of bounds.- File:
src/components/airdrop/CampaignHero.tsx:99 - Details:
logScale(50_000_000)withLOG_MIN = log10(100_000)lands at about 90% while$100Mlands at 100%; both labels use-translate-x-1/2, so the last two labels are only ~10% of the container apart and the$100Mlabel extends past the right edge. The issue specifically called for all 4 milestones to be readable/spaced and labels to stay within bounds. - Suggestion: Use the spacing described in the issue, constrain/clamp edge label positions, or render labels with edge-aware alignment. For example, make the first label left-aligned, the last label right-aligned, and ensure
$50Mhas enough separation from$100Mon desktop.
- File:
Decision
Request changes. The color cleanup is fine, but the milestone label overlap/bounds acceptance criteria are not met yet.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Summary
The updated PR resolves the prior blocker by using fixed milestone positions at 25/50/75/100% and right-aligning the final desktop label, so the labels are readable and stay within bounds. The chart color now matches the site accent token value from globals.css.
Findings
- None.
Decision
Approved. Acceptance criteria for #1043 are met. The currently reported PR checks are passing (Vercel and Vercel Preview Comments); no lint-and-typecheck or e2e check is currently present on the latest status rollup.
realproject7
left a comment
There was a problem hiding this comment.
re2 re-review after update: APPROVE — Fixed equal spacing (25/50/75/100%) resolves the label crowding. Piecewise linear interpolation in mcapToX is correct — finds the right segment and lerps within it. Last label right-aligned to stay in bounds. Accent color and padding unchanged from prior review. Good fix.
Summary
log10) for milestone X positions — $1M/$10M/$50M/$100M now evenly spread across chart width#00ff88neon green with site accent color (#8B4513) from CSS tokenspad: 10) to prevent label clipping at chart edgesFixes #1043
Test plan
🤖 Generated with Claude Code