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

[Bug] Horizontal Legend Wrapping doesn't account properly for usage of <br> tags #3175

Closed
Braintelligence opened this issue Oct 29, 2018 · 18 comments · Fixed by #3446
Closed
Assignees
Labels
bug something broken

Comments

@Braintelligence
Copy link

Braintelligence commented Oct 29, 2018

Reproduction:

  1. Set screen size so narrow that all legend items will be displayed in a vertical fashion, with legend.orientation = "h".
  2. Create two traces where the first one has a very long name that will make the second trace's name wrap into the next line and do not use <br> in the first traces name.
  3. Put at least one <br> in the second trace's name.

Here's an example: https://codepen.io/anon/pen/EdJyQm?editors=0010

If you put a <br> anywhere in the name of trace6, the name of trace7 will be shown fine.
If you leave it out and your last trace contains a <br> the second line will be cut off.

Please ping me up if you need further explanation of the problem.

@Braintelligence
Copy link
Author

Braintelligence commented Oct 30, 2018

Actually it seems that the last legend item with <br> will be cut off if there is any other legend item without <br>. This is a significant bug that I don't know how to take care off, other than conditionally switching to legend.orientation = "v" if a certain clientWidth is underrun.

EDIT:
Actually it seems that if the first element of all legend items contains the same amount of <br> as the last element, there is no problem.
Could it be that the "margin" for the legend is calculated solely by the first element?

@Braintelligence
Copy link
Author

Braintelligence commented Oct 30, 2018

@alexcjohnson Do I understand correctly that maxTraceHeight is set to 0 here

maxTraceHeight = 0,
and then used for calculations here
rowHeight = rowHeight + maxTraceHeight;
(even though it never could amount to anything but 0 and only after that it is being set to anything at all
maxTraceHeight = Math.max(legendItem.height, maxTraceHeight);
Or do I miss something here?

EDIT: Ah, it's a forEach. But then how is maxTraceHeight relevant for the last legend item? It is calculated but never used then. Am I right?

@Braintelligence
Copy link
Author

Braintelligence commented Oct 30, 2018

So I verified the behaviour and can reproduce it 100%:

Either the first trace/a trace in the first row has to contain the same amount of <br> as the last trace/all traces in the last row: https://codepen.io/anon/pen/pxmwZz?editors=0010

Or any trace in between (rows) has to contain at least the same amount of <br> as the last trace/all traces in the last row + 1 more <br>: https://codepen.io/anon/pen/MPdoXY?editors=0010

Something must be different for the first legend item/first row calculation that doesn't apply to all other legend items. What could that be?

EDIT: Man, this is the only wall I've hit yet that I couldn't program a somewhat efficient workaround for. 🙈 The only things that I can think of would be heavily resource-hungry...

@Braintelligence Braintelligence changed the title [Bug] Horizontal Legend gets cut off at bottom for narrow screen sizes and specific <br> tags [Bug] Horizontal Legend Wrapping doesn't account properly for usage of <br> tags Oct 30, 2018
@Braintelligence
Copy link
Author

Hey, everyone. Just wanted to know if anyone of the contributors/members has read this, since it isn't labeled and there's no comment yet =(...

@etpinard
Copy link
Contributor

#3024 might be enough to fix this.

@Braintelligence

This comment has been minimized.

@etpinard
Copy link
Contributor

Sorry for not taking a more serious look at this report before. Turns out this thing is a different beast than what #3024 attempts to solve.

@Braintelligence in

can you confirm that switching to legend.orientation: 'v' gives the correct result?

@Braintelligence
Copy link
Author

Braintelligence commented Jan 23, 2019

Ummm, guys?
grafik
😭

EDIT: Trying to find out why this still happens for me now... using current 1.44 release (also happened on the version from the codepen, so I hoped release would fix it).

EDIT2: Wow... I can't reproduce this in Codepen for the life of me. The plotly version is definitely correct and it's not a caching issue. (CTRL+F5 and called locally anyway without any webserver involved)
And if I change the order of traces this happens:
grafik

@Braintelligence
Copy link
Author

Braintelligence commented Jan 23, 2019

Ok now this is super weird.
Totally same file looks in Firefox like this:
grafik
And in Chrome like this:
grafik
Also no problem on MSEdge...

While I can't reproduce it at all in a CodePen. Neither with Firefox nor with Chrome.

Headache is expanding 😭.

EDIT: Ok, ok, hold on. Same Firefox in a privacy tab shows no problems. What's going on o_o...
EDIT2: Strange. I can't get it to work in Firefox private tab anymore, as well. Tried an old Firefox 54 portable and it also had problems. I still can't reproduce it in CodePen on either browser and it works in Chrome, Edge and Internet Explorer as expected. Damnit.

@Braintelligence
Copy link
Author

Braintelligence commented Jan 23, 2019

Ok I can narrow it down some more: It only happens in Firefox in mobile view!
grafik
You see mobile view on the left for the big tab in the background and Firefox turned small by dragging the mouse at the window edges on the right in the foreground.

@alexcjohnson @etpinard Can you make any sense of this special behaviour for the mobile developer view? I can't wrap my head around it.

EDIT: Mobile view in Chrome works, though!

Braintelligence referenced this issue Jan 23, 2019
Fix wrapped horizontal legends height computations
@etpinard
Copy link
Contributor

etpinard commented Jan 23, 2019

Hmm https://codepen.io/anon/pen/MPdoXY?editors=0010 looks ok to me on mobile FF64 on a Pixel2 android phone

image

Could you share a bit more info about the device you're using and which mobile FF version you're on?

Thank you.

@Braintelligence
Copy link
Author

I already found the code line responsible for the problem: 0251a0c#r32036135

@etpinard
Copy link
Contributor

Sure, but I can't reproduce it.

@Braintelligence
Copy link
Author

Braintelligence commented Jan 23, 2019

Ok, so the formula is:
fullLayout._size.w > borderwidth + fullTracesWidth - traceGap

Look at this picture. Left side is Firefox mobile view at 360x640 and right is Chrome mobile view at 360x640:
grafik

Firefox: 242 > 0 + 246.23 - 5 -> true (because it amounts to about 241,23)
Chrome: 242 > 0 + 247.61 - 5 -> false (because it amounts to about 242,61)

Due to the tiny scrollbar that appears on firefox mobile developer view the fullTracesWidth seems to be calculated differently, I guess. There's no scrollbar in Chrome mobile developer view.

No matter what. fullTracesWidth seems to be calculated wrong for Firefox mobile developer view.

EDIT: If I add some pixels to the width in Firefox the legend really fits in one line, so I've hit an edge case here. Nonetheless the calculations is off for Firefox mobile developer view.
The onerowlegend detection fires even though the legend doesn't draw both legend items next to each other.

@etpinard
Copy link
Contributor

Nonetheless the calculations is off for Firefox mobile developer view.

Is it also off on mobile FF?

@Braintelligence
Copy link
Author

I would presume not, because you don't have a scrollbar there. Sadly I can't test that right now. I can test it tomorrow, though.

I remember having had such issues with scrollbars in Firefox mobile developer view for some of my own business logic, that's why I checked this in the first place and seem to have hit the right spot.

@Braintelligence
Copy link
Author

I can't reproduce it easily on my smartphone, because the edge case happens for displays with 360px width. (Don't have a smartphone on me with that width but that's still the most important mobile display width to support if I remember the statistics correctly, so that's what I test for when using the mobile developer view in Firefox for PC.)

The problem stands, though: I hit an edge-case where a difference in 1~2px is not calculated correctly in Firefox mobile developer view only. It can be broken down to the fact that the legend items are not drawn next to each other but the calculation if everything fits in one row says it does fit in one row; this shouldn't be happening if the logic behind knowing if the legend can be rendered in one row and if the legend fits in one row are the same and is executed under the same conditions. But this is exactly what's happening :(...

A quick and dirty fix that comes up would be to implement a var that tells us how many rows were actually rendered in the legend and do something with it in the onerow-detection.

@Braintelligence
Copy link
Author

Braintelligence commented Feb 4, 2019

Since using the new version I have another issue now.
I have a hidden div that I render to get the height of the legend for positioning of annotations in a downloadable PNG.

The height of the legend isn't calculated correctly anymore for an edge-case of a few pixels; it thinks that a legend fits in one row even though it doesn't. Adding one more character to a legend item or subtracting a few pixels from the div width helps, but something is off here.

Nevermind, my bad. It was a differing marginright... Sorry 🐱

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

Successfully merging a pull request may close this issue.

2 participants