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

Update tooltip custom label example #5454

Merged
merged 5 commits into from Jul 14, 2018
Merged

Conversation

jung35
Copy link
Contributor

@jung35 jung35 commented Apr 27, 2018

The canvas.getBoundingClientRect()

returns the size of an element and its position relative to the viewport

So the tooltip should also reflect that as fixed positioning and not absolute

@simonbrunel
Copy link
Member

@jung35 can you build a jsfiddle with this change?

@jung35
Copy link
Contributor Author

jung35 commented May 7, 2018

I also added pointerevent none because you can't hover over data points with the tooltip blocking from hovering over data point

here is example with
absolute https://jsfiddle.net/r3Lz7371/
vs
fixed https://jsfiddle.net/r3Lz7371/1/

tooltipEl.style.left = position.left + tooltipModel.caretX + 'px';
tooltipEl.style.top = position.top + tooltipModel.caretY + 'px';
tooltipEl.style.fontFamily = tooltipModel._bodyFontFamily;
tooltipEl.style.fontSize = tooltipModel.bodyFontSize + 'px';
tooltipEl.style.fontStyle = tooltipModel._bodyFontStyle;
tooltipEl.style.padding = tooltipModel.yPadding + 'px ' + tooltipModel.xPadding + 'px';
tooltipEl.style.pointerEvents = 'none';
Copy link
Contributor

Choose a reason for hiding this comment

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

you used tabs here, but it's spaces everywhere else in the file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops, kinda made the change on github editor. defaulted to tabs

@simonbrunel simonbrunel added this to the Version 2.8 milestone Jun 24, 2018
@simonbrunel
Copy link
Member

With position: fixed, if we scroll while the tooltip is displayed, the tooltip doesn't follow the chart but stays under the mouse (which is also a valid use case). With position: absolute, we need to take in account the x/y scroll position:

var pos = this._chart.canvas.getBoundingClientRect();
var x = pos.left + window.pageXOffset;
var y = pos.top + window.pageYOffset;

// ...

el.style.left = x + tooltipModel.caretX + 'px';
el.style.top = y + tooltipModel.caretY + 'px';

// ...

https://jsfiddle.net/r3Lz7371/16/

I think I prefer the absolute behavior because it works better if the tooltip always stay visible, but since it's an example and thus a matter of taste, I don't really mind.

@benmccann @etimberg?

@benmccann
Copy link
Contributor

absolute is better in my mind

@etimberg
Copy link
Member

I agree @simonbrunel @benmccann. Absolute is better to me

@benmccann
Copy link
Contributor

@jung35 can you update to use absolute?

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

Successfully merging this pull request may close these issues.

None yet

4 participants