[WEB-3697] chore: chart components #6835
Conversation
WalkthroughThis pull request updates several chart components across the project. In the constants file, a constant is renamed to better reflect axis labeling. Multiple chart components (area, bar, line, pie, and treemap) have been enhanced with new props, state management, and interactivity such as legend handling and tooltip improvements. New components like a custom legend, custom tooltip, and a custom active shape for pie charts were added. Type definitions have been updated to support additional customization options for charts. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant C as Chart Component
participant L as Legend
participant T as Tooltip
U->>L: Hover/Click on legend item
L->>C: Trigger event to update activeLegend state
C->>C: Update state (activeLegend, activeBar/activeLine/activeIndex)
C->>T: Re-render tooltip with updated active state
T-->>U: Display updated chart information
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (12)
packages/constants/src/chart.ts (1)
2-2: Consider referencing LABEL_CLASSNAME instead of duplicating the valueThe new
AXIS_LABEL_CLASSNAMEhas the exact same value asLABEL_CLASSNAME. While the semantic naming improvement is good, consider referencing the existing constant to follow DRY principles.-export const AXIS_LABEL_CLASSNAME = "uppercase text-custom-text-300/60 text-sm tracking-wide"; +export const AXIS_LABEL_CLASSNAME = LABEL_CLASSNAME;packages/propel/src/charts/pie-chart/active-shape.tsx (1)
4-31: Add type definition, displayName, and documentationThis is a nice enhancement for pie charts, creating a visual highlight effect for active sectors. To improve the component:
- Define a proper type interface instead of using
any- Add a displayName for debugging
- Include documentation about the component's purpose
import React from "react"; import { Sector } from "recharts"; -export const CustomActiveShape = React.memo((props: any) => { +/** + * CustomActiveShape renders an enhanced visual for active pie chart sectors + * by displaying both the sector and an outer ring effect in the same color. + */ +interface CustomActiveShapeProps { + cx: number; + cy: number; + cornerRadius: number; + innerRadius: number; + outerRadius: number; + startAngle: number; + endAngle: number; + fill: string; +} + +export const CustomActiveShape = React.memo((props: CustomActiveShapeProps) => { const { cx, cy, cornerRadius, innerRadius, outerRadius, startAngle, endAngle, fill } = props; return ( <g> <Sector cx={cx} cy={cy} innerRadius={innerRadius} outerRadius={outerRadius} cornerRadius={cornerRadius} startAngle={startAngle} endAngle={endAngle} fill={fill} /> <Sector cx={cx} cy={cy} startAngle={startAngle} endAngle={endAngle} cornerRadius={cornerRadius} innerRadius={outerRadius + 6} outerRadius={outerRadius + 10} fill={fill} /> </g> ); }); +CustomActiveShape.displayName = "CustomActiveShape";packages/propel/src/charts/components/tooltip.tsx (1)
20-20: Consider stronger type safety when filtering payload items.Using template literals to convert dataKey to string might cause unexpected behavior if dataKey is a complex object.
- const filteredPayload = payload?.filter((item) => item.dataKey && itemKeys.includes(`${item.dataKey}`)); + const filteredPayload = payload?.filter((item) => item.dataKey && itemKeys.includes(String(item.dataKey)));packages/propel/src/charts/pie-chart/tooltip.tsx (2)
7-7: Consider making dotColor required or providing a default value.The dotColor is optional but there's no fallback if it's not provided, which might lead to missing visual indicators.
- dotColor?: string; + dotColor: string;Alternatively, add a default color in the component implementation.
28-30: Add color validation or fallback for dotColor.The component directly applies dotColor without checking if it's valid or present, which could cause rendering issues.
- style={{ - backgroundColor: dotColor, - }} + style={{ + backgroundColor: dotColor || 'currentColor', + }}packages/propel/src/charts/bar-chart/bar.tsx (3)
21-36: Consider replacinganytype with a proper interface.Using
anyreduces type safety. Create a dedicated interface for the component props to improve maintainability.-export const CustomBar = React.memo((props: any) => { +interface CustomBarProps { + opacity: number; + fill: string; + x: number; + y: number; + width: number; + height: number; + dataKey: string; + stackKeys: string[]; + payload: any; // Consider defining a more specific type + textClassName: string; + showPercentage: boolean; + showTopBorderRadius: boolean; + showBottomBorderRadius: boolean; +} + +export const CustomBar = React.memo((props: CustomBarProps) => {
56-56: Consider adding validation for negative height.Adding a check for negative height could prevent potential rendering issues.
- if (!height) return null; + if (!height || height < 0) return null;
71-73: Consider adding transitions for fill property.The transition class only applies to opacity changes. Adding transitions for fill would make color changes smoother.
- className="transition-opacity duration-200" + className="transition-all duration-200"packages/propel/src/charts/components/legend.tsx (1)
70-72: Resolve TypeScript error with proper typing.The code uses
@ts-expect-errorwhich indicates a type issue. Consider addressing this with proper type definitions.- {/* @ts-expect-error recharts types are not up to date */} - {formatter?.(item.value, { value: item.value }, index) ?? item.payload?.name} + {formatter?.(item.value, { value: item.value } as any, index) ?? + (item.payload && 'name' in item.payload ? item.payload.name : item.value)}Alternatively, create a proper type definition file for the Recharts library to address this issue more comprehensively.
packages/propel/src/charts/pie-chart/root.tsx (3)
64-96: Pie configuration leverages new features well.
The integration ofactiveIndex,cornerRadius, and conditional labeling logic (showLabel+customLabel) offers robust customization. Consider a safeguard ifpayload.countis absent or null to avoid rendering undefined text.
110-123: Legend handling is intuitive.
Hover-based fading of slices is a user-friendly effect. As a possible enhancement, highlight the corresponding slice index when hovering over the legend for consistent interactivity across both chart and legend.
135-142: Tooltip content derivation is solid.
Using the cell’sfillfor the dot color andtooltipLabelfor dynamic labeling is well-structured. Consider renaming deeply nested items (e.g.,payload[0]?.payload?.payload) for extra clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
packages/constants/src/chart.ts(1 hunks)packages/propel/src/charts/area-chart/root.tsx(1 hunks)packages/propel/src/charts/bar-chart/bar.tsx(2 hunks)packages/propel/src/charts/bar-chart/root.tsx(3 hunks)packages/propel/src/charts/components/legend.tsx(1 hunks)packages/propel/src/charts/components/tick.tsx(1 hunks)packages/propel/src/charts/components/tooltip.tsx(1 hunks)packages/propel/src/charts/line-chart/root.tsx(1 hunks)packages/propel/src/charts/pie-chart/active-shape.tsx(1 hunks)packages/propel/src/charts/pie-chart/root.tsx(1 hunks)packages/propel/src/charts/pie-chart/tooltip.tsx(1 hunks)packages/propel/src/charts/tooltip.tsx(0 hunks)packages/propel/src/charts/tree-map/root.tsx(1 hunks)packages/types/src/charts.d.ts(5 hunks)
💤 Files with no reviewable changes (1)
- packages/propel/src/charts/tooltip.tsx
🧰 Additional context used
🧬 Code Definitions (3)
packages/propel/src/charts/components/legend.tsx (1)
packages/types/src/charts.d.ts (1)
TChartLegend(1-5)
packages/propel/src/charts/pie-chart/root.tsx (4)
packages/types/src/charts.d.ts (1)
TPieChartProps(100-119)packages/propel/src/charts/pie-chart/active-shape.tsx (1)
CustomActiveShape(4-31)packages/propel/src/charts/components/legend.tsx (1)
getLegendProps(7-31)packages/propel/src/charts/pie-chart/tooltip.tsx (1)
CustomPieChartTooltip(12-39)
packages/propel/src/charts/area-chart/root.tsx (5)
packages/types/src/charts.d.ts (1)
TAreaChartProps(87-93)packages/propel/src/charts/components/tick.tsx (2)
CustomXAxisTick(7-13)CustomYAxisTick(16-22)packages/constants/src/chart.ts (1)
AXIS_LABEL_CLASSNAME(2-2)packages/propel/src/charts/components/legend.tsx (1)
getLegendProps(7-31)packages/propel/src/charts/components/tooltip.tsx (1)
CustomTooltip(17-59)
🪛 Biome (1.9.4)
packages/propel/src/charts/line-chart/root.tsx
[error] 44-44: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
[error] 47-47: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
packages/propel/src/charts/bar-chart/root.tsx
[error] 46-46: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
[error] 49-49: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
packages/propel/src/charts/area-chart/root.tsx
[error] 35-35: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
[error] 38-38: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (44)
packages/propel/src/charts/components/tick.tsx (1)
5-5: LGTM! Color and style adjustment for better readabilityThe change from "fill-custom-text-400" to "fill-custom-text-300" and removal of "capitalize" improves the styling consistency of axis ticks.
packages/propel/src/charts/tree-map/root.tsx (1)
34-36: LGTM! Enhanced tooltip interactivitySetting
pointerEvents: "auto"enables the tooltip to receive mouse events, which allows for more interactive tooltips that can respond to hover or click events.packages/propel/src/charts/components/tooltip.tsx (5)
7-15: Well-structured props type definition.The component's props type is clear and descriptive, making it easy to understand what data the CustomTooltip component expects.
26-28: Good use of Card component with appropriate constraints.The max-height and scrolling behavior ensure the tooltip remains usable even when displaying many data points.
42-53: Well-structured nested layout for tooltip items.The nested divs with appropriate classes provide good control over truncation and spacing.
59-60: Component is properly memoized and named.Using React.memo for performance optimization and setting displayName for debugging is a good practice.
38-40:Details
❓ Verification inconclusive
Verify that 0.2 opacity provides sufficient visibility.
The opacity of 0.2 for non-active items might make them too faint for users to see clearly, especially on certain displays.
🏁 Script executed:
#!/bin/bash # Check if similar opacity values are used elsewhere in the codebase for inactive elements # Look for opacity values in the range of 0.1-0.3 in the codebase echo "Searching for similar opacity values in the codebase..." rg "opacity-\[?0\.[12][0-9]\]?" --type tsx --type jsx rg "opacity:?\s*0\.[12][0-9]" --type tsx --type jsx --type css rg "opacity=\"?0\.[12][0-9]\"?" --type tsx --type jsxLength of output: 379
Double-check tooltip opacity setting
The tooltip component applies Tailwind’s
opacity-20(0.2 opacity) for non-active items. Please manually verify—across various displays and under different lighting conditions—that this styling provides sufficient visibility and contrast. If it appears too faint, consider increasing the opacity value (e.g., to 0.3) to enhance readability.
🏁 Script executed:
#!/bin/bash echo "Searching for similar opacity values around 0.2 in .tsx, .jsx, and .css files..." rg -e 'opacity[-:=\s]*0\.2\b' --glob '*.tsx' --glob '*.jsx' --glob '*.css'Length of output: 369
Action Required: Manually Verify Opacity Visibility
The tooltip component uses Tailwind’s
opacity-20(i.e. 0.2 opacity) for de-emphasizing non-active items. Our automated search did not yield broader context for similar usage, so please manually verify that this opacity level ensures sufficient visibility and contrast on various displays and under different lighting conditions. If the 0.2 opacity appears too faint, consider increasing it (for example, toopacity-30) to maintain adequate readability.packages/propel/src/charts/pie-chart/tooltip.tsx (2)
16-19: Good use of Card component with appropriate constraints.The styling improvements with max-height, width, and scrolling behavior ensure the tooltip remains usable.
25-33: Enhanced tooltip item styling with truncation.The nested layout with truncation support for long text improves the readability of the tooltip.
packages/propel/src/charts/bar-chart/bar.tsx (3)
18-19: Clear constants for border radius values.Separating top and bottom border radius constants makes the code more maintainable.
52-54: Clean implementation of conditional border radius.The approach to conditionally apply border radius is clear and maintainable.
60-70: Improved SVG path with proper border radius application.The updated SVG path construction correctly implements the separate top and bottom border radius.
packages/propel/src/charts/components/legend.tsx (4)
7-31: Well-structured legend props generator function.The
getLegendPropsfunction provides a clean way to configure legend styles based on layout type.
33-38: Good use of TypeScript for enhanced type safety.The component uses proper TypeScript typing with React.forwardRef and selective picking of required props.
46-48: Responsive styling based on layout direction.The conditional classes based on layout direction ensure proper styling in both vertical and horizontal modes.
53-58: Good spacing management for legend items.The conditional padding and margin based on item position creates a well-balanced layout.
packages/propel/src/charts/line-chart/root.tsx (3)
4-14: Imports for Recharts and React hooks look appropriate
All newly introduced imports (e.g.,CartesianGrid,Legend, etc.) seem necessary for the updated chart features.
39-40: State management for active legend/line
Using React state to track the active line and legend item is a clean approach to manage hover styles and emphasis on specific data.
125-133: Legend integration
The new legend logic effectively handles mouse events and filters out non-active items. Nice addition for interactive charts.packages/propel/src/charts/bar-chart/root.tsx (3)
4-14: Refined imports for expanded chart features
The added imports (e.g.,CartesianGrid,Legend) align with the stateful enhancements for bars and legends.
41-42: State variables for interactivity
activeBarandactiveLegendwill help highlight focused bars on hover, enriching user experience.
126-134: Legend component
The conditional render ofLegendalong with the interactive event handlers is consistent with other chart components.packages/propel/src/charts/area-chart/root.tsx (3)
3-4: Additional imports for ComposedChart and comparison line
The imports (e.g.,Line,ComposedChart) properly support overlaying lines with area charts.
30-31: Localized state for hovered area
Storing the active area and legend item fosters clear boundaries for hover effects.
172-185: Comparison line addition
Overlaying a straight line from the origin to the final data point is a creative visual indicator, extending chart context without clutter.packages/propel/src/charts/pie-chart/root.tsx (8)
3-4: Imports are correct.
UsinguseMemoanduseStateis appropriate for controlling state. The new Recharts imports are consistent with the proposed features.
8-9: Separate utilities enhance maintainability.
Factoring out legend props and active shape logic into separate components fosters clarity and reusability.
13-29: Props destructuring is well-organized.
All newly introduced props (e.g.,legend,margin,showLabel,customLabel,centerLabel,cornerRadius,paddingAngle,tooltipLabel) align with the updated type definitions, enhancing customization options.
31-32: State usage for interactivity is clear.
Maintaining separateactiveIndexandactiveLegendstates is a clean approach for controlling slice highlights and legend interactions.
35-48: Memoized rendering of cells is performant.
EmployinguseMemoand handling mouse events within individual cells efficiently manages re-renders and improves responsiveness.
58-61: Fallback margin values are prudent.
Providing defaults prevents layout regressions and makes the chart more resilient to missing prop definitions.
99-108: Center label logic is neatly integrated.
Optional rendering of a center label adds clarity to the chart. EnsurecenterLabel.textis set to avoid an empty label.
126-132: Custom tooltip cursor and pointer events.
Allowing pointer events on the tooltip container preserves interactivity. ThecurrentColorfill aligns the cursor highlight with the chart’s text color scheme.packages/types/src/charts.d.ts (11)
1-5: Legend type specification is straightforward.
Definingalign,verticalAlign, andlayoutclarifies legend placement options.
7-12: Marginal flexibility.
TheTChartMargintype standardizes optional numeric boundaries, improving chart layout consistency.
23-24: Optional xAxis properties enhance reusability.
MakinglabelandstrokeColoroptional suits scenarios where minimal axis decoration is desired.
28-31: yAxis enhancements for dynamic data.
Optionaldomainandlabel, plus a configurablestrokeColor, afford greater adaptability for diverse charting requirements.
34-35: Legend & margin integration in TChartProps.
These additions unify chart layout and legend control under a single reusable interface.
45-46: TBarItem extended customization.
Includinglabelandfillclarifies bar item definitions and improves descriptive data visualizations.
60-66: Rich styling options in TLineItem.
Added fields (e.g.,showDot,smoothCurves,dashedLine) allow for more expressive line charts.
79-84: TAreaItem now covers key aesthetic needs.
Fill and stroke opacities, plus optional dot and smoothing, enable sophisticated area chart rendering.
89-92: Comparison line support.
An optionalcomparisonLinefosters quick visual references for user data analysis.
95-97: Expansion of TCellItem with fill.
Defining afillcolor for each cell accommodates diverse color schemes in pie or treemap data.
100-119: Broadened TPieChartProps.
Fields likecornerRadius,paddingAngle,showLabel, andtooltipLabelsignificantly boost PieChart customization. ThecenterLabeladds a helpful focal point to the chart.
| const itemLabels: Record<string, string> = useMemo( | ||
| () => lines.reduce((acc, line) => ({ ...acc, [line.key]: line.label }), {}), | ||
| [lines] |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid using spread syntax in reducers to improve performance
Repeatedly spreading objects in a .reduce() can lead to O(n²) complexity. Consider using direct assignment in a loop or Object.assign.
Below is an example diff illustrating the refactor for both itemLabels and itemDotColors:
- const itemLabels: Record<string, string> = useMemo(
- () => lines.reduce((acc, line) => ({ ...acc, [line.key]: line.label }), {}),
- [lines]
- );
- const itemDotColors = useMemo(
- () => lines.reduce((acc, line) => ({ ...acc, [line.key]: line.stroke }), {}),
- [lines]
- );
+ const itemLabels: Record<string, string> = useMemo(() => {
+ const result: Record<string, string> = {};
+ for (const lineItem of lines) {
+ result[lineItem.key] = lineItem.label;
+ }
+ return result;
+ }, [lines]);
+
+ const itemDotColors: Record<string, string> = useMemo(() => {
+ const result: Record<string, string> = {};
+ for (const lineItem of lines) {
+ result[lineItem.key] = lineItem.stroke;
+ }
+ return result;
+ }, [lines]);Also applies to: 47-47
🧰 Tools
🪛 Biome (1.9.4)
[error] 44-44: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
Description
This PR includes updated chart components.
Type of Change
Reference
[WEB-3697]
Summary by CodeRabbit
New Features
Refactor