Better tooltips for charts; ability to change formats for numbers and dates (WIP) #2468
Conversation
| @@ -2,6 +2,8 @@ import moment from 'moment/moment'; | |||
| import numeral from 'numeral'; | |||
| import _ from 'underscore'; | |||
|
|
|||
| numeral.options.scalePercentBy100 = false; | |||
There was a problem hiding this comment.
By default, for formats like 0[.]00% numeral will multiply value by 100 - so 0.5 will become 50%. This option disables this behavior so 0.5 will become 0.5%, and 50 - 50%.
| @@ -34,6 +40,10 @@ const PlotlyChart = () => ({ | |||
| let layout = {}; | |||
| let data = []; | |||
|
|
|||
| const tooltip = $('<div>') | |||
| .addClass('plotly-chart-tooltip') | |||
| .appendTo('body'); | |||
There was a problem hiding this comment.
So we render our own tooltip... I can see how this can become a maintenance nightmare, chasing various Plotly edge cases 🤔
What do we lose if we go with Plotly's builtin tooltip?
There was a problem hiding this comment.
I tried a lot of settings, and noticed only two edge cases: bar charts with stacking enabled, and pie charts. for both we just need to compute position for tooltip in a different way (for pie we already receive position in event data).
Own tooltip allows to show all data in a single place (like in Highcharts) - x-axis value, all y values, etc. With default tooltip, we can customize text for y-values but not for x, and (if you remember) Plotly shows tooltip for each series for the same x value , and one more tooltip for x value (which also has a different style).
If you're afraid that it may be a nightmare - of course, I can simplify it, but we will lose some formatting and styling.
There was a problem hiding this comment.
Past experience shows that there are always unexpected scenarios with charts :) Once we merge and deploy this, we're committed to it. But I don't think we need this distraction right now (having to maintain and fix issues related to the tooltip) -- we have more interesting stuff to do.
So let's shelf this for now, and implement something simpler:
- First step: expose the ability to set
hoverformatfor the X and Y axes. - Second step: feature to set the
hovertextproperty using data from the current point.
While it's not as complete as what you implemented here, it will serve most of the cases.
Issue #728
Don't merge it!
This PR introduces a lot of changes, I need to be sure that everything works correctly.
Some screenshots: