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

Outside tooltip on bar charts scroll offset not calculated correctly #18635

Closed
runfaj opened this issue Mar 7, 2023 · 7 comments · Fixed by #18681
Closed

Outside tooltip on bar charts scroll offset not calculated correctly #18635

runfaj opened this issue Mar 7, 2023 · 7 comments · Fixed by #18681

Comments

@runfaj
Copy link

runfaj commented Mar 7, 2023

Actual behaviour

Here's a fiddle I was able to reproduce with https://jsfiddle.net/runfaj/wx2cabpv/1/. Essentially in certain scroll situations, it seems like inverted charts don't calculate the horizontal scroll offset correctly. In this fiddle, if you size the preview pane to about 700 px, so the tiles on the far right need to be scrolled to, then you hover, you'll see the tooltips no longer line up on the points.
image

If you shrink that area so the scroll increased, the problem is exacerbated to where the tooltips are completely out of view.
image

I added a couple examples the the far right and far bottom and it seems there's no issues with non-inverted charts or vertical positioning.

Upon digging in the code, it appears the defaultPositioner in the tooltip is calculated quite differently than what is provided for the getPosition function that is available for the tooltip.positioner. You'll see if you define a positioner in this same fiddle:

positioner(boxWidth, boxHeight, point) {
  const pos = this.getPosition(boxWidth, boxHeight, point);
  return pos;
},

Then the tooltips don't even calculate against the chart container's offset anymore, so the tooltips go haywire on all the tiles.

Since it doesn't appear that the defaultPositioner is working and it also appears to be a completely private method, I don't see any way to fix this issue.

Expected behaviour

Tooltips should always align to the top of the point for columns or to the right of the point for bars (at least in this example). This would be a fix for the defaultPositioner.

Also, it would be helpful potentially to also update the tooltip.positioner call:

  • so the provided point receives the same inputs that the defaultPositioner has available (like the anchorX/Y).
  • update so getPosition matches the same offsetting behavior as the defaultPositioner.

Product version

highcharts 10.3.3

Affected browser(s)

mostly tested in chrome

@highsoft-bot highsoft-bot added this to To do in Development-Flow via automation Mar 7, 2023
@bm64
Copy link
Member

bm64 commented Mar 9, 2023

Hello @runfaj and thanks for reporting the bug.

@bm64 bm64 added the Type: Bug label Mar 9, 2023
@runfaj
Copy link
Author

runfaj commented Mar 10, 2023

@bm64 I'm not familiar with highcharts development flows - do you know a general timeline for this type of bug so I can report back to my PMs? We're working on transitioning off of amcharts for all our customers and I believe this is the last bug we had to work through after today!

@hubertkozik hubertkozik self-assigned this Mar 13, 2023
@bm64
Copy link
Member

bm64 commented Mar 13, 2023

@runfaj Usually we prioritise bugs / feature requests based on community feedback or based on how important they are for the library to work correctly. In this case we'll try to take a look at it right away. I'd expect it to be fixed within next two weeks and available with next release.

@runfaj
Copy link
Author

runfaj commented Mar 13, 2023

Awesome, you guys rock!

@hubertkozik
Copy link
Member

@runfaj You were right! There was an issue inside the Tooltip.prototype.getPostion function, which didn't take into account the scrollWidth of an container. Below, I have made a workaround for that issue, it is inside the IIFE function. If you want to use it, just copy-paste it into your project and everything should work as intended. A similar solution will be used in HC Core.

Live demo simplified: https://jsfiddle.net/BlackLabel/pkt7xsc6/
Live demo with tiles: https://jsfiddle.net/BlackLabel/k7zLd56y/

@runfaj
Copy link
Author

runfaj commented Mar 15, 2023

@hubertkozik Awesome! I'll get this in for now and keep track of this issue so I know when it has made it to the published library for cleanup. Thanks!!

@runfaj
Copy link
Author

runfaj commented Mar 22, 2023

@TorsteinHonsi @hubertkozik I found one more case after a bunch of testing that might need some additional tweaks for this. I've replicated in a fiddle here: https://jsfiddle.net/runfaj/nmx6oa5w/19/

In this fiddle, I'm using the same workaround presented in #18635 (comment) (the example with tiles), but adding an overlay that replicates a more complex layout:
image

You can see in my screenshot that the tooltip is quite a bit off and in the fiddle, it isn't even visible. This appears to have to do with the top/left that is applied to the body that essentially keeps the user's scroll position, but allows hiding scrollbars. I've replicated that at the bottom of this fiddle. The tooltips work correctly when that top/left is not present, but it is a constraint that I need to keep in our application.

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

Successfully merging a pull request may close this issue.

4 participants