recharts
feat: add ability to set static domains to improve UX for toggling data
Closes #3099
This PR introduces the includeHidden prop to Axis elements. Setting this to true ensures that all datapoints within a chart contribute to its domain calculation, even when they are hidden. This allows developers to improve the end-user experience for charts that allow toggling datapoints, as it can improve their ability to make comparisons on narrower sets of data. Documentation for this feature is added in https://github.com/recharts/recharts.org/pull/223.
Thank you for your contribution! Could you please provide a test plan with a toy dataset, that I can use to verify that this works as expected?
Further, it would be great if you could split up the PR into (a) refactoring and (b) adding a feature, which would reduce the blast radius and decrease the chance of having to revert everything if a problem comes up.
@ckifer @nikolasrieble thanks for your comments!
I've just split the changes out into multiple PRs, and have added an interactive demo of these changes to the demo directory, so you should be able to see the old and new behaviour side-by-side to verify yourselves. Let me know if you'd like any other changes.
Also is this a valid prop for all axis types?
@ckifer it is a valid prop for any axis that extend BaseAxisProps which, as it stands, is only xAxis and yAxis
@ckifer @nikolasrieble I've just updated this PR to fix conflicts, and move to an RTL/Jest test. I've also replaced the demo with a pair of stories, which should make the behaviour easier to observe. Is there anything else you need from me at this point? I'd love to see this feature added soon!
Also as a side-note, the pre-commit and pre-push hooks are consistently failing for me, with errors seemingly unrelated to the changes in this PR. Sure you'd be are aware if this is a shared problem, but flagging just in case it's an issue with my set up.
hey @neefrehman
I hear you, but wanted to wait to merge this until the Jest migration was done at least. I will pull down these changes and check them out, hopefully we can get this in for next minor release (2.5).
The pre-commit and pre-push hooks work for me and assuming they work for Niko as well, in what way are they failing for you?
@ckifer Thanks!
The pre-push failures I had were all lint related. It looks like the storybook file I added had some issues that my editor took a while to recognise!
The merge commit also triggered a handful of linter errors for test files, which were triggered by lint-staged. My subsequent commit didn't trigger them.
Okay so the hooks themselves are working - good.
The demo pages have all sorts of lint errors so feel free to skip verification for those but only those. The storybook files should be linted, but there's possibility we miss things on merge.
Relevant docs:
- https://recharts.org/en-US/api/XAxis#includeHidden
- https://recharts.org/en-US/api/YAxis#includeHidden
