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

Tooltip fixes (getLabelAndValue on null controller, null getParsed) #11596

Merged
merged 2 commits into from Nov 29, 2023

Conversation

joshkel
Copy link
Contributor

@joshkel joshkel commented Nov 28, 2023

I encountered #11315 under the following circumstances:

  1. Position the cursor over the chart area, such that it causes a tooltip to be shown.
  2. Move the cursor out of the chart area, such that the tooltip remains visible.
  3. Cause the chart contents to be changed, such that the dataset referenced by the active tooltip element is no longer valid.
  4. Move the mouse again. This triggers an inChartArea = false event, so it reuses the previous, now invalid, active elements.

This fixes #11315 under the circumstances for which I've reproduced it, but there may be others.

#11365 appears to be caused by a similar issue (reused active elements that reference now-invalid datasetIndexes of valid datasets), so I've addressed that too.

These issues appear to be caused by #9970 - that added logic to leave active elements alone on chart update, to accommodate cases where the user was programmatically maintaining the active elements themselves.

I encountered chartjs#11315 under the following circumstances:

1. Position the cursor over the chart area, such that it causes a
   tooltip to be shown.
2. Move the cursor out of the chart area, such that the tooltip remains
   visible.
3. Cause the chart contents to be changed, such that the dataset
   referenced by the active tooltip element is no longer valid.
4. Move the mouse again.  This triggers an `inChartArea = false` event,
   so it reuses the previous, now invalid, active elements.

This fixes chartjs#11315 under the circumstances for which I've reproduced it,
but there may be others.
@joshkel joshkel changed the title Fix for getLabelAndValue on null controller Tooltip fixes (getLabelAndValue on null controller, null getParsed) Nov 28, 2023
@mihai-peteu
Copy link

@etimberg May need to increase the max build size of chartjs, size-limit is failing at the moment:

 "name": "dist/chart.js",
    "passed": false,
    "size": 22408,
    "sizeLimit": 22400,

@etimberg
Copy link
Member

yeah, @kurkle @LeeLenaleee what do you think about just removing that compressed size step? It seems to fail more often than not and we just slowly increase the size

@etimberg
Copy link
Member

Will merge and override the CI for now then we can remove the step in a separate PR

@etimberg etimberg merged commit 429d99d into chartjs:master Nov 29, 2023
8 of 9 checks passed
@etimberg etimberg added this to the Version 4.4.1 milestone Nov 29, 2023
@LeeLenaleee
Copy link
Collaborator

LeeLenaleee commented Nov 29, 2023

Also opend an issue for this once, we already thought about removing it before 😅
#11303

@joshkel joshkel deleted the tooltip-getLabelAndValue branch November 29, 2023 20:20
@mihai-peteu
Copy link

Does the Release action need to be manually run?

@etimberg
Copy link
Member

yeah, releases are manual. We'll need to bump the package.json version first and then run the release on github.

@mihai-peteu
Copy link

Nice, I noticed the release-drafter was run. So next up is releasing a new patch version, then vue-chartjs can upgrade its dependency ;)

(cc @apertureless 🙏 )

@DevNowOrNever
Copy link

DevNowOrNever commented Jan 19, 2024

@joshkel hi you!
Current source don't check null value for [controller]

/src/plugins/plugin.tooltip.js

notcheckNULL

So, I get error at line 1188.

plugin.tooltip.js:1188  Uncaught TypeError: Cannot read properties of null (reading 'getParsed')
    at plugin.tooltip.js:1188:51

Please check committed source again.
Commit : 11596

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants