feat(BarChart): expose bar size and gap props#450
Conversation
| @@ -114,6 +114,38 @@ interface BarChartProps { | |||
| * @see https://recharts.org/en-US/api/Bar#activeBar | |||
| */ | |||
There was a problem hiding this comment.
What: Consider validating the types for barSize, barCategoryGap, and barGap. Currently, they accept a number or a string but lack type enforcement that could prevent unexpected values.
Why: Validating prop types helps to catch bugs early and maintain consistent usage of the props, which is important for debugging and ensuring the component behaves as expected. Incorrect values can lead to runtime issues or unexpected styling in the rendered chart.
How: Implement PropTypes or TypeScript runtime checks to validate incoming values. For example, if using TypeScript, you could create a custom type guard that ensures that values are strictly either numbers or valid percentage strings before passing them to the Recharts components.
There was a problem hiding this comment.
Matches the existing pattern in this file — activeBar, xAxisFontColor, borderRadius, and other typed props rely on the TypeScript signature for compile-time validation, no runtime PropTypes. The ?: number | string types here are sufficient and stay consistent with the rest of the component.
| @@ -143,6 +175,10 @@ const BarChart = ( { | |||
| yAxisProps, | |||
| tooltipProps, | |||
| activeBar, | |||
There was a problem hiding this comment.
What: Ensure props related to styling don't overwrite important CSS properties set by Recharts. Double-check if there's a possibility of conflicting with existing styling if users have set custom styles on the Bar or BarChart components.
Why: Inconsistent styles can result in visual bugs or user dissatisfaction. By being cautious about style overrides, you ensure better integration with users' custom designs.
How: You might want to document the behavior when users custom styles and clarify how the new props might affect them. It could also be beneficial to include safeguards or fallbacks in case of invalid values.
There was a problem hiding this comment.
No CSS is involved here — these props are forwarded to Recharts which translates them into SVG attributes (rect width/height) on the bar elements. There are no styles being overridden so there is nothing for a custom user style to conflict with.
| @@ -171,6 +207,8 @@ const BarChart = ( { | |||
| data={ data } | |||
There was a problem hiding this comment.
What: Document the implications of setting a maxBarSize or barSize when combined with barCategoryGap and barGap. Provide improvement in your documentation for future developers.
Why: Understanding how different props interact with each other is crucial for developers to use the component correctly without unintended side effects. Missing context can lead to inappropriate usage and confusion.
How: Add examples in your JSDoc comments showing how these props might interact, and reference potential edge cases developers should be aware of.
There was a problem hiding this comment.
Each prop is documented with a @see link to the corresponding Recharts API page where the interaction is described. Duplicating that documentation here would re-document Recharts behavior — the existing pattern in this file (e.g., activeBar at line 113-116) does the same: short JSDoc + @see link.
555ea19 to
40b22d6
Compare
|
Closing this PR as it is affected, and I have cleared the worktree previously so there is no local copy for the branch. |
Summary
Recharts'
BarChartandBarcomponents accept four standard props for controlling bar width and spacing —maxBarSize,barSize,barCategoryGap, andbarGap. The Force UI wrapper currently swallows all of them, so consumers cannot cap or shrink bar widths without forking the wrapper.This PR forwards them through:
barCategoryGapandbarGapgo onto the inner<BarChart>maxBarSizeandbarSizego onto each<Bar>All four props are optional. When unset, Recharts' default sizing applies, so existing behavior is preserved for current consumers.
Why
When using
<BarChart>for charts with a small number of data points, the default sizing fills the full band and produces visually heavy bars. There is no current API to cap or set bar width without re-implementing the wrapper.Changes
maxBarSize?: number(forwarded to each<Bar>)barSize?: number | string(forwarded to each<Bar>)barCategoryGap?: number | string(forwarded to inner<BarChart>)barGap?: number | string(forwarded to inner<BarChart>)Testing
Verified locally by patching the compiled
dist/components/bar-chart/bar-chart.es.jsin a consuming project and passingmaxBarSize={ 32 }— bars correctly cap at 32px wide regardless of chart width.maxBarSizeset produces visually thinner barsBackwards compatibility
All new props are optional and default to
undefined, which Recharts treats as "use auto sizing". No existing consumers need to change anything.