Build svg text #1783
Build svg text #1783
Conversation
also show zero-width-space characters explicitly
| } else { | ||
| throw new Error('Cannot initialize DOMParser'); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🔪 DOMParser - but now replaced with something that's tested to work in IE 🎉
|
|
||
| exports.xml_entity_encode = function(str) { | ||
| return str.replace(/&(?!\w+;|\#[0-9]+;| \#x[0-9A-F]+;)/g, '&'); | ||
| }; |
There was a problem hiding this comment.
moved these encoders to snaphost/tosvg, which is now the only place they're used because svg_text_utils is creating text nodes directly, so doesn't need to do any conversion/sanitizing.
| @@ -301,7 +301,8 @@ describe('svg+text utils', function() { | |||
| var node = mockTextSVGElement('SO<sub>4<br>44</sub>'); | |||
| expect(node.html()).toBe( | |||
| '<tspan class="line" dy="0em">SO' + | |||
| '<tspan style="font-size:70%" dy="0.3em">4</tspan></tspan>' + | |||
| '<tspan style="font-size:70%" dy="0.3em">4</tspan>' + | |||
| '<tspan dy="-0.21em"></tspan></tspan>' + | |||
There was a problem hiding this comment.
I don't think this has any visual consequences, but <sup>/<sub> tags previously didn't get their proper reset elements when they spanned line breaks.
| @@ -74,7 +74,7 @@ describe('svg+text utils', function() { | |||
| } | |||
|
|
|||
| afterEach(function() { | |||
| d3.select('#text').remove(); | |||
| d3.selectAll('.text-tester').remove(); | |||
There was a problem hiding this comment.
the cases I added (previously and in the next commit) that test several input strings with identical output were not properly cleaned up with just select.
There was a problem hiding this comment.
Nice! I've seen those forgotten <text> nodes before while npm run test-jasmine. Thanks 🎉
| var textCases = [ | ||
| '<b><i><sup>many<br>lines<br>modified', | ||
| '<b><i><sup>many<br>lines<br>modified</sup></i></b>', | ||
| '<b><i><sup>many</sup><br><sup>lines</sup></i><br><i><sup>modified', |
There was a problem hiding this comment.
This behavior is supported by regular HTML - and can be really useful to decouple styling from line breaks.
We tried weakly to support this previously but only allowed closing and reopening a single tag. The new version made it easy to handle arbitrary nesting.
| @@ -264,19 +210,14 @@ var ENTITY_TO_UNICODE = Object.keys(stringMappings.entityToUnicode).map(function | |||
| }; | |||
| }); | |||
|
|
|||
| var UNICODE_TO_ENTITY = Object.keys(stringMappings.unicodeToEntity).map(function(k) { | |||
There was a problem hiding this comment.
So, this part was useless?
There was a problem hiding this comment.
Oh I see, right, this is useless now as we're building the text nodes directly instead of using window.DomParser. Nice!
src/lib/svg_text_utils.js
Outdated
| currentNode = nodeStack[nodeStack.length - 1].node; | ||
| } | ||
|
|
||
| var hasLines = str.match(BR_TAG); |
There was a problem hiding this comment.
var hasLines = BR_TAG.test(str);would be best here, as .test always returns a boolean. Moreover, I remember benchmarking regex.test vs str.match, and .test a while ago (RIP jsFiddle) came out on top.
There was a problem hiding this comment.
nice catch! done in 3113795
|
Looks awesome. Building 💃 |
For our rich text displays: instead of converting pseudo-HTML into svg XML and then parsing this into elements, this PR converts the pseudo-HTML directly into
<tspan>(and<a>) elements. Interestingly the speed gain isn't as pronounced as it was in #1776 usinginnerHTMLbut it helps a bit, and also addresses a couple of deficiencies in the previous version.