feat: add bar/line toggle to contribution graph#38
Conversation
|
@liascope is attempting to deploy a commit to the PRIYANSHU DOSHI's projects Team on Vercel. A member of the Team first needs to authorize it. |
Priyanshu-byte-coder
left a comment
There was a problem hiding this comment.
Thanks for the contribution! The bar/line toggle itself works well, but this PR has a critical regression that needs to be fixed before merging.
Critical: Time range selector was removed ❌
The original component had a 7d / 14d / 30d / 90d range picker that lets users change the data window. This PR deletes RANGES, the days state, and the dynamic fetch(...?days=${days}) — locking the chart to 30 days forever.
The acceptance criteria in issue #17 specifically says:
Toggle button visible next to the 7d/14d/30d/90d tabs
Please restore the range selector and place the bar/line toggle alongside it, not instead of it.
What to restore:
const RANGES = [
{ label: "7d", days: 7 },
{ label: "14d", days: 14 },
{ label: "30d", days: 30 },
{ label: "90d", days: 90 },
];
const [days, setDays] = useState(30);
useEffect(() => {
setLoading(true);
fetch(`/api/metrics/contributions?days=${days}`)
...
}, [days]);Minor issues
-
allowDecimals={false}removed from<YAxis>— without this, the Y-axis can show values like 1.5 commits. Add it back to both BarChart and LineChart. -
Tooltip styling stripped —
borderRadius: "8px",fontSize: "12px"onlabelStyle, andcursor={{ fill: "#334155" }}were removed. Please restore them for visual consistency. -
Indentation in the toggle button block is inconsistent — some lines use 2 spaces, others 4+. Keep it uniform.
Once the range selector is restored and the minor items addressed, this is ready to merge. The toggle implementation itself (chartType state, conditional rendering, early-return loading skeleton) is clean — nice work on those parts!
Priyanshu-byte-coder
left a comment
There was a problem hiding this comment.
All the feedback addressed nicely — time range selector is back, both chart types have proper axis/tooltip styling, and the toggle sits cleanly next to the range tabs. Just one tiny thing I'll clean up on my end after merging: missing newline at end of file and a stray blank line with whitespace. Nothing blocking. Merging now, thanks for the quick turnaround!
8feb1f9
into
Priyanshu-byte-coder:main
Summary
Adds a toggle to switch between BarChart and LineChart in the ContributionGraph component.
Users can now switch visualization modes to better analyze commit activity trends over time.
Closes #17
Type of Change
Changes Made
chartTypestate (bar | line)How to Test
Steps for the reviewer to verify this works:
Checklist
npm run lintpasses locallynpm run type-check)