Skip to content

Conversation

@etimberg
Copy link
Member

@etimberg etimberg commented Oct 10, 2016

Created the concept of tooltip modes. User defined modes are supported by changing Chart.Tooltip.modes map.

The default mode is 'average'. We may consider changing this if we think the old behaviour is really a bug that no one wanted. Currently. this will not change existing charts.

tooltips: {
  positionMode: 'average'
}

Also implemented is the 'nearest' mode which places the tooltip on the element closest to the event that triggered the tooltip. Implementing this required removing some of the optimizations from ChartController#eventHandler. This may not be acceptable so there may need to be some refactoring.

This addresses #2056

CC @chartjs/maintainers

custom | Function | null | See [section](#advanced-usage-external-tooltips) below
mode | String | 'nearest' | Sets which elements appear in the tooltip. See [Interaction Modes](#interaction-modes) for details
intersect | Boolean | true | if true, the tooltip mode applies only when the mouse position intersects with an element. If false, the mode will be applied at all times.
positionMode | String | 'average' | The mode for positioning the tooltip. 'average' mode will place the tooltip at the average position of the items displayed in the tooltip. 'nearest' will place the tooltip at the position of the element closest to the event position
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about simply use position as option name?

Copy link
Member Author

Choose a reason for hiding this comment

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

That sound better

/**
* @namespace Chart.Tooltip.modes
*/
Chart.Tooltip.modes = {
Copy link
Member

@simonbrunel simonbrunel Oct 10, 2016

Choose a reason for hiding this comment

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

I would choose another name because it can be confusing with the tooltip.mode option (not sure what name to pick then, maybe Chart.Tooltip.positioners).

// Hover animations
if (!me.animating) {
// If entering, leaving, or changing elements, animate the change via pivot
if (!helpers.arrayEquals(me.active, me.lastActive) ||
Copy link
Member

Choose a reason for hiding this comment

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

Does that mean the chart is redrawn at every mouse move? If so, that's not good :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately, yes. What do you think about detecting if there are changes in update and returning a Boolean. If there are changes, we redraw.

Alternatively, we factor the position code out to here and pass it to the tooltip before update.

@tannerlinsley
Copy link
Contributor

👍
On Mon, Oct 10, 2016 at 6:16 AM Evert Timberg notifications@github.com
wrote:

@etimberg commented on this pull request.

In docs/01-Chart-Configuration.md
#3453:

@@ -214,6 +214,7 @@ enabled | Boolean | true | Are tooltips enabled
custom | Function | null | See [section](#advanced-usage-

external-tooltips) below
mode | String | 'nearest' | Sets which elements appear in the tooltip. See Interaction Modes for details
intersect | Boolean | true | if true, the tooltip mode applies only when the mouse position intersects with an element. If false, the mode will be applied at all times.
+positionMode | String | 'average' | The mode for positioning the tooltip. 'average' mode will place the tooltip at the average position of the items displayed in the tooltip. 'nearest' will place the tooltip at the position of the element closest to the event position

That sound better


You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub
#3453, or mute the thread
https://github.com/notifications/unsubscribe-auth/AFUmCXYv7gre4qODUZda1VDuW4ELaXTNks5qyiyTgaJpZM4KSLF_
.

@etimberg
Copy link
Member Author

I changed Chart.Tooltip.modes to Chart.Tooltip.positioners and made the config name position.

I still need to figure out how to better do the updates so that we do not continuously redraw.

@etimberg
Copy link
Member Author

@simonbrunel I followed your advice and split up the eventHandler method into separate pieces.

We now do

var changed = me.handleEvent(e);
changed |= me.legend.handleEvent(e);
changed |= me.tooltip.handleEvent(e);

then only re-render if we changed. The tooltip.handleEvent method will return true if the position changes which handles the case I was running into.

@etimberg
Copy link
Member Author

@simonbrunel have you had a chance to review the latest changes I made?

@etimberg etimberg merged commit a86c47c into master Oct 14, 2016
@simonbrunel simonbrunel deleted the tooltip-position-modes branch October 15, 2016 20:48
@simonbrunel simonbrunel added this to the Version 2.4 milestone Oct 15, 2016
exwm pushed a commit to exwm/Chart.js that referenced this pull request Apr 30, 2021
Adds new tooltip position option that allows configuring where a tooltip is displayed on the graph in relation to the elements that appear in it
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.

4 participants