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

Legend overlaps since 1.11.0 #771

Closed
lp-jonathan opened this issue Jul 24, 2016 · 19 comments · Fixed by #4160
Closed

Legend overlaps since 1.11.0 #771

lp-jonathan opened this issue Jul 24, 2016 · 19 comments · Fixed by #4160
Assignees
Labels
bug something broken
Milestone

Comments

@lp-jonathan
Copy link

lp-jonathan commented Jul 24, 2016

This appears to be a regression introduced in 1.11.0 - probably by: #53 and is still evident in "latest;

In 1.10.2 and earlier, when a legend was too wide and would overlap a pie chart, the legend would not show.
Since 1.11.0, the legend is shown overlapping the pie chart.

This can be replicated very easily with the following data declaration (default everything else):

var data = [{
    labels:['hello','this is a very long string and bad things are likely to happen', 'short string','moderately long string'],
    values:[4,4,4,4],
    type:'pie'
}];

Of these two behaviours, not showing the legend is preferred. The ideal solution (feature request I guess) would be to scale the legend to the a suitable non-overlapping size, then wrap the text within that.

@etpinard
Copy link
Contributor

etpinard commented Jul 25, 2016

Reproducible example here: http://codepen.io/etpinard/pen/dXZQGP

Is this really a bug though? To me the behaviour in latest makes more sense - where by default (when no layout.legend options have been set) the legend items are squeezed into the graph.

That said, something is up with legend positioning. Take a look at http://codepen.io/etpinard/pen/mELQPA - even by setting legend.x to something > 1, the legend refuses to be placed on the right of the graph.

I doubt that this affects only pie charts. The legend drawing code is pretty trace type agnostic. To be investigated.

@etpinard etpinard changed the title Legend overlaps pie chart since 1.11.0 Legend overlaps since 1.11.0 Jul 25, 2016
@lp-jonathan
Copy link
Author

Thanks for creating the reproduction.

Curious - in my application when I had a too-long legend, it simply
didn't show the legend, I assumed that was a design feature. But your
first 1.10 example shows the legend but mostly obscured. In that
scenario neither is ideal. The second example is the best of the three.

My suggestion would be to enable word-wrap and allow the developer to
specify the maximum width of the legend.

On 25/07/2016 18:38, Étienne Tétreault-Pinard wrote:

Reproducible example here: http://codepen.io/etpinard/pen/dXZQGP

Is this really a bug though? To me the behaviour in |latest| makes
more sense - where by default (when no |layout.legend| options have
been set) the legend items are squeezed into the graph.

That said, something is up with legend positioning. Take a look at
http://codepen.io/etpinard/pen/mELQPA - even by setting |legend.x| to
something |> 1|, the legend refuses to be placed on the right of the
graph.

I doubt that affect only pie charts. The legend drawing code is pretty
trace type agnostic. To be investigated.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#771 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ATqs82JMf8dyw7MjT2kJoR6grLkiOOm-ks5qZPSigaJpZM4JTiVV.

@n-riesco

This comment has been minimized.

@etpinard
Copy link
Contributor

Another report in #840


From @odow

Narrow screens cause horizontal legends with lots of entries to bug.

https://codepen.io/odow/pen/mEarpy

In this example, the three column layout is fine, but when the width forces a two column layout, the legend is no longer below the plot area but on top of, and scrolling is enabled.


From @n-riesco

The problem is in the current algorithm to position the legend. The algorithm goes like this:

  1. Plotly tries to locate the legend at the requested x and y.
  2. If the legend doesn't fit there, Plotly tries to expand the plot margins.
  3. If, after expanding the margins, any of the sides of the legend box is still outside the plotting area, Plotly resizes the legend box to fit into the plotting area (This is what we're seeing in the codepen example above; the top and bottom sides of the legend box were outside the plotting area, and Plotly has moved them into the plotting area).
  4. If, after resizing, the legend doesn't fit into the legend box, a vertical scrollbar is added.

In the case of horizontal legends that don't fit into the plotting area (even after trying to expand the margins), this algorithm ends up with a legend box that covers the whole plotting area.

I guess that the first question to answer is what Plotly should do with horizontal legends that don't fit the plotting area (even after expanding the plot margins).

@mattdonut
Copy link

I've been looking into the issues with legend placement and auto margins in horizontal mode, and I have found a few things.

https://jsfiddle.net/Lfxvm3zz/2/

It looks like Plotly goes through a few steps when placing the legend:

  1. Compute the legend's dimensions
  2. Expand the base margins to accommodate the legend if it can
  3. If the legend is still passing outside the bounds of the viewport, move it to bring it into view (potentially on top of the plot)
  4. Clip the legend to a rect no larger than the plot
  5. Add a scrollbar if needed

Because the margins are checked before the scrollbar clipping is done, it always considers the full height of the legend. This height can easily be taller than the viewport if you have a large number of traces with any long trace names, which will cause Plotly to not be able to include the legends height in the margin spacings. In these cases, no margin is added and the legend is pushed up onto the plot, then clipped at the height of the plot, covering it almost completely.
The scrollbar axis and the margin scaling axis align for horizontal legends, so there is no way for Plotly to infer the height the legend should be clipped to.

I think a maxHeight attribute or some other method of deciding how tall the legend should be allowed to be may be needed for horizontal mode legends so that it can be included in the margin calculations.

@didip
Copy link

didip commented Mar 15, 2017

Hi all,

Related to this "overlapping legend when placed at the bottom" problem,

Have you guys considered providing a JS method that returns each item in the legend and the chosen colors (Plotly.getLegend() for example)? Then user can draw their own legends on plain HTML.

Is that a possibility?

@PPPW
Copy link

PPPW commented Aug 31, 2017

I have got a similar issue:
https://codepen.io/anon/pen/BdvvKV?editors=1010

When the legend text is too long, it also overlaps. You can change the "name" of the trace to see the difference.
I wonder is there a way to tell when it will overlap? If so then it's fine to me: when I know it will overlap, I can hide the legend.
I can kind of tell when it will overlap by checking whether:
$('#chartDiv g.xy').attr('transform') == $('#chartDiv g.legend').attr('transform').replace(/ /g, '')

But it'll be better if Plotly can provide some function to tell this... Thanks!

@seansfkelley
Copy link

This is still a problem on latest. I also have a pen that reproduces the issue in a manner similar to how it manifests in my project, where some plots have a large number of legend items. If you resize the window to make it narrower, you will see the legend jump around between being well-behaved, ending up above the chart or obscuring the chart entirely.

My space constraints are pretty lax, but I wasn't able to find a suitable workaround. I was hoping to find some kind of setting that would do something like preserve a fixed size for the plot area and then use up as much additional space as it needs for the legend, but such an option doesn't appear to exist.

@brylie
Copy link

brylie commented May 29, 2019

As is becoming evident, this is a general issue with any Plotly chart rendered in a small container. For what it's worth, this issue is being 'wrestled with' in downstream projects, e.g. like this:

https://github.com/getredash/redash/blob/9292ae8d3fe3d22b10554a10ce4cefe76214c5d4/client/app/visualizations/chart/plotly/utils.js#L746-L807

@brylie
Copy link

brylie commented May 29, 2019

Basically, there isn't much reason I can think of for a legend to overlap the chart. It basically defeats the purpose of the visualization, particularly when the legend obstructs a significant portion of the chart.

@seansfkelley
Copy link

FWIW my workaround ended up being providing the users a button that doubles the height of the chart on demand. Pretty lame, but easier to implement than a drag-to-resize feature. I couldn't find an effective way to automatically determine whether the plot in an unusable state, so I made it the user's problem instead (luckily that's acceptable for my use-case, but it still rubs me the wrong way).

@Nauss
Copy link

Nauss commented Jul 5, 2019

In my use case (see #4018), regardless of the problem of the legend it self (whether it should overlap the plot or not), the plot area size computation is broken.

We clearly see in the second pen that even though the choice is made to display the legend over the plot, the plot area is computed like the legend was outside.

Could this be fixed before this whole issue ?

Regarding the issue, I would agree with @brylie in this comment. And in the case the user still wants to display the legend over the plot, he/she should be able to do it by setting the legend positioning options.

@rrmn
Copy link

rrmn commented Jul 9, 2019

Possible workaround

(taken from several SO threads and GH issues)

in server.R

output$plot <- renderPlotly({
    ggplotly(gg) %>% 
        layout(legend = list(orientation = "h", x = 0.4, y = -0.2)) %>%
        style(legendgroup = NULL)
})

in ui.R

fluidRow(    
    box(plotlyOutput("plot", height = "100%", width = "100%"),
        width = 6, height = 500)

It's super hacky, but an approach similar to above (mostly after playing around with different width and height of the box()) fixes 95% of the issues in our case.

@etpinard etpinard added this to the v1.50.0 milestone Aug 16, 2019
@etpinard
Copy link
Contributor

etpinard commented Aug 16, 2019

Hi all, first I'd like to apologise for not taking care of the numerous bugs reported here sooner. I'm happy to say that the next minor release (v1.50.0) should fix the reported issues. At the very least, we will offer workarounds, most likely by adding a new legend attribute.

drop-firstRender-in-legend-draw is the branch I'm currently using. You can give it a spin using: https://48109-45646037-gh.circle-artifacts.com/0/dist/plotly.js

Note that some of the work here overlaps with PR #3024 that we should close, by the way. Moreover, the work done here will make implementing #1340, #3151, #1199 and #960 (to name a few) much easier.


In brief, two things made fixing these bugs pretty tricky:

  • how Legend.draw constrains the legend x, y positions in order to always make the legend fit (no matter how big) inside the graph's width/height viewport. (see 1.49.2 logic here)
  • how our auto-margin algo drops "push" values greater than half of the graph's width or height (that's done here)

Both points could be considered wrong and simply dropped, but I chose to honour them (at least until a major bump).


Going through the reported examples:

https://codepen.io/etpinard/pen/dXZQGP shows the legend overlapping the plot area. Here, the constrained legend x position is wrong. See https://codepen.io/etpinard/pen/VwZjjpm which shows both a non-constrained version (using new attribute tentatively named fitinside) and the fixed constrained version.

https://codepen.io/odow/pen/mEarpy shows a legend with a bad constrained y position and a bad computed height. See fixed version in https://codepen.io/etpinard/pen/KKPMMqb

https://jsfiddle.net/Lfxvm3zz/2/ does not honour legend.x whatsoever. That one still behaves badly on my dev branch.

https://codepen.io/Nauss/pen/MMBdrw shows a very bad case of a wrong x position constraint. Fixed in https://codepen.io/etpinard/pen/GRKqqQO

https://codepen.io/anon/pen/KbZbRv?editors=0010 show bad behaviour on window resize, see fixed version here: https://codepen.io/etpinard/pen/MWgeeQM

From #2337 https://jsfiddle.net/6oq39dpy/6/ shows a messed up y constraint. Fixed in https://jsfiddle.net/2dm3y158/

From #2131 https://codepen.io/etpinard/pen/ZaEEyL?editors=0010 and https://codepen.io/plotly/pen/gvEaRK show bad behaviour for horizontal legends with legend.x < 0. See fixed versions in https://codepen.io/etpinard/pen/VwZjjxb?editors=1010 and https://codepen.io/etpinard/pen/RwbRRyL

You can also check out the image diffs off 7d0a524 for more before/after comparisons.

cc @plotly/plotly_js

@etpinard
Copy link
Contributor

https://jsfiddle.net/Lfxvm3zz/2/ does not honour legend.x whatsoever. That one still behaves badly on my dev branch.

Now fixed with d568610 (bundle: https://48124-45646037-gh.circle-artifacts.com/0/dist/plotly.min.js) -> https://jsfiddle.net/yuwfnj7t/

@seansfkelley
Copy link

Awesome, thanks @etpinard! Can't wait to get these fixes and remove my unfortunate workaround.

etpinard added a commit that referenced this issue Aug 30, 2019
- see #771
- introduce measures _maxHeight, _maxWidth and _effHeight
  to track margin pushes, scrollbox and horizontal row wrapping
- simplify (and fix) legend (x,y) constraint into graph width/height
- introduce some l,r,b,y "max" margin-push logic
- update baselines!
  + from previous "new legend mocks" commit
  + and `horizontal_wrap-alll-lines` which now spans the graph's width
etpinard added a commit that referenced this issue Aug 30, 2019
- see #771
- introduce measures _maxHeight, _maxWidth and _effHeight
  to track margin pushes, scrollbox and horizontal row wrapping
- simplify (and fix) legend (x,y) constraint into graph width/height
- introduce some l,r,b,y "max" margin-push logic
- paves way to expose legend `maxheight` and `maxwidth`
- update baselines!
  + from previous "new legend mocks" commit
  + and `horizontal_wrap-alll-lines` which now spans the graph's width
etpinard added a commit that referenced this issue Aug 30, 2019
- see #771
- introduce measures _maxHeight, _maxWidth and _effHeight
  to track margin pushes, scrollbox and horizontal row wrapping
- simplify (and fix) legend (x,y) constraint into graph width/height
- introduce some l,r,b,y "max" margin-push logic
- paves way to expose legend `maxheight` and `maxwidth`
- update baselines!
  + from previous "new legend mocks" commit
  + and `horizontal_wrap-alll-lines` which now spans the graph's width
etpinard added a commit that referenced this issue Aug 30, 2019
- see #771
- introduce measures _maxHeight, _maxWidth and _effHeight
  to track margin pushes, scrollbox and horizontal row wrapping
- simplify (and fix) legend (x,y) constraint into graph width/height
- introduce some l,r,b,y "max" margin-push logic
- paves way to expose legend `maxheight` and `maxwidth`
- update baselines!
  + from previous "new legend mocks" commit
  + and `horizontal_wrap-alll-lines` which now spans the graph's width
etpinard added a commit that referenced this issue Aug 30, 2019
- see #771
- introduce measures _maxHeight, _maxWidth and _effHeight
  to track margin pushes, scrollbox and horizontal row wrapping
- simplify (and fix) legend (x,y) constraint into graph width/height
- introduce some l,r,b,y "max" margin-push logic
- paves way to expose legend `maxheight` and `maxwidth`
- update baselines!
  + from previous "new legend mocks" commit
  + and `horizontal_wrap-alll-lines` which now spans the graph's width
@TUSHAR4541
Copy link

Great job @etpinard.
One suggestion, Can we pass max-height/height of legend box ?

@etpinard
Copy link
Contributor

etpinard commented Sep 6, 2019

One suggestion, Can we pass max-height/height of legend box ?

Hi @TUSHAR4541 - I wasn't planning on adding that for the next release (v1.50.0), but it should be fairly easy to add eventually. You can subscribe to #1340 for the latest updates.

etpinard added a commit that referenced this issue Sep 19, 2019
- see #771
- introduce measures _maxHeight, _maxWidth and _effHeight
  to track margin pushes, scrollbox and horizontal row wrapping
- simplify (and fix) legend (x,y) constraint into graph width/height
- introduce some l,r,b,y "max" margin-push logic
- paves way to expose legend `maxheight` and `maxwidth`
- update baselines!
  + from previous "new legend mocks" commit
  + and `horizontal_wrap-alll-lines` which now spans the graph's width
VerstandInvictus referenced this issue Dec 13, 2019
- when one sets autoexpand to false, fullLayout.margin are
  honoured no matter how big the legend gets. In this case, (I'd argue)
  the legend should NOT move from its provided (or default) (x,y)
  position to make it fit on the graph.
- if some users want the legend to auto-expand the margin while keeping
  the provided/default (x,y), we could eventually add a `legend.fitinside`
  boolean attribute
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.