Threshold percentage on labels for pie charts #135
Conversation
WalkthroughThe changes introduce a percentage threshold control for pie chart labels. A new utility function processes chart configurations to dynamically compute label visibility and formatting based on a labelThreshold property. This processor is integrated across six chart rendering components and a new UI control is added to the pie chart customizations panel. Changes
Sequence DiagramsequenceDiagram
actor User
participant UI as ChartCustomizations
participant Store as State/Customizations
participant Renderer as Chart Component<br/>(e.g., ChartPreview)
participant Processor as processChartConfig
participant ECharts as ECharts Instance
User->>UI: Set labelThreshold value
UI->>Store: updateCustomization(labelThreshold)
Store->>Renderer: Pass config with labelThreshold
Renderer->>Processor: processChartConfig(config)
Note over Processor: Compute per-item label<br/>visibility based on threshold
Processor-->>Renderer: Return processedConfig
Renderer->>ECharts: setOption(processedConfig)
ECharts-->>User: Render chart with<br/>filtered labels
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Areas requiring attention:
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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.
Actionable comments posted: 0
🧹 Nitpick comments (4)
lib/chart-config-processor.ts (3)
11-17: Consider using structuredClone or a proper deep clone library.
JSON.parse(JSON.stringify())has several limitations:
- Loses function references (though you add them back later)
- Cannot handle circular references
- Drops
undefined,Symbol, and non-JSON types- Performance overhead for large configs
For modern environments, use
structuredClone(config)or a library likelodash.cloneDeepfor better reliability.- // Create a deep copy to avoid mutating the original - const processedConfig = JSON.parse(JSON.stringify(config)); + // Create a deep copy to avoid mutating the original + const processedConfig = structuredClone(config);
34-59: Simplify nested ternary for better readability.The deeply nested ternary operator (lines 44-52) is difficult to read and maintain. Consider extracting this logic into a helper function or using a switch statement.
function getFormatterString(labelFormat: string): string { switch (labelFormat) { case 'percentage': return '{d}%'; case 'value': return '{c}'; case 'name_percentage': return '{b}\n{d}%'; case 'name_value': return '{b}\n{c}'; default: return '{d}%'; } }Then use:
formatter: showLabel ? getFormatterString(labelFormat) : ''
63-79: Eliminate duplicate formatter logic.The formatter logic appears twice: once as template strings (lines 44-52) and again in the series-level formatter function (lines 67-77). This duplication increases maintenance burden and risk of inconsistency.
Consider extracting a shared helper function that both code paths can use, or rely solely on per-item formatters when threshold is active.
components/charts/ChartCustomizations.tsx (1)
477-498: LGTM! Consider clarifying the default value in the UI.The percentage threshold control is well-implemented with proper validation. The default value of 5 is reasonable, but users might benefit from seeing this explicitly indicated in the UI (e.g., via placeholder text or help text) rather than only when the field is empty.
Optional enhancement:
<Input id="labelThreshold" type="number" min="0" max="100" step="0.1" value={customizations.labelThreshold ?? 5} + placeholder="5 (default)" onChange={(e) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
components/charts/ChartCustomizations.tsx(1 hunks)components/charts/ChartExport.tsx(2 hunks)components/charts/ChartExportDropdownForList.tsx(2 hunks)components/charts/ChartPreview.tsx(2 hunks)components/charts/MiniChart.tsx(2 hunks)components/dashboard/chart-element-v2.tsx(2 hunks)components/dashboard/chart-element-view.tsx(2 hunks)lib/chart-config-processor.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
{components,app}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
{components,app}/**/*.{ts,tsx}: Use Tailwind utility-first styling, CVA for component variants, and the cn() utility for class merging
Guard browser-only APIs with typeof window !== 'undefined' in client-side code
Files:
components/charts/ChartCustomizations.tsxcomponents/dashboard/chart-element-view.tsxcomponents/charts/ChartExportDropdownForList.tsxcomponents/charts/ChartPreview.tsxcomponents/dashboard/chart-element-v2.tsxcomponents/charts/MiniChart.tsxcomponents/charts/ChartExport.tsx
components/charts/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Implement charts following existing patterns in components/charts/, selecting the appropriate library (ECharts/Nivo/Recharts) and supporting PNG/PDF export
Files:
components/charts/ChartCustomizations.tsxcomponents/charts/ChartExportDropdownForList.tsxcomponents/charts/ChartPreview.tsxcomponents/charts/MiniChart.tsxcomponents/charts/ChartExport.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Define TypeScript interfaces/types for API responses to ensure type safety
Use the @/* path alias for imports instead of deep relative paths
Files:
components/charts/ChartCustomizations.tsxcomponents/dashboard/chart-element-view.tsxcomponents/charts/ChartExportDropdownForList.tsxcomponents/charts/ChartPreview.tsxlib/chart-config-processor.tscomponents/dashboard/chart-element-v2.tsxcomponents/charts/MiniChart.tsxcomponents/charts/ChartExport.tsx
🧠 Learnings (1)
📚 Learning: 2025-10-13T07:21:21.746Z
Learnt from: CR
PR: DalgoT4D/webapp_v2#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-13T07:21:21.746Z
Learning: Applies to components/charts/**/*.{ts,tsx} : Implement charts following existing patterns in components/charts/, selecting the appropriate library (ECharts/Nivo/Recharts) and supporting PNG/PDF export
Applied to files:
components/charts/ChartExportDropdownForList.tsxcomponents/charts/ChartPreview.tsxcomponents/dashboard/chart-element-v2.tsxcomponents/charts/MiniChart.tsxcomponents/charts/ChartExport.tsx
🧬 Code graph analysis (6)
components/dashboard/chart-element-view.tsx (1)
lib/chart-config-processor.ts (1)
processChartConfig(11-96)
components/charts/ChartExportDropdownForList.tsx (1)
lib/chart-config-processor.ts (1)
processChartConfig(11-96)
components/charts/ChartPreview.tsx (1)
lib/chart-config-processor.ts (1)
processChartConfig(11-96)
components/dashboard/chart-element-v2.tsx (1)
lib/chart-config-processor.ts (1)
processChartConfig(11-96)
components/charts/MiniChart.tsx (1)
lib/chart-config-processor.ts (1)
processChartConfig(11-96)
components/charts/ChartExport.tsx (1)
lib/chart-config-processor.ts (1)
processChartConfig(11-96)
🔇 Additional comments (6)
components/charts/ChartExportDropdownForList.tsx (1)
14-14: LGTM!The integration of
processChartConfigis clean and well-documented with a clear comment explaining its purpose.Also applies to: 118-120
components/charts/ChartExport.tsx (1)
21-21: LGTM!The integration properly processes the config before export and includes a helpful comment. Disabling animation for export is a good practice.
Also applies to: 90-95
components/charts/MiniChart.tsx (1)
14-14: LGTM!Clean integration with clear documentation. The processing step is consistently applied before setting options.
Also applies to: 84-86
components/dashboard/chart-element-v2.tsx (1)
41-41: LGTM!The config processing is correctly applied after grid modifications and before setting the chart options. Documentation is clear.
Also applies to: 654-658
components/dashboard/chart-element-view.tsx (1)
39-39: LGTM!The processing is correctly positioned after applying styled config and before setting chart options. The implementation is consistent with other chart components.
Also applies to: 894-900
components/charts/ChartPreview.tsx (1)
8-8: LGTM!The preprocessing step is correctly integrated after modifying the config for proper margins and before setting the chart option.
Also applies to: 66-70
|
a. threshold is 1.5% -> labels are seen b. threshold is now 3% -> labels are hidden |
Summary by CodeRabbit
New Features
Improvements