Skip to content

Fix treemap tooltip crash#3579

Merged
ckifer merged 2 commits into
recharts:masterfrom
ckifer:fix/treemap-tt
May 10, 2023
Merged

Fix treemap tooltip crash#3579
ckifer merged 2 commits into
recharts:masterfrom
ckifer:fix/treemap-tt

Conversation

@ckifer

@ckifer ckifer commented May 10, 2023

Copy link
Copy Markdown
Member

Description

Bug released in 2.6 with refactor of Tooltip to a function component. Add optionals to prevent crashing when coordinate is null.

This resulted in other errors surrounding event pooling https://legacy.reactjs.org/docs/legacy-event-pooling.html

This isn't an issue in React 17+ but throws errors in react <17 so call e.persist() in the offending functions in Treemap - this resolves errors in React 16 and has no effect in React 17 (see https://legacy.reactjs.org/blog/2020/08/10/react-v17-rc.html#no-event-pooling)

Related Issue

#3578

Motivation and Context

Fix 2.6.0 bug #3578

How Has This Been Tested?

Screenshots (if appropriate):

image

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.

@ckifer ckifer changed the title Fix/treemap tt Fix treemap tooltip crash May 10, 2023
Comment on lines +95 to +103
render: (args: Record<string, any>) => {
return (
<ResponsiveContainer width="100%" height={400}>
<Treemap {...args}>
<Tooltip />
</Treemap>
</ResponsiveContainer>
);
},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could add a play function to hover over the chart, and then verify that the tooltip is rendered. Thus such an issue could be auto-detected the next time.

@nikolasrieble nikolasrieble left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to go for the fix - one suggestion for improving the test - might make sense to invest the extra time now.

@ckifer

ckifer commented May 10, 2023

Copy link
Copy Markdown
Member Author

Agreed, but I don't have the time to get a test out (gave it an attempt, couldn't figure it out in the time I had) - I will merge this for the fix and follow up later

@ckifer
ckifer merged commit cfa141d into recharts:master May 10, 2023
@ckifer
ckifer deleted the fix/treemap-tt branch May 10, 2023 16:55
renovate Bot referenced this pull request in UI5/webcomponents-react May 11, 2023
[![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.5.0` ->
`2.6.2`](https://renovatebot.com/diffs/npm/recharts/2.5.0/2.6.2) |
[![age](https://badges.renovateapi.com/packages/npm/recharts/2.6.2/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/npm/recharts/2.6.2/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/npm/recharts/2.6.2/compatibility-slim/2.5.0)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/npm/recharts/2.6.2/confidence-slim/2.5.0)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>recharts/recharts</summary>

###
[`v2.6.2`](https://togithub.com/recharts/recharts/releases/tag/v2.6.2)

[Compare
Source](https://togithub.com/recharts/recharts/compare/v2.6.1...v2.6.2)

fix bug where ts types error and cannot be generated when exporting in a
consuming package. Fixes
[https://github.com/recharts/recharts/pull/3581](https://togithub.com/recharts/recharts/pull/3581)

#### What's Changed

- fix: sankey types - add explicitly return type for renderTooltip by
[@&#8203;ckifer](https://togithub.com/ckifer) in
[https://github.com/recharts/recharts/pull/3581](https://togithub.com/recharts/recharts/pull/3581)

**Full Changelog**:
recharts/recharts@v2.6.1...v2.6.2

###
[`v2.6.1`](https://togithub.com/recharts/recharts/releases/tag/v2.6.1)

[Compare
Source](https://togithub.com/recharts/recharts/compare/v2.6.0...v2.6.1)

#### What's Changed

Quick fix for a bug crashing treemaps with tooltips in 2.6.0

- Fix treemap tooltip crash by
[@&#8203;ckifer](https://togithub.com/ckifer) in
[https://github.com/recharts/recharts/pull/3579](https://togithub.com/recharts/recharts/pull/3579)

**Full Changelog**:
recharts/recharts@v2.6.0...v2.6.1

###
[`v2.6.0`](https://togithub.com/recharts/recharts/releases/tag/v2.6.0)

[Compare
Source](https://togithub.com/recharts/recharts/compare/v2.5.0...v2.6.0)

#### What's Changed

Storybook documentation updates as well as our first accessibility
feature thanks to
[@&#8203;julianna-langston](https://togithub.com/julianna-langston) and
some refactoring to function components!

##### Feat

- \[a11y] Keyboard navigation support for categorical charts by
[@&#8203;julianna-langston](https://togithub.com/julianna-langston) in
[https://github.com/recharts/recharts/pull/3546](https://togithub.com/recharts/recharts/pull/3546)
- See docs
[here](https://release--63da8268a0da9970db6992aa.chromatic.com/?path=/docs/api-accessibility--docs)

##### Fix

- Update type for `equidistantPreserveStart` axis interval by
[@&#8203;frontier159](https://togithub.com/frontier159) in
[https://github.com/recharts/recharts/pull/3511](https://togithub.com/recharts/recharts/pull/3511)
- Stop focusing on tooltip when displayed - use event listeners instead
by [@&#8203;ArkaFred](https://togithub.com/ArkaFred) in
[https://github.com/recharts/recharts/pull/3515](https://togithub.com/recharts/recharts/pull/3515)

##### Docs

- lots of storybook changes - [go check it
out](https://release--63da8268a0da9970db6992aa.chromatic.com/)!
    -   Getting close to parity with existing docs on recharts.org
    -   Added accessibility add-on
- Added accessibility docs to go with the keyboard navigation support
    -   Storybook 7 stable
    -   Much more!

##### Refactor

- refactor: `Curve` to function component by
[@&#8203;akamfoad](https://togithub.com/akamfoad) in
[https://github.com/recharts/recharts/pull/3477](https://togithub.com/recharts/recharts/pull/3477)
- refactor: `PolarGrid` to function component by
[@&#8203;akamfoad](https://togithub.com/akamfoad) in
[https://github.com/recharts/recharts/pull/3471](https://togithub.com/recharts/recharts/pull/3471)
- refactor: `Cross` to component function by
[@&#8203;akamfoad](https://togithub.com/akamfoad) in
[https://github.com/recharts/recharts/pull/3475](https://togithub.com/recharts/recharts/pull/3475)
- refactor: tooltip to function component by
[@&#8203;ckifer](https://togithub.com/ckifer) in
[https://github.com/recharts/recharts/pull/3336](https://togithub.com/recharts/recharts/pull/3336)
- fix: defaultProps of tooltip, fix viewBox by
[@&#8203;ckifer](https://togithub.com/ckifer) in
[https://github.com/recharts/recharts/pull/3554](https://togithub.com/recharts/recharts/pull/3554)
- refactor: `Text` to function component by
[@&#8203;akamfoad](https://togithub.com/akamfoad) in
[https://github.com/recharts/recharts/pull/3463](https://togithub.com/recharts/recharts/pull/3463)

#### New Contributors

- [@&#8203;akamfoad](https://togithub.com/akamfoad) made their first
contribution in
[https://github.com/recharts/recharts/pull/3466](https://togithub.com/recharts/recharts/pull/3466)
- [@&#8203;nicholasgcoles](https://togithub.com/nicholasgcoles) made
their first contribution in
[https://github.com/recharts/recharts/pull/3467](https://togithub.com/recharts/recharts/pull/3467)
- [@&#8203;linhuiw](https://togithub.com/linhuiw) made their first
contribution in
[https://github.com/recharts/recharts/pull/3508](https://togithub.com/recharts/recharts/pull/3508)
- [@&#8203;frontier159](https://togithub.com/frontier159) made their
first contribution in
[https://github.com/recharts/recharts/pull/3511](https://togithub.com/recharts/recharts/pull/3511)
- [@&#8203;ArkaFred](https://togithub.com/ArkaFred) made their first
contribution in
[https://github.com/recharts/recharts/pull/3515](https://togithub.com/recharts/recharts/pull/3515)
- [@&#8203;julianna-langston](https://togithub.com/julianna-langston)
made their first contribution in
[https://github.com/recharts/recharts/pull/3540](https://togithub.com/recharts/recharts/pull/3540)

**Full Changelog**:
recharts/recharts@v2.5.0...v2.6.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.

---

- [ ] <!-- rebase-check -->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://app.renovatebot.com/dashboard#github/SAP/ui5-webcomponents-react).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS43NC4wIiwidXBkYXRlZEluVmVyIjoiMzUuNzUuMCIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Lukas Harbarth <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants