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

Series with too long names damage both legend and tooltip #6659

Closed
MiroLiska opened this Issue Apr 28, 2017 · 11 comments

Comments

Projects
None yet
4 participants
@MiroLiska

MiroLiska commented Apr 28, 2017

Expected behaviour

The series with names longer than the current chart container can display should not cause legend and tooltip damage.

Actual behaviour

If the chart is made small and there is no space anymore to show the full name of the series, both legend and tooltip will be illegibly cropped,

Live demo with steps to reproduce

http://jsfiddle.net/z1tm4u3w/ shows the problem
http://jsfiddle.net/z1tm4u3w/1/ shows the possible solution

Affected browser(s)

All

@KacperMadej

This comment has been minimized.

Show comment
Hide comment
@KacperMadej

KacperMadej May 2, 2017

Contributor

Hi,

Thank you for reporting about the issue and for providing a possible solution.

Contributor

KacperMadej commented May 2, 2017

Hi,

Thank you for reporting about the issue and for providing a possible solution.

@TorsteinHonsi

This comment has been minimized.

Show comment
Hide comment
@TorsteinHonsi

TorsteinHonsi May 5, 2017

Collaborator

The commit above fixes the Legend part of the issue. Now I'll have a look at the tooltip.

Collaborator

TorsteinHonsi commented May 5, 2017

The commit above fixes the Legend part of the issue. Now I'll have a look at the tooltip.

@MiroLiska

This comment has been minimized.

Show comment
Hide comment
@MiroLiska

MiroLiska May 25, 2017

Hi

The legend solution seems to be fine, but the tooltips are not satisfying yet. I have two problems with your tooltip solution (referring to release 5.0.12):

Please have a look at desired look and feel at http://jsfiddle.net/z1tm4u3w/4/ (based on 5.0.10 + my solution presented above):
io4

And now your solution from 5.0.12 - the tooltip is too narrow and wrapped - http://jsfiddle.net/z1tm4u3w/5/ :
io5

Please note that the text wrapping is deadly - the wrapped numbers are very misleading.

MiroLiska commented May 25, 2017

Hi

The legend solution seems to be fine, but the tooltips are not satisfying yet. I have two problems with your tooltip solution (referring to release 5.0.12):

Please have a look at desired look and feel at http://jsfiddle.net/z1tm4u3w/4/ (based on 5.0.10 + my solution presented above):
io4

And now your solution from 5.0.12 - the tooltip is too narrow and wrapped - http://jsfiddle.net/z1tm4u3w/5/ :
io5

Please note that the text wrapping is deadly - the wrapped numbers are very misleading.

@TorsteinHonsi

This comment has been minimized.

Show comment
Hide comment
@TorsteinHonsi

TorsteinHonsi May 26, 2017

Collaborator

it does not consider the 'tooltip-outside-box',

You can set the width to "auto": http://jsfiddle.net/z1tm4u3w/7/

I do not know how to turn off the text wrapping

You can set textOverflow to "ellipsis": http://jsfiddle.net/z1tm4u3w/6/

Collaborator

TorsteinHonsi commented May 26, 2017

it does not consider the 'tooltip-outside-box',

You can set the width to "auto": http://jsfiddle.net/z1tm4u3w/7/

I do not know how to turn off the text wrapping

You can set textOverflow to "ellipsis": http://jsfiddle.net/z1tm4u3w/6/

@MiroLiska

This comment has been minimized.

Show comment
Hide comment
@MiroLiska

MiroLiska May 26, 2017

Your width:auto is internally still using my solution with ellipsis iteration :) . When you use this style on /5/ (no iteration) , and make the text quite longer, you will see that the tooltip width has no limits now, which is probably not what you want. : http://jsfiddle.net/z1tm4u3w/8/show/ . Something like 'max-width' is necessary.

The solution with textOverflow set to ellipsis is not good, because it cuts off the data value, which is the only reason why I want to have the tooltip.

MiroLiska commented May 26, 2017

Your width:auto is internally still using my solution with ellipsis iteration :) . When you use this style on /5/ (no iteration) , and make the text quite longer, you will see that the tooltip width has no limits now, which is probably not what you want. : http://jsfiddle.net/z1tm4u3w/8/show/ . Something like 'max-width' is necessary.

The solution with textOverflow set to ellipsis is not good, because it cuts off the data value, which is the only reason why I want to have the tooltip.

@TorsteinHonsi

This comment has been minimized.

Show comment
Hide comment
@TorsteinHonsi

TorsteinHonsi May 26, 2017

Collaborator

The auto width comes from this line in Tooltip.refresh. So for this to work with the outside-box study we could override the spacingBox similar to what we do wth chartWidth and chartHeight in the plugin.

The solution with textOverflow set to ellipsis is not good, because it cuts off the data value, which is the only reason why I want to have the tooltip.

That's why we went for word wrapping by default. It would be best to ellipsis the series name itself, but then we become dependent on what the tooltip formatters produce...

Collaborator

TorsteinHonsi commented May 26, 2017

The auto width comes from this line in Tooltip.refresh. So for this to work with the outside-box study we could override the spacingBox similar to what we do wth chartWidth and chartHeight in the plugin.

The solution with textOverflow set to ellipsis is not good, because it cuts off the data value, which is the only reason why I want to have the tooltip.

That's why we went for word wrapping by default. It would be best to ellipsis the series name itself, but then we become dependent on what the tooltip formatters produce...

@MiroLiska

This comment has been minimized.

Show comment
Hide comment
@MiroLiska

MiroLiska May 26, 2017

By using standard format or standard-like formatters, my solution works OK : see e.g. http://jsfiddle.net/z1tm4u3w/9/
pf1

For non-standard formatter generating HTML tables or similar we can simply turn it off, but maybe it would work anyway, never tested it.

In my opinion, it does not matter if you wrap or cut the data values. It is confusing. We do not have 2... people in our village, but neither 215
0

MiroLiska commented May 26, 2017

By using standard format or standard-like formatters, my solution works OK : see e.g. http://jsfiddle.net/z1tm4u3w/9/
pf1

For non-standard formatter generating HTML tables or similar we can simply turn it off, but maybe it would work anyway, never tested it.

In my opinion, it does not matter if you wrap or cut the data values. It is confusing. We do not have 2... people in our village, but neither 215
0

@TorsteinHonsi

This comment has been minimized.

Show comment
Hide comment
@TorsteinHonsi

TorsteinHonsi May 26, 2017

Collaborator

One generic solution I can imagine would be to be able mark which part of the formatted string should be allowed to truncate.

pointFormat: '<span style="color:{point.color}">\u25CF</span> <span class="highcharts-truncatable">{series.name}:</span> <b>{point.y}</b><br/>'

Then the buildText function would pick this up.

Collaborator

TorsteinHonsi commented May 26, 2017

One generic solution I can imagine would be to be able mark which part of the formatted string should be allowed to truncate.

pointFormat: '<span style="color:{point.color}">\u25CF</span> <span class="highcharts-truncatable">{series.name}:</span> <b>{point.y}</b><br/>'

Then the buildText function would pick this up.

@MiroLiska

This comment has been minimized.

Show comment
Hide comment
@MiroLiska

MiroLiska May 26, 2017

OK, why not. Please note that there must be a good logic for deciding how wide can be such a tooltip (not the truncatable span!).
min(window.width, 500) or something like that, in order to support both mobile and desktop screens.

MiroLiska commented May 26, 2017

OK, why not. Please note that there must be a good logic for deciding how wide can be such a tooltip (not the truncatable span!).
min(window.width, 500) or something like that, in order to support both mobile and desktop screens.

@sgjoy1

This comment has been minimized.

Show comment
Hide comment
@sgjoy1

sgjoy1 Jul 21, 2017

I'm running into a similar issue. I'm trying to build a sparkline style chart, but the width of the chart forces the tooltip to constrict the container size and the text overflows.
Here's a fiddle detailing the issue.

The solution would suggested above (adding the style of width: 'auto') does solve the issue, but it seems a little odd that the tooltip's container is sized based off the spacingBox of the chart.

Here's a fiddle with the fix

sgjoy1 commented Jul 21, 2017

I'm running into a similar issue. I'm trying to build a sparkline style chart, but the width of the chart forces the tooltip to constrict the container size and the text overflows.
Here's a fiddle detailing the issue.

The solution would suggested above (adding the style of width: 'auto') does solve the issue, but it seems a little odd that the tooltip's container is sized based off the spacingBox of the chart.

Here's a fiddle with the fix

@KacperMadej

This comment has been minimized.

Show comment
Hide comment
@KacperMadej

KacperMadej Jul 23, 2017

Contributor

@sgjoy1 By default tooltip should fit inside of a chart and overflow is hidden, so tooltip size is based on the chart size. The fix in your second demo seems to be working fine.

Contributor

KacperMadej commented Jul 23, 2017

@sgjoy1 By default tooltip should fit inside of a chart and overflow is hidden, so tooltip size is based on the chart size. The fix in your second demo seems to be working fine.

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