Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Highlight & Zoom Broken in v2.4 #3369

Closed
1 task done
swively opened this issue Feb 14, 2023 · 9 comments · Fixed by #3375
Closed
1 task done

Highlight & Zoom Broken in v2.4 #3369

swively opened this issue Feb 14, 2023 · 9 comments · Fixed by #3375
Labels
bug General bug label

Comments

@swively
Copy link

swively commented Feb 14, 2023

  • I have searched the issues of this repository and believe that this is not a duplicate.

Reproduction link

https://recharts.org/en-US/examples/HighlightAndZoomLineChart

Steps to reproduce

Check out the recharts.org example page for Highlight and Zoom Line Chart, its broken there as well.
Once you zoom in any amount, all inner data points are no longer highlightable (tooltip or click), only the first and last data points, so you can no longer interact with the charts and tooltips are broken

What is expected?

You should be able to further zoom, as well as see a tooltip for each datapoint in the frame.

What is actually happening?

All inner datapoints are no longer accessible

Environment Info
Recharts v2.4.1
React 18.2
System OSX
Browser Chrome
@ckifer ckifer added the bug General bug label label Feb 14, 2023
@ckifer
Copy link
Member

ckifer commented Feb 14, 2023

Thanks for the report, I see what you mean - will take a look as soon as I can

@ckifer
Copy link
Member

ckifer commented Feb 14, 2023

Here is a more up to date codesandbox using hooks - https://codesandbox.io/s/highlight-zomm-line-chart-forked-holzzh?file=/src/App.tsx

Same behavior though

ckifer added a commit that referenced this issue Feb 15, 2023
<!--- Provide a general summary of your changes in the Title above -->

## Description

<!--- Describe your changes in detail -->
- Add HighlightAndZoom example from an issue and from the website, use
hooks version

## Related Issue

<!--- This project only accepts pull requests related to open issues -->
<!--- If suggesting a new feature or change, please discuss it in an
issue first -->
<!--- If fixing a bug, there should be an issue describing it with steps
to reproduce -->
<!--- Please link to the issue here: -->
#3369 - issue with currently
broken zoom functionality
#3333 - example from this
issue

## Motivation and Context

<!--- Why is this change required? What problem does it solve? -->
- adds a new example to test this ffunctionality on

## How Has This Been Tested?

<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, and the tests you ran
to -->
<!--- see how your change affects other areas of the code, etc. -->
run storybook

## Screenshots (if appropriate):

## Types of changes

<!--- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->

- [ ] 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:

<!--- Go over all the following points, and put an `x` in all the boxes
that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->

- [x] 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.
- [x] All new and existing tests passed.

Co-authored-by: Coltin Kifer <ckifer@amazon.com>
@ckifer
Copy link
Member

ckifer commented Feb 15, 2023

Added an example in the storybook here - https://master--63da8268a0da9970db6992aa.chromatic.com/?path=/story/examples-linechart-highlightandzoom--highlight-and-zoom

Will use this to debug (@Yilun-Sun @nikolasrieble @proke03 also feel free to use this to debug if you want to help before I can get around to it)

@ckifer
Copy link
Member

ckifer commented Feb 15, 2023

Somewhere after d8ea0c8 and before 322c4b6

@ckifer
Copy link
Member

ckifer commented Feb 15, 2023

Between 2732d71 and 322c4b6

@ckifer
Copy link
Member

ckifer commented Feb 15, 2023

Winner winner I believe 642dd59

or PR

#3293

@ckifer
Copy link
Member

ckifer commented Feb 15, 2023

tooltipTicks go from 20 to 2 here

const { orderedTooltipTicks: ticks, tooltipAxis: axis, tooltipTicks } = state;
after zoom

image
Makes sense why the issue occurs

@ckifer
Copy link
Member

ckifer commented Feb 15, 2023

if allowDataOverflow is set to false for the XAxis only then the Y axis behaves as it should. Issue is with tick calculation when allowDataOverflow is true on the XAxis

This is even called out in the PR desc

We could short-circuit the domain creation in one scenario:

  • allowDataOverflow is set to true
  • the specified domain (start AND end) was provided via props with two values

Action item will be to look at the PR diff to figure out why exactly this isn't behaving as it should. Probably a larger issue :/

@ckifer
Copy link
Member

ckifer commented Feb 15, 2023

Released 2.4.2 - check out https://codesandbox.io/s/highlight-zomm-line-chart-forked-holzzh?file=/src/App.tsx and let me know if you have any other issues.

Thanks @swively

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug General bug label
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants