Skip to content

No innerHTML and pseudo-html cleanup - #1792

Merged
alexcjohnson merged 37 commits into
masterfrom
no-innerHTML
Jun 19, 2017
Merged

alexcjohnson merged 37 commits into
masterfrom
no-innerHTML

Conversation

@alexcjohnson

@alexcjohnson alexcjohnson commented Jun 16, 2017 •

Copy link
Copy Markdown
Collaborator

Started out as a bugfix but kind of turned into a spring cleaning...

In an effort to fix bounding box calculations in IE (one part of #1782) - which were broken by trying to use the innerHTML property of an SVG element - I wound up going through every user of Drawing.bBox and trying to make sure it was a <text> or .*-math-group node that had previously had svgTextUtils.convertToTspans called on it. Because if so, I could use the data-unformatted attribute, along with whether or not the node was converted to MathJax, in place of the innerHTML in the cache key, which is also much smaller than the innerHTML, reducing my memory concerns about this cache.

But then while doing THAT it became clear that all the monkey business using tspan.line elements to count lines and to properly align multi-line text was a disaster, and not done properly in a lot of places, so I centralized all that in two new utilities:

  • svgTextUtils.positionText - to be used instead of Drawing.setPosition, handles multiline alignment, and also gets called within convertToTspans so if you use transform instead of the x and y attributes to position the text, you never even need to use this at all.
  • svgTextUtils.lineCount - nuff said. No more hacky and/or broken selectors for counting lines of text.

So after that, almost nobody outside of svgTextUtils even needs to know about <tspan> or the .line class.

Along the way I found a few places we were not supporting or had bugs supporting pseudo-HTML but easily could, and added it: rangeselectors, updatemenus, sliders, carpet axes (titles and tick labels). And in cases that currently break with MathJax I explicitly cancel it (with the data-notex attribute). (MathJax test mocks coming in a future PR)

And while I was working in test_syntax.js I took care of #1773 as well.

There are still a few Drawing.bBox users that call it on elements larger than a <text> or MathJax group node, that at this point in this branch we will NOT cache at all. For 🐎 I need to either convert them to their base nodes or find an alternate way to give them a robust cache key:

  • colorbar/draw:473 & 490
  • titles/index:146 & 171

Loading
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 this pull request may close these issues.

2 participants