Plotly Charts: use .legend to determine legends size #4935
Conversation
|
There was a reason why I implemented this hack instead of using Plotly's features (query to reproduce):
I'll explain you how coordinates in Plotly work so you don't need to look for in in docs and sources. There is a plot area - it's 1x1 box with coordinates from (0.0; 0.0) to (1.0; 1.0) and everything else is placed outside of it, including axes, legend and so on. Outside of plot there are only margins (that ones we can setup in The idea of my hack was to place horizontal legend in top-left corner of plot so Plotly will not reserve extra space for it, and then measure it and offset using CSS. There is an issue to fix this behavior in Plotly (either by fixing legend positioning algorithm or by allowing to choose coordinate system - relative to plot, or plot with axes or container/absolute). But it's still not implemented 😞 |
I figured this was the case, I could see that happening with the other graph already. I just wondered if they may had added any other feature to cover this, which would've been better to us. I'll revert the test commit and go back with the "updating the fix" approach. I'll provide more details on that, but first I'll try to understand better other issues I found: - scrollbox taking part of the plot sometimes (Seems part of how Plotly handles the drawing, as other elements have their own sizing values); - Plots without legend don't resize (it's currently coupled to the "updatePlot" for the legends, so it makes sense to find a different flow). Thanks for the explanation @kravets-levko 🙂 |
gabrieldutra
changed the title
| const bounds = reduce( | ||
| legend.querySelectorAll(".traces"), | ||
| (result, node) => { | ||
| const b = node.getBoundingClientRect(); | ||
| result = result || b; | ||
| return { | ||
| top: Math.min(result.top, b.top), | ||
| bottom: Math.max(result.bottom, b.bottom), | ||
| }; | ||
| }, | ||
| null | ||
| ); |
There was a problem hiding this comment.
@kravets-levko what's causing the bug is that there is some "margin" for the legends background, computing each of the items separately don't take it into account, so it separating less than needed and cutting part of the text. I made some tests and it seems the .legend holds the same information we had before + it also considers the margin for the background size.
I'm continuing to test this change, but so far looks fine, lmk if you see any situation where this may cause a problem.
| } else { | ||
| updatePlot(plotlyElement, pick(layout, ["width", "height"])); |
There was a problem hiding this comment.
This solves another issue that Charts without legends weren't updating when resized.
* Plotly Charts: use .bg to determine legends size * Test: remove hack for legend below plotly * Revert "Test: remove hack for legend below plotly" This reverts commit d8efb0c. * Use .legend to calculate bounds * Also update plots without legend
What type of PR is this? (check all applicable)
Description
The bug (Preview)
Currently Charts that have legends moved to the bottom can "cut" part of their text.
With this fix (Preview)
Related Tickets & Documents
--
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
See description ☝️