-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Fix support for Latex in legends #3018
Conversation
legend dimensions
Also extract the number 40 into a new legend constant "textOffsetX"
The alignment issue in #2303 is fixed by
I don't think the scrollbar issue in #966 is reproducible in the current master. I've never seen a scroll bar (with or without this fix), but I can confirm that the particular example in the codepen of #996 renders fine now and without a scroll bar. So I'd call it fixed 🙂 |
#966 seems to have disappeared between 1.34 and 1.35 despite the fact that they both have the horizontal alignment problem. Likely #2426 fixed the scrollbar issue, but since MathJax was unusable in legends until this PR, I'll let you close #966 when this PR is merged. |
Fantastic work @jonmmease - 💃 |
Awesome work @jonmmease This one will be released today as part of |
I'm all set to release 1.41.1, so I'll go ahead and merge this PR. I hope @jonmmease won't mind. Thanks again for this fix! |
Amazing! I was waiting for this one to fix my regression app. Any idea when 1.41.1 will be bundled for plotly.py? Thanks! |
@xhlulu it will be in plotly.py 3.3 which should be out by the end of next week. If plotly.js releases a 1.41.2 next week we'll try to line up with that. |
Thank you |
This PR introduces a few small fixes for MathJax Latex names in the legend. I know this has worked in the past, but it has not worked consistently in my testing. When it "doesn't work" the entire legend is not shown, and there are SVG console errors complaining that
rect
width/height cannot be set to NaN.I believe this to be a concurrency issue that is addressed in 74328f3. The issue is that the call to
drawTexts
(line 123 insrc/components/legend/draw.js
) triggers an async rendering call for MathJax text, and then the call tocomputeLegendDimensions
(new line 130) assumes the text has already been rendered.Commit f23eb74 introduces no new logic, it just applies the indent that was omitted in 74328f3 to make the logical changes clearer.
Commit 1482077 applies the same horizontal offset to latex legend values as are applied to non-latex legend values (this was zero previously). And then I moved this to a more appropriate location and refactored to extract the constant in 222357c.
Commits acf5d7b and 6c27224 re-enable the Latex legend string in the mathjax mock. The mock now renders consistently as follows: