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

Added interaction mode to 'nearest' to fixed cluttered tooltips #1402

Merged

Conversation

gayanSandamal
Copy link
Contributor

@gayanSandamal gayanSandamal commented Oct 15, 2024

Description

Before

Screen.Recording.2024-10-15.at.12.20.18.mov

After

Screen.Recording.2024-10-16.at.14.23.05.mov

This doesn't require any E2E tests or document updates

Related Issue(s)

#1396

Checklist

  • I have read the contribution guidelines
  • Suitable unit/system level tests have been added and they pass
  • Documentation has been updated
    • Upgrade instructions
    • Configuration details
    • Concepts
  • Changes flowforge.yml?
    • Issue/PR raised on FlowFuse/helm to update ConfigMap Template
    • Issue/PR raised on FlowFuse/CloudProject to update values for Staging/Production

Labels

  • Includes a DB migration? -> add the area:migration label

@gayanSandamal gayanSandamal linked an issue Oct 15, 2024 that may be closed by this pull request
@gayanSandamal gayanSandamal self-assigned this Oct 15, 2024
@joepavitt
Copy link
Collaborator

So, with the after here, I'd still expect to see, in the tooltip, a value for each series, that matches the x value. This change means that we only see the single line?

@gayanSandamal
Copy link
Contributor Author

So, with the after here, I'd still expect to see, in the tooltip, a value for each series, that matches the x value. This change means that we only see the single line?

@joepavitt Thanks for the clarification! That makes sense now. I noticed that the existing implementation is correct and the issue is with the data points; the data point values are keep stacking when taking the cursor through the chart line

Screen.Recording.2024-10-15.at.16.18.31.mov

@joepavitt
Copy link
Collaborator

the data point values are keep stacking when taking the cursor through the chart line

Ooh, good find, so an underlying ChartJS bug possibly? Can you check through their existing bug list to see if it's something that has been reported.

@gayanSandamal
Copy link
Contributor Author

the data point values are keep stacking when taking the cursor through the chart line

Ooh, good find, so an underlying ChartJS bug possibly? Can you check through their existing bug list to see if it's something that has been reported.

Thanks for the suggestion! I'm checking the ChartJS bug list now. I'll update you soon

@joepavitt
Copy link
Collaborator

I'm checking the ChartJS bug list now.

Any update please @gayanSandamal?

@gayanSandamal
Copy link
Contributor Author

I'm checking the ChartJS bug list now.

Any update please @gayanSandamal?

@joepavitt I found the exact same issue reported chartjs/Chart.js#8293

After carefully going through the solutions provided they have are recommending to use 'nearest' (which was my initial solution 2608ef1) option instead x and all the examples they have included are single line charts.

But for our scenario we cannot use that when the data points are overlapping as the tooltip gets the values of from the intersection.

I've tried the below options

  • Setting intersect to false
  • Set pointHitRadius to 0
  • Set pointRadius to 0
  • Trying different "Custom Interaction Modes"

The only thing we can do here is reduce the pointerRadius and the it doesn't fix this problem entirely, supports up to a certain level of data points and it's not easy to get the tooltip because user needs to carefully place the cursor right on the data points get the tooltip.

Screenshot 2024-10-16 at 01 51 42

@joepavitt
Copy link
Collaborator

How about using a filter per: chartjs/Chart.js#5231

@gayanSandamal
Copy link
Contributor Author

How about using a filter per: chartjs/Chart.js#5231

Thanks a lot. I'm checking this

@gayanSandamal
Copy link
Contributor Author

How about using a filter per: chartjs/Chart.js#5231

@joepavitt PR is updated with this commit 21076d1

PR description is also updated #1402 (comment)

@bartbutenaers
Copy link
Contributor

How about using a filter per: chartjs/Chart.js#5231

Hi @joepavitt,
Without the intension of becoming a party-stopper...
That all seems to be very ChartJs specific stuff.
Doesn't this kind of code will make it very hard afterwards to switch to another charting library?
Unless the replacement of ChartJs by another library is not an established fact anymore, then please forget my comment!

@joepavitt
Copy link
Collaborator

Doesn't this kind of code will make it very hard afterwards to switch to another charting library?

Yes, but it's part of the ChartJS config, if/when we switch to Apache, we would need to make an equivalent. I can't justify getting Gayan to spend 4-6 weeks on migrating to Apache eCharts at the moment.

Unless the replacement of ChartJs by another library is not an established fact anymore, then please forget my comment!

It's something we hope to do, but it's a monstrous piece of work, and we're regularly distracted by the many issues and smaller feature requests being open

@joepavitt joepavitt merged commit 782c463 into main Oct 21, 2024
2 checks passed
@joepavitt joepavitt deleted the 1396-v1180-ui-chart-group-tooltips-in-highy-populated-charts branch October 21, 2024 09:37
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.

v1.18.0: ui-chart group tooltips in highy populated charts
3 participants