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

larger point/hover radius does not offset tooltip, obscures caret #3599

Closed
jeffzap76 opened this issue Nov 14, 2016 · 3 comments
Closed

larger point/hover radius does not offset tooltip, obscures caret #3599

jeffzap76 opened this issue Nov 14, 2016 · 3 comments

Comments

@jeffzap76
Copy link

jeffzap76 commented Nov 14, 2016

Using a pointRadius and pointHoverRadius larger than the default results in a tooltip whose caret is buried beneath the point:
image

I've updated to 2.4.0 in the hopes that some of the recent tooltip-related fixes might have addressed this, but it doesn't appear so.

I reverted to defaults on point and hover radius and border thickness and realized that the caret is actually always somewhat obscured by the point; the tooltip is anchored off the x/y of the true data point and never takes the point size or shape into account, really.

After studying the code, I was able to hack a workaround by overriding the 'average' positioner that is used for determining the tooltip anchor coordinate to include a 'padding' property on its return object that I see would be used to set a local 'caretPadding' property that is already factored into the tooltip position math. I might suggest externalizing this caretPadding concept as it seems like it's very close to the intended spirit of my request, so obviously someone was thinking along these lines. :-)

  var origPositioner = Chart.Tooltip.positioners.average;
  Chart.Tooltip.positioners.average = function(elements, eventPosition) {
    console.log('using override tooltip avg positioner, adding padding');
    var pos = origPositioner(elements, eventPosition);
    pos.padding = 12;
    return pos;
  };

image

@etimberg
Copy link
Member

@jeffzap76 it feels like https://github.com/chartjs/Chart.js/blob/master/src/core/core.tooltip.js#L517 is more of a typo than anything. I don't think any of the positioners were supposed to return padding ... it should come from the options.

@jeffzap76
Copy link
Author

I agree - it would seem reasonable for tooltip options to contain that caretPadding property.

@jeffzap76
Copy link
Author

Actually my initial report and screenshots are misleading -- I forgot that I was pulling some visual effects off via a plugin with an afterDraw() implementation that was causing the tooltip caret to be drawn over. So that apparent defect is not really a defect.

What remains valid here is to discuss whether caretPadding should be externalized to the options and used to support tooltip positioning offset in case someone doesn't want the caret to point at the dead center of an enlarged data point.

@etimberg etimberg added this to the Version 2.6 milestone Feb 12, 2017
etimberg added a commit that referenced this issue Mar 28, 2017
… value was essentially hard coded to

 2 because of a typo that read it from the positioner output. Based on #3599 we agreed to make this into
a config setting.
etimberg added a commit that referenced this issue Mar 28, 2017
… value was essentially hard coded to

 2 because of a typo that read it from the positioner output. Based on #3599 we agreed to make this into
a config setting.
ericnkatz pushed a commit to ericnkatz/Chart.js that referenced this issue May 24, 2017
… value was essentially hard coded to

 2 because of a typo that read it from the positioner output. Based on chartjs#3599 we agreed to make this into
a config setting.
roicos pushed a commit to roicos/Chart.js that referenced this issue Aug 21, 2017
… value was essentially hard coded to

 2 because of a typo that read it from the positioner output. Based on chartjs#3599 we agreed to make this into
a config setting.
exwm pushed a commit to exwm/Chart.js that referenced this issue Apr 30, 2021
… value was essentially hard coded to

 2 because of a typo that read it from the positioner output. Based on chartjs#3599 we agreed to make this into
a config setting.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants