feat(ui): add Y-axis with numbers to project overview charts#330
feat(ui): add Y-axis with numbers to project overview charts#330nielskaspers wants to merge 1 commit intoOpenpanel-dev:mainfrom
Conversation
Show Y-axis tick labels on project card charts for quick value reference. Uses the existing useYAxisProps hook for consistent formatting (short numbers like 1K, 10K) and dynamic width. Fixes Openpanel-dev#326
|
|
📝 WalkthroughWalkthroughModified the project chart component to dynamically render Y-axis numeric values by integrating a Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/start/src/components/projects/project-chart.tsx (1)
97-111:⚠️ Potential issue | 🔴 CriticalReact Rules of Hooks violation:
useYAxisPropsis called after conditional return.The hook call on line 111 occurs after the early return on lines 99-101. When
data.lengthchanges between 0 and non-zero, React will see a different number of hook calls, causing a runtime error.Move the hook call before the early return to ensure hooks are called unconditionally.
🐛 Proposed fix
export function ProjectChart({ data, dots = false, color = 'blue', }: { dots?: boolean; color?: 'blue' | 'green' | 'red'; data: { value: number; date: Date; revenue: number }[]; }) { const [activeBar, setActiveBar] = useState(-1); + const yAxisProps = useYAxisProps({}); if (data.length === 0) { return null; } // Transform data for Recharts (needs timestamp for time-based x-axis) const chartData = data.map((item) => ({ ...item, timestamp: item.date.getTime(), })); const maxValue = Math.max(...data.map((d) => d.value), 0); const maxRevenue = Math.max(...data.map((d) => d.revenue), 0); - const yAxisProps = useYAxisProps({});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/start/src/components/projects/project-chart.tsx` around lines 97 - 111, The hook useYAxisProps is being called after an early return when data.length === 0, which violates the Rules of Hooks; move the useYAxisProps(...) call so it executes unconditionally before the "if (data.length === 0) return null;" check (i.e., call useYAxisProps above the early-return and keep the rest of chartData, maxValue, maxRevenue logic unchanged) so hooks are invoked in the same order regardless of data.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/start/src/components/projects/project-chart.tsx`:
- Around line 97-111: The hook useYAxisProps is being called after an early
return when data.length === 0, which violates the Rules of Hooks; move the
useYAxisProps(...) call so it executes unconditionally before the "if
(data.length === 0) return null;" check (i.e., call useYAxisProps above the
early-return and keep the rest of chartData, maxValue, maxRevenue logic
unchanged) so hooks are invoked in the same order regardless of data.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f81ef5a9-4c72-4a5a-9114-9ed00ed73e3a
📒 Files selected for processing (1)
apps/start/src/components/projects/project-chart.tsx
2 participants
Summary
Issue
Fixes #326
Changes
useYAxisPropshook inProjectChartcomponenthide width={0}) with visible Y-axis using consistent formatting (short numbers: 1K, 10K, etc.)Testing
/:organizationId/)Summary by CodeRabbit