Fix support for Latex in legends

[jonmmease]

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 in src/components/legend/draw.js) triggers an async rendering call for MathJax text, and then the call to computeLegendDimensions (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:

[etpinard]

Looking good to my 👀

Thanks for splitting commits f23eb74 and 74328f3 making this PR very easy to review.

[etpinard]

Does this fix #2303 or/and #966 ?

[jonmmease]

The alignment issue in #2303 is fixed by

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.

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 🙂

[alexcjohnson]

I don't think the scrollbar issue in #966 is reproducible in the current master.

#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.

[alexcjohnson]

Fantastic work @jonmmease - 💃

[etpinard]

Awesome work @jonmmease

This one will be released today as part of 1.41.1.

[etpinard]

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!

[xhluca]

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!

[jonmmease]

@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.

[xhluca]

Thank you

[jonmmease]

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 in src/components/legend/draw.js) triggers an async rendering call for MathJax text, and then the call to computeLegendDimensions (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:

[etpinard]

Looking good to my 👀

Thanks for splitting commits f23eb74 and 74328f3 making this PR very easy to review.

[etpinard]

Does this fix #2303 or/and #966 ?

[jonmmease]

The alignment issue in #2303 is fixed by

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.

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 🙂

[alexcjohnson]

I don't think the scrollbar issue in #966 is reproducible in the current master.

#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.

[alexcjohnson]

Fantastic work @jonmmease - 💃

[etpinard]

Awesome work @jonmmease

This one will be released today as part of 1.41.1.

[etpinard]

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!

[xhluca]

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!

[jonmmease]

@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.

[xhluca]

Thank you

[jonmmease]

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 in src/components/legend/draw.js) triggers an async rendering call for MathJax text, and then the call to computeLegendDimensions (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:

[etpinard]

Looking good to my 👀

Thanks for splitting commits f23eb74 and 74328f3 making this PR very easy to review.

[etpinard]

Does this fix #2303 or/and #966 ?

[jonmmease]

The alignment issue in #2303 is fixed by

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.

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 🙂

[alexcjohnson]

I don't think the scrollbar issue in #966 is reproducible in the current master.

#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.

[alexcjohnson]

Fantastic work @jonmmease - 💃

[etpinard]

Awesome work @jonmmease

This one will be released today as part of 1.41.1.

[etpinard]

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!

[xhluca]

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!

[jonmmease]

@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.

[xhluca]

Thank you

[jonmmease]

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 in src/components/legend/draw.js) triggers an async rendering call for MathJax text, and then the call to computeLegendDimensions (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:

[etpinard]

Looking good to my 👀

Thanks for splitting commits f23eb74 and 74328f3 making this PR very easy to review.

[etpinard]

Does this fix #2303 or/and #966 ?

[jonmmease]

The alignment issue in #2303 is fixed by

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.

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 🙂

[alexcjohnson]

I don't think the scrollbar issue in #966 is reproducible in the current master.

#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.

[alexcjohnson]

Fantastic work @jonmmease - 💃

[etpinard]

Awesome work @jonmmease

This one will be released today as part of 1.41.1.

[etpinard]

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!

[xhluca]

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!

[jonmmease]

@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.

[xhluca]

Thank you

[jonmmease]

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 in src/components/legend/draw.js) triggers an async rendering call for MathJax text, and then the call to computeLegendDimensions (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:

[etpinard]

Looking good to my 👀

Thanks for splitting commits f23eb74 and 74328f3 making this PR very easy to review.

[etpinard]

Does this fix #2303 or/and #966 ?

[jonmmease]

The alignment issue in #2303 is fixed by

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.

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 🙂

[alexcjohnson]

I don't think the scrollbar issue in #966 is reproducible in the current master.

#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.

[alexcjohnson]

Fantastic work @jonmmease - 💃

[etpinard]

Awesome work @jonmmease

This one will be released today as part of 1.41.1.

[etpinard]

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!

[xhluca]

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!

[jonmmease]

@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.

[xhluca]

Thank you

[jonmmease]

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 in src/components/legend/draw.js) triggers an async rendering call for MathJax text, and then the call to computeLegendDimensions (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:

[etpinard]

Looking good to my 👀

Thanks for splitting commits f23eb74 and 74328f3 making this PR very easy to review.

[etpinard]

Does this fix #2303 or/and #966 ?

[jonmmease]

The alignment issue in #2303 is fixed by

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.

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 🙂

[alexcjohnson]

I don't think the scrollbar issue in #966 is reproducible in the current master.

#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.

[alexcjohnson]

Fantastic work @jonmmease - 💃

[etpinard]

Awesome work @jonmmease

This one will be released today as part of 1.41.1.

[etpinard]

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!

[xhluca]

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!

[jonmmease]

@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.

[xhluca]

Thank you

[jonmmease]

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 in src/components/legend/draw.js) triggers an async rendering call for MathJax text, and then the call to computeLegendDimensions (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:

[etpinard]

Looking good to my 👀

Thanks for splitting commits f23eb74 and 74328f3 making this PR very easy to review.

[etpinard]

Does this fix #2303 or/and #966 ?

[jonmmease]

The alignment issue in #2303 is fixed by

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.

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 🙂

[alexcjohnson]

I don't think the scrollbar issue in #966 is reproducible in the current master.

#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.

[alexcjohnson]

Fantastic work @jonmmease - 💃

[etpinard]

Awesome work @jonmmease

This one will be released today as part of 1.41.1.

[etpinard]

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!

[xhluca]

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!

[jonmmease]

@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.

[xhluca]

Thank you

[jonmmease]

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 in src/components/legend/draw.js) triggers an async rendering call for MathJax text, and then the call to computeLegendDimensions (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:

[etpinard]

Looking good to my 👀

Thanks for splitting commits f23eb74 and 74328f3 making this PR very easy to review.

[etpinard]

Does this fix #2303 or/and #966 ?

[jonmmease]

The alignment issue in #2303 is fixed by

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.

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 🙂

[alexcjohnson]

I don't think the scrollbar issue in #966 is reproducible in the current master.

#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.

[alexcjohnson]

Fantastic work @jonmmease - 💃

[etpinard]

Awesome work @jonmmease

This one will be released today as part of 1.41.1.

[etpinard]

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!

[xhluca]

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!

[jonmmease]

@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.

[xhluca]

Thank you