feat: Implement activeBar for Bar component #3756

andrewangelle · 2023-09-13T13:24:21Z

Description

Implementing support for activeBar prop on Bar component. I attempted to follow the same behavior of activeDot for a Line.

Related Issue

#3723

Motivation and Context

Feature request for a consistent interface between Line/Bar for customizing the active element that renders via the Tooltip

How Has This Been Tested?

Unit tests and storybook

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my changes.
All new and existing tests passed.

andrewangelle · 2023-09-13T13:24:35Z

#3723

andrewangelle · 2023-09-13T13:30:16Z

Question for maintainers:

When i checked off I have updated the documentation accordingly., I was referring the prop documentation in storybook. I noticed that there is a separate repo for the site documentation for recharts.org. Will there need to be a separate PR to update documentation there?

PR opened in recharts.org repo

charpeni · 2023-09-13T20:02:41Z

Yes, API definitions are defined here: https://github.com/recharts/recharts.org/tree/master/src/docs/api.

Here's an example of a corresponding recharts.org pull request: recharts/recharts.org#271.

ckifer · 2023-09-13T20:12:16Z

Yeah sorry for not getting back. For changes like this for now we'll have to update both - the storybook will eventually replace the custom docs site because the docs site is a pain to update.

ckifer

LGTM overall, though I wish we could use less any (I know it isn't entirely feasible with what you have to work with).

One clarification needed on if we're able to change style properties

src/chart/generateCategoricalChart.tsx

ckifer

Concerned about the type change to name, otherwise LGTM

src/cartesian/Bar.tsx

src/chart/generateCategoricalChart.tsx

ckifer

🚀

ckifer · 2023-09-15T23:41:10Z

Is it just me or is this story realllly slow?

I pulled it down to mess with it - insanely laggy :/

Second one is good though

nikolasrieble · 2023-09-16T07:29:37Z

Great contribution, thank you @andrewangelle! If you would like to follow up, you could consider adding this ability also to other charts. To my best knowledge we know have activeDot and activeBar. It seems consistent to also extend this to other elements such as Funnel.

nikolasrieble · 2023-09-16T07:31:12Z

src/chart/generateCategoricalChart.tsx

+        strokeWidth: 2,
+        stroke: '#fff',


Nit: It would be nice to extract hardcoded constants named to the top of the file (we might even want to reuse constant across files as consistent defaults).

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [recharts](https://togithub.com/recharts/recharts) | [`2.8.0` -> `2.9.0`](https://renovatebot.com/diffs/npm/recharts/2.8.0/2.9.0) | [![age](https://developer.mend.io/api/mc/badges/age/npm/recharts/2.9.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/recharts/2.9.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/recharts/2.8.0/2.9.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/recharts/2.8.0/2.9.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>recharts/recharts (recharts)</summary> ### [`v2.9.0`](https://togithub.com/recharts/recharts/releases/tag/v2.9.0) [Compare Source](https://togithub.com/recharts/recharts/compare/v2.8.0...v2.9.0) #### What's Changed Quite a lot this minor release! We sent out a cry for help and many answered - thank you so much for that 🙌🏼 This release aims at internal maintainability, long lingering bugs, and needed improvements. Highlights include equidistant tick improvements, an active bar feature, and an ~85k/~9kb (gzipped) bundle size reduction 🚀 ##### Feat - `Bar`: Implement activeBar for Bar component by [@andrewangelle](https://togithub.com/andrewangelle) in [recharts/recharts#3756 - `CartesianGrid`: add `syncWithticks`, `horizonalValues`, and `verticalValues` props to allow more grid line customization by [@morozovkirill](https://togithub.com/morozovkirill) in [recharts/recharts#3746 solves [recharts/recharts#2153 - `CartesianAxis`: Improve interval option 'equidistantPreserveStart' by [@nikolasrieble](https://togithub.com/nikolasrieble) in [recharts/recharts#3768 - `CartesianAxis`: Throw an invariant when axisIds do not match between chart and axis components by [@ckifer](https://togithub.com/ckifer) - `Brush`: add onDragEnd event to Brush component by [@simkesd](https://togithub.com/simkesd) in [recharts/recharts#3774 ##### Fix - Active Shape improvements - `Funnel`: activeShape should work with Tooltip by [@andrewangelle](https://togithub.com/andrewangelle) in [recharts/recharts#3772 - `Scatter`: activeShape should work with Tooltip by [@andrewangelle](https://togithub.com/andrewangelle) in [recharts/recharts#3839 - `Pie`: activeShape should work with Tooltip by [@andrewangelle](https://togithub.com/andrewangelle) in [recharts/recharts#3818 - `RadialBar`: activeShape should work with Tooltip by [@andrewangelle](https://togithub.com/andrewangelle) in [recharts/recharts#3803 - `CartesianGrid`: Remove offset attribute from lines by [@branberry](https://togithub.com/branberry) in [recharts/recharts#3854 solves [recharts/recharts#3810 - `ResponsiveContainer`: style prop is now passed down correctly by [@d-gottlieb](https://togithub.com/d-gottlieb) in [recharts/recharts#3726 - `Legend`: "Functions are not valid as a React child" error in <Legend/> when a function is passed as the payload [#3749](https://togithub.com/recharts/recharts/issues/3749) by [@chris-mcdonald-dev](https://togithub.com/chris-mcdonald-dev) in [recharts/recharts#3750 - `Tooltip`: Fix tooltip position when container uses transform scale by [@MateuszTrN](https://togithub.com/MateuszTrN) in [recharts/recharts#3748 - `Tooltip`: Tooltip does not include data from all charts when a separate dataset is passed to chart prop data and specified on Line/Area/etc prop data by [@andrewangelle](https://togithub.com/andrewangelle) in [recharts/recharts#3733 fixes [recharts/recharts#3669 ##### Refactor Impossible to mention all of the great refactoring done this release thanks to [@PavelVanecek](https://togithub.com/PavelVanecek) (33 PRs in one month!!) and others! Notable improvements include (non-breaking) type safety enhancements and source code file size reductions, and unit test improvements that will help reduce regressions. ##### Chore - Upgrade react-smooth to 2.0.5 - potentially fixes [recharts/recharts#1135 (edit: this was already fixed) - Add performance testing tool by [@PavelVanecek](https://togithub.com/PavelVanecek) in [recharts/recharts#3829 - remove reduceCSSCalc by [@HHongSeungWoo](https://togithub.com/HHongSeungWoo) in [recharts/recharts#3820 ##### Storybook - Add storybook-addon-performance by [@PavelVanecek](https://togithub.com/PavelVanecek) in [recharts/recharts#3826 - many storybook improvements and fixes #### 🚀 New Contributors (!!) 🚀 - [@d-gottlieb](https://togithub.com/d-gottlieb) made their first contribution in [recharts/recharts#3726 - [@andrewangelle](https://togithub.com/andrewangelle) made their first contribution in [recharts/recharts#3733 - [@wanisramdani](https://togithub.com/wanisramdani) made their first contribution in [recharts/recharts#3751 - [@MateuszTrN](https://togithub.com/MateuszTrN) made their first contribution in [recharts/recharts#3748 - [@chris-mcdonald-dev](https://togithub.com/chris-mcdonald-dev) made their first contribution in [recharts/recharts#3750 - [@PavelVanecek](https://togithub.com/PavelVanecek) made their first contribution in [recharts/recharts#3759 - [@simkesd](https://togithub.com/simkesd) made their first contribution in [recharts/recharts#3774 - [@samtmorgan](https://togithub.com/samtmorgan) made their first contribution in [recharts/recharts#3778 - [@Shashangbhagat](https://togithub.com/Shashangbhagat) made their first contribution in [recharts/recharts#3786 - [@morozovkirill](https://togithub.com/morozovkirill) made their first contribution in [recharts/recharts#3746 - [@branberry](https://togithub.com/branberry) made their first contribution in [recharts/recharts#3854 - [@HHongSeungWoo](https://togithub.com/HHongSeungWoo) made their first contribution in [recharts/recharts#3820 **Full Changelog**: recharts/recharts@v2.8.0...v2.9.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/SAP/ui5-webcomponents-react).  --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Lukas Harbarth <lukas.harbarth@sap.com>

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [recharts](https://togithub.com/recharts/recharts) | [`2.8.0` -> `2.9.3`](https://renovatebot.com/diffs/npm/recharts/2.8.0/2.9.3) | [![age](https://developer.mend.io/api/mc/badges/age/npm/recharts/2.9.3?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/recharts/2.9.3?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/recharts/2.8.0/2.9.3?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/recharts/2.8.0/2.9.3?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>recharts/recharts (recharts)</summary> ### [`v2.9.3`](https://togithub.com/recharts/recharts/releases/tag/v2.9.3) [Compare Source](https://togithub.com/recharts/recharts/compare/v2.9.2...v2.9.3) #### Fix `Brush`: Fix an issue where after 2.9 `Brush` does not correctly slice data when using `Line` components - fixes [recharts/recharts#3929 - thank you [@HHongSeungWoo](https://togithub.com/HHongSeungWoo) ### [`v2.9.2`](https://togithub.com/recharts/recharts/releases/tag/v2.9.2) [Compare Source](https://togithub.com/recharts/recharts/compare/v2.9.1...v2.9.2) Fix another TS issue from 2.9. ##### Fix - `Line/ActiveDot`: Fix breaking type change for the `onClick` function of `activeDot` on `Line` - this resolves [recharts/recharts#3922 - thank you [@andrewangelle](https://togithub.com/andrewangelle) for the quick turnaround ### [`v2.9.1`](https://togithub.com/recharts/recharts/releases/tag/v2.9.1) [Compare Source](https://togithub.com/recharts/recharts/compare/v2.9.0...v2.9.1) Bug fixes following 2.9.0 #### Fix - `TypeScript`: fix breaking change in `ActiveShape` types - fixes [recharts/recharts#3911 - thanks [@andrewangelle](https://togithub.com/andrewangelle) - `CartesianGrid`: fix breaking change where you could no longer render `CartesianGrid` without a y-axis - fixes [recharts/recharts#3907 - thanks [@akamfoad](https://togithub.com/akamfoad) - `Line`: fix infinite loop when `strokeDasharray` is `'0'` on `Line` - fixes [recharts/recharts#3899 (and maybe others) **Full Changelog**: recharts/recharts@v2.9.0...v2.9.1 ### [`v2.9.0`](https://togithub.com/recharts/recharts/releases/tag/v2.9.0) [Compare Source](https://togithub.com/recharts/recharts/compare/v2.8.0...v2.9.0) #### What's Changed Quite a lot this minor release! We sent out a cry for help and many answered - thank you so much for that 🙌🏼 This release aims at internal maintainability, long lingering bugs, and needed improvements. Highlights include equidistant tick improvements, an active bar feature, and an ~85k/~9kb (gzipped) bundle size reduction 🚀 ##### Feat - `Bar`: Implement activeBar for Bar component by [@andrewangelle](https://togithub.com/andrewangelle) in [recharts/recharts#3756 - `CartesianGrid`: add `syncWithticks`, `horizonalValues`, and `verticalValues` props to allow more grid line customization by [@morozovkirill](https://togithub.com/morozovkirill) in [recharts/recharts#3746 solves [recharts/recharts#2153 - `CartesianAxis`: Improve interval option 'equidistantPreserveStart' by [@nikolasrieble](https://togithub.com/nikolasrieble) in [recharts/recharts#3768 - `CartesianAxis`: Throw an invariant when axisIds do not match between chart and axis components by [@ckifer](https://togithub.com/ckifer) - `Brush`: add onDragEnd event to Brush component by [@simkesd](https://togithub.com/simkesd) in [recharts/recharts#3774 ##### Fix - Active Shape improvements - `Funnel`: activeShape should work with Tooltip by [@andrewangelle](https://togithub.com/andrewangelle) in [recharts/recharts#3772 - `Scatter`: activeShape should work with Tooltip by [@andrewangelle](https://togithub.com/andrewangelle) in [recharts/recharts#3839 - `Pie`: activeShape should work with Tooltip by [@andrewangelle](https://togithub.com/andrewangelle) in [recharts/recharts#3818 - `RadialBar`: activeShape should work with Tooltip by [@andrewangelle](https://togithub.com/andrewangelle) in [recharts/recharts#3803 - `CartesianGrid`: Remove offset attribute from lines by [@branberry](https://togithub.com/branberry) in [recharts/recharts#3854 solves [recharts/recharts#3810 - `ResponsiveContainer`: style prop is now passed down correctly by [@d-gottlieb](https://togithub.com/d-gottlieb) in [recharts/recharts#3726 - `Legend`: "Functions are not valid as a React child" error in <Legend/> when a function is passed as the payload [#3749](https://togithub.com/recharts/recharts/issues/3749) by [@chris-mcdonald-dev](https://togithub.com/chris-mcdonald-dev) in [recharts/recharts#3750 - `Tooltip`: Fix tooltip position when container uses transform scale by [@MateuszTrN](https://togithub.com/MateuszTrN) in [recharts/recharts#3748 - `Tooltip`: Tooltip does not include data from all charts when a separate dataset is passed to chart prop data and specified on Line/Area/etc prop data by [@andrewangelle](https://togithub.com/andrewangelle) in [recharts/recharts#3733 fixes [recharts/recharts#3669 ##### Refactor Impossible to mention all of the great refactoring done this release thanks to [@PavelVanecek](https://togithub.com/PavelVanecek) (33 PRs in one month!!) and others! Notable improvements include (non-breaking) type safety enhancements and source code file size reductions, and unit test improvements that will help reduce regressions. ##### Chore - Upgrade react-smooth to 2.0.5 - potentially fixes [recharts/recharts#1135 (edit: this was already fixed) - Add performance testing tool by [@PavelVanecek](https://togithub.com/PavelVanecek) in [recharts/recharts#3829 - remove reduceCSSCalc by [@HHongSeungWoo](https://togithub.com/HHongSeungWoo) in [recharts/recharts#3820 ##### Storybook - Add storybook-addon-performance by [@PavelVanecek](https://togithub.com/PavelVanecek) in [recharts/recharts#3826 - many storybook improvements and fixes #### 🚀 New Contributors (!!) 🚀 - [@d-gottlieb](https://togithub.com/d-gottlieb) made their first contribution in [recharts/recharts#3726 - [@andrewangelle](https://togithub.com/andrewangelle) made their first contribution in [recharts/recharts#3733 - [@wanisramdani](https://togithub.com/wanisramdani) made their first contribution in [recharts/recharts#3751 - [@MateuszTrN](https://togithub.com/MateuszTrN) made their first contribution in [recharts/recharts#3748 - [@chris-mcdonald-dev](https://togithub.com/chris-mcdonald-dev) made their first contribution in [recharts/recharts#3750 - [@PavelVanecek](https://togithub.com/PavelVanecek) made their first contribution in [recharts/recharts#3759 - [@simkesd](https://togithub.com/simkesd) made their first contribution in [recharts/recharts#3774 - [@samtmorgan](https://togithub.com/samtmorgan) made their first contribution in [recharts/recharts#3778 - [@Shashangbhagat](https://togithub.com/Shashangbhagat) made their first contribution in [recharts/recharts#3786 - [@morozovkirill](https://togithub.com/morozovkirill) made their first contribution in [recharts/recharts#3746 - [@branberry](https://togithub.com/branberry) made their first contribution in [recharts/recharts#3854 - [@HHongSeungWoo](https://togithub.com/HHongSeungWoo) made their first contribution in [recharts/recharts#3820 **Full Changelog**: recharts/recharts@v2.8.0...v2.9.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - "after 4pm every weekday" in timezone Europe/Paris, Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/specfy/specfy).  Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

## Description Implementing support for `activeBar` prop on Bar component. I attempted to follow the same behavior of activeDot for a Line.  ## Related Issue recharts#3723     ## Motivation and Context Feature request for a consistent interface between Line/Bar for customizing the active element that renders via the Tooltip  ## How Has This Been Tested? Unit tests and storybook    ## Screenshots (if appropriate): <img width="964" alt="Screenshot 2023-09-13 at 9 19 29 AM" src="https://github.com/recharts/recharts/assets/24420033/582fd859-edae-43f5-a346-ae5c358e7515"> ## Types of changes  - [ ] Bug fix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) ## Checklist:   - [x] My code follows the code style of this project. - [x] My change requires a change to the documentation. - [x] I have updated the documentation accordingly. - [x] I have added tests to cover my changes. - [x] All new and existing tests passed.

ckifer · 2024-02-01T02:17:21Z

src/cartesian/Bar.tsx

@@ -115,6 +121,7 @@ export class Bar extends PureComponent<Props, State> {
    hide: false,
    data: [] as BarRectangleItem[],
    layout: 'vertical',
+    activeBar: true,


I'm thinking this shouldn't be true by default #4103

## Description  We, a few months ago, introduced changes for a consistent interface of `activeShape` #3756 In doing so we created a regression when a bar `shape` is different than the default activeBar of `true` (and therefore a regular Rectangle component) This causes components to change to Bar on hover - see [sandbox](https://codesandbox.io/p/sandbox/mixed-bar-chart-forked-2wdtzj?file=%2Fpackage.json%3A11%2C21) Also noticed TS freaking out in some of our tests. This.. isn't good and means that the usability of some of these things is quite terrible ... ## Related Issue     #4103 and #4101 ## Motivation and Context  - Fix regression ## How Has This Been Tested?    - Tests continue to pass besides the ones that should have broken - Test by hand, everything else is the same ## Screenshots (if appropriate): <img width="453" alt="image" src="https://github.com/recharts/recharts/assets/25180830/2e27eeb8-5c38-4f5a-b05a-94f91ae60fb8"> Still works Does not break custom shape <img width="207" alt="image" src="https://github.com/recharts/recharts/assets/25180830/23f35287-db95-4157-9a76-c5971f0f1b76"> ## Types of changes  - [X] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) ## Checklist:   - [X] My code follows the code style of this project. - [ ] My change requires a change to the documentation. - [ ] I have updated the documentation accordingly. - [X] I have added tests to cover my changes. - [ ] I have added a storybook story or extended an existing story to show my changes - [X] All new and existing tests passed. Co-authored-by: Coltin Kifer <ckifer@amazon.com>

feat: Implement activeBar for Bar component

3872f44

andrewangelle mentioned this pull request Sep 13, 2023

include activeBar in Bar docs recharts/recharts.org#278

Merged

ckifer reviewed Sep 13, 2023

View reviewed changes

src/chart/generateCategoricalChart.tsx Outdated Show resolved Hide resolved

src/chart/generateCategoricalChart.tsx Outdated Show resolved Hide resolved

src/chart/generateCategoricalChart.tsx Show resolved Hide resolved

fix: type assignments for activeBar changes

5b88c92

ckifer requested changes Sep 15, 2023

View reviewed changes

src/cartesian/Bar.tsx Outdated Show resolved Hide resolved

src/chart/generateCategoricalChart.tsx Show resolved Hide resolved

fix: revert type change to name

3801a87

ckifer approved these changes Sep 15, 2023

View reviewed changes

ckifer merged commit 0a0dbd9 into recharts:master Sep 15, 2023
5 checks passed

ckifer mentioned this pull request Sep 15, 2023

Consistent interface: Line / Bar - activeDot/ activeBar #3723

Closed

nikolasrieble reviewed Sep 16, 2023

View reviewed changes

andrewangelle deleted the issue.3723 branch September 24, 2023 14:48

ckifer reviewed Feb 1, 2024

View reviewed changes

ckifer mentioned this pull request Feb 6, 2024

fix: activeBar shouldn't be true by default #4139

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Implement activeBar for Bar component #3756

feat: Implement activeBar for Bar component #3756

andrewangelle commented Sep 13, 2023

andrewangelle commented Sep 13, 2023

andrewangelle commented Sep 13, 2023 •

edited

Loading

charpeni commented Sep 13, 2023

ckifer commented Sep 13, 2023

ckifer left a comment

ckifer left a comment

ckifer left a comment

ckifer commented Sep 15, 2023 •

edited

Loading

nikolasrieble commented Sep 16, 2023

nikolasrieble Sep 16, 2023

ckifer Feb 1, 2024

feat: Implement activeBar for Bar component #3756

feat: Implement activeBar for Bar component #3756

Conversation

andrewangelle commented Sep 13, 2023

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

andrewangelle commented Sep 13, 2023

andrewangelle commented Sep 13, 2023 • edited Loading

charpeni commented Sep 13, 2023

ckifer commented Sep 13, 2023

ckifer left a comment

Choose a reason for hiding this comment

ckifer left a comment

Choose a reason for hiding this comment

ckifer left a comment

Choose a reason for hiding this comment

ckifer commented Sep 15, 2023 • edited Loading

nikolasrieble commented Sep 16, 2023

nikolasrieble Sep 16, 2023

Choose a reason for hiding this comment

ckifer Feb 1, 2024

Choose a reason for hiding this comment

andrewangelle commented Sep 13, 2023 •

edited

Loading

ckifer commented Sep 15, 2023 •

edited

Loading