[WEB-4371] feat: bar chart component with lollipop shape variant#7268
[WEB-4371] feat: bar chart component with lollipop shape variant#7268sriramveeraghanta merged 6 commits intopreviewfrom
Conversation
|
Pull Request Linked with Plane Work Items Comment Automatically Generated by Plane |
WalkthroughThe changes refactor the bar chart rendering logic by introducing typed props, utility functions, and constants for consistent styling. A new lollipop bar variant with a dotted option and a factory for shape variants are added. The bar chart root now supports flexible shape rendering and an optional custom tooltip. Types are updated to support shape variants. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BarChart
participant barShapeVariants
participant BarComponent
participant Tooltip
User->>BarChart: Render BarChart with bars and optional customTooltipContent
BarChart->>barShapeVariants: Select shape variant (bar/lollipop/lollipop-dotted) for each bar
barShapeVariants->>BarComponent: Render bar with props and configuration
BarChart->>Tooltip: On hover, render tooltip
alt customTooltipContent provided
BarChart->>Tooltip: Render customTooltipContent
else
BarChart->>Tooltip: Render default CustomTooltip
end
Suggested labels
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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/propel/src/charts/bar-chart/bar.tsx(3 hunks)packages/propel/src/charts/bar-chart/root.tsx(4 hunks)packages/types/src/charts/index.d.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
packages/types/src/charts/index.d.ts (1)
56-57: Type definitions look good!The new shape variant types are well-defined and maintain backward compatibility by making the property optional.
Also applies to: 67-67
packages/propel/src/charts/bar-chart/root.tsx (1)
70-71: Excellent implementation of shape variants and custom tooltip!The integration is clean and maintains backward compatibility:
- The default "bar" variant ensures existing usage continues to work
- Custom tooltip content provides flexibility while keeping the default behavior intact
- The shape variant lookup is efficient and extensible
Also applies to: 142-155
removed the unknown props Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
removed console log Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/propel/src/charts/bar-chart/bar.tsx (1)
21-21: Consider using a more specific type for payload.The
payload: anytype reduces type safety. Consider using a generic type parameter or a more specific interface.-interface TShapeProps { +interface TShapeProps<T = any> { x: number; y: number; width: number; height: number; dataKey: string; - payload: any; + payload: T; opacity?: number; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/propel/src/charts/bar-chart/bar.tsx(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (5)
packages/propel/src/charts/bar-chart/bar.tsx (5)
7-12: Well-structured constants for styling consistency.The constants are properly defined and will help maintain consistent styling across the chart components.
44-54: Solid SVG path generation utility.The
getBarPathfunction correctly generates SVG paths for rounded rectangles with configurable top and bottom border radii.
56-70: Clean and reusable PercentageText component.Good abstraction for percentage text rendering with proper SVG attributes and styling.
158-177: Well-designed factory pattern for shape variants.The
createShapeVariantfactory properly handles optional border radius functions and fill type variations, enabling flexible shape variant creation.
179-186: Clean export structure with proper component naming.The
barShapeVariantsexport provides a clean API for dynamic shape selection, and the display names follow React best practices for debugging.
JayashTripathy
changed the title
|
Caution No docstrings were generated. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/propel/src/charts/bar-chart/bar.tsx (2)
15-33: Improve type safety by avoidinganytypes.The type definitions provide good structure, but using
anyreduces type safety benefits.Consider creating more specific types:
interface TShapeProps { x: number; y: number; width: number; height: number; dataKey: string; - payload: any; + payload: TChartData<string, string>; opacity?: number; } interface TBarProps extends TShapeProps { - fill: string | ((payload: any) => string); + fill: string | ((payload: TChartData<string, string>) => string); stackKeys: string[]; textClassName?: string; showPercentage?: boolean; showTopBorderRadius?: boolean; showBottomBorderRadius?: boolean; dotted?: boolean; }
121-150: CustomBarLollipop implementation looks solid, but verify text positioning.The lollipop component properly addresses previous review issues (removed console.log, eliminated invalid SVG attributes, uses PercentageText component). However, the percentage text positioning at
y={y}might cause overlap with the circle.Consider adjusting the text position:
{showPercentage && ( - <PercentageText x={x + width / 2} y={y} percentage={currentBarPercentage} className={textClassName} /> + <PercentageText x={x + width / 2} y={y - DEFAULT_LOLLIPOP_CIRCLE_RADIUS - 4} percentage={currentBarPercentage} className={textClassName} /> )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/propel/src/charts/bar-chart/bar.tsx(3 hunks)packages/types/src/charts/index.d.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/types/src/charts/index.d.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (6)
packages/propel/src/charts/bar-chart/bar.tsx (6)
7-13: Well-defined constants improve maintainability.The constants are properly named and provide clear configuration for the chart styling. Good separation of concerns.
45-55: Excellent utility function for SVG path generation.The
getBarPathfunction provides a clean abstraction for generating rounded rectangle paths, improving code reusability and maintainability.
57-71: Good abstraction for consistent text rendering.The
PercentageTextcomponent addresses the previous review concern about inconsistent text rendering between variants. This promotes consistency and reusability.
74-119: Well-structured CustomBar component with proper optimizations.The component properly uses React.memo for performance, integrates the new utility functions, and maintains clean separation of concerns. The conditional rendering and text positioning logic is well-implemented.
152-181: Excellent factory pattern implementation.The
createShapeVariantfactory andbarShapeVariantsmapping provide a clean, extensible architecture for managing different chart variants. This design makes it easy to add new variants while maintaining consistency.
184-185: Good practice setting display names for debugging.Setting display names on React components improves the debugging experience in React DevTools.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/propel/src/charts/bar-chart/bar.tsx (1)
146-146: Consider adjusting percentage text positioning for better readability.The percentage text is currently positioned at
y={y}, which places it directly on the circle center. This may cause readability issues due to overlap with the circle.Consider positioning the text slightly below or above the circle:
- <PercentageText x={x + width / 2} y={y} percentage={currentBarPercentage} className={textClassName} /> + <PercentageText x={x + width / 2} y={y - DEFAULT_LOLLIPOP_CIRCLE_RADIUS - 4} percentage={currentBarPercentage} className={textClassName} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/propel/src/charts/bar-chart/bar.tsx(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (8)
packages/propel/src/charts/bar-chart/bar.tsx (8)
4-13: Well-structured constants and imports.The new constants provide clear configuration values for the component dimensions and styling, improving maintainability and consistency.
14-33: Clean type definitions with proper separation of concerns.The
TShapePropsandTBarPropsinterfaces provide good type safety while maintaining flexibility. The separation between base shape properties and bar-specific properties follows good design principles.
45-55: Efficient SVG path generation utility.The
getBarPathfunction correctly generates SVG path strings for rounded rectangles using quadratic Bézier curves. This eliminates code duplication and provides a reusable utility for consistent border radius rendering.
57-71: Good abstraction for percentage text rendering.The
PercentageTextcomponent provides consistent styling and positioning for percentage labels. Usingfill="currentColor"allows proper color inheritance from the parent context.
74-119: Improved CustomBar implementation with proper typing.The refactored component uses the new type definitions and utility functions effectively. The conditional rendering logic for percentage text is well-implemented, considering both height constraints and percentage validity.
121-150: Well-implemented lollipop shape variant.The
CustomBarLollipopcomponent correctly implements the lollipop visualization with proper SVG elements. The conditional dotted line feature adds good flexibility, and the fill function support maintains consistency with the standard bar variant.
152-186: Excellent factory pattern implementation for shape variants.The
createShapeVariantfactory function provides a clean, extensible approach for creating different shape variants while maintaining consistent prop handling. The type-safe mapping inbarShapeVariantsensures proper variant selection and component rendering.
189-190: Proper component display names for debugging.Setting display names on the components improves the debugging experience in React DevTools.
* feat: enhance bar chart component with shape variants and custom tooltip * Update packages/propel/src/charts/bar-chart/bar.tsx removed the unknown props Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update packages/propel/src/charts/bar-chart/bar.tsx removed console log Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * refactor: replace inline percentage text with PercentageText component in bar chart * Added new variant - lollipop-dotted * added some comments --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
3 participants
Description
This PR extends the bar chart component with lollipop shape variant
Type of Change
Summary by CodeRabbit