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

Highstock - flags overlap with "circlepin" and "squarepin" shape type #9726

Closed
ppotaczek opened this issue Dec 19, 2018 · 12 comments
Closed

Comments

@ppotaczek
Copy link
Contributor

Expected behaviour

The flags should behave in similar way like with "flag" shape type:
http://jsfiddle.net/BlackLabel/hwym791L/

Actual behaviour

The flags overlap.

Live demo with steps to reproduce

Open jsfiddle: http://jsfiddle.net/BlackLabel/2dz4tuja/

Product version

Highstock 7.0.1

Affected browser(s)

All

@simdevmon
Copy link

Another example (overflows x-axis) https://jsfiddle.net/dgqxz172/

@TorsteinHonsi
Copy link
Collaborator

The second case there is probably a mistake in how H.distribute is used - it should align within the box. I'm busy today, but will have a look at it tomorrow.

@jegli
Copy link

jegli commented Mar 7, 2019

Thanks @sebastianbochan for referencing the issue. And my apologies for not searching more thoroughly before opening a new issue.

Just wanted to post my example here as well: https://jsfiddle.net/cop0t871/5/
Thank you guys for your work. 👍

@artola
Copy link

artola commented Apr 23, 2019

@TorsteinHonsi Do you have some update on this issue? I am interested in the overflows on x-axis. Could you give me some hint where to look in the codebase to try it?

@artola
Copy link

artola commented Apr 23, 2019

@TorsteinHonsi I think that the code causing the overflow might be around this line:

value -= alignFactor * ((width || bBox.width) + 2 * padding);

It seems that when alignFactor is 0.5 (for center) it might produce negative values when value is close to 0. Some kind of condition (e.g., Math.max) could avoid it.

See: https://jsfiddle.net/3dco5m2y/2/

Fix:

value = Math.max(0, value - alignFactor * ((width || bBox.width) + 2 * padding));

@TorsteinHonsi
Copy link
Collaborator

Thanks for your patience! The issue was with the way the results of the Highcharts.distribute function were handled.

Reported cases with the fix applied:

@artola
Copy link

artola commented Apr 27, 2019

@TorsteinHonsi This fix is probably "most of the time" ok, but requires allowOverlapX: false, because with allowOverlapX: true the label is still cropped, fiddle, because the distribute function is only triggered in the first case.

@TorsteinHonsi
Copy link
Collaborator

Yes I see the problem. As you say, this is a different case since we don't run the distribute function. We currently have logic to only render the flag if the anchor point is inside the plot area, but we don't have logic to crop it out if the content of the flag spills out of the plot area.

@artola
Copy link

artola commented Apr 30, 2019

Yes I see the problem. As you say, this is a different case since we don't run the distribute function. We currently have logic to only render the flag if the anchor point is inside the plot area, but we don't have logic to crop it out if the content of the flag spills out of the plot area.

Might be that you find a better solution, else please consider this easy fix:

#9726 (comment)

@TorsteinHonsi
Copy link
Collaborator

else please consider this easy fix

I'm afraid that solution goes too deep, it would affect all text labels in all situations (including axis labels, data labels, titles, subtitles, tooltip etc..), not only flags. We need a solution that simply hides each flag if a part of it is overflowing.

@TorsteinHonsi
Copy link
Collaborator

Another thing is that flags actually don't have any collision detection or cropping capabilites at all if we turn on allowOverlapX. Non only do the flags overflow on the sides, but they overlap each other and the top. https://jsfiddle.net/highcharts/0meyL5ra/1/

It is this problem we are addressing with setting allowOverlapX to false. Is there a specific reason you don't use the default?

@artola
Copy link

artola commented May 2, 2019

@TorsteinHonsi In my use case, I will go for allowOverlapX: false. I think that in general it is the wanted option, might make no sense the extra effort for the cropping of just 1 label in the specific case of allowOverlapX: true.

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

6 participants