Uh oh!
There was an error while loading. Please reload this page.
Try out visibility:inherit default for text - #990
Conversation
| _context.text('') | ||
| .style({ | ||
| visibility: 'visible', | ||
| visibility: 'inherit', |
There was a problem hiding this comment.
This is definitively the first step to take. 👍
My only concern here is possible incompatibilities in Illustrator and svg conversion tools (e.g. Batik).
To avoid any rolling 🎲 , I'm thinking about adding a few lines to the toSVG routine making sure all exported images get visibility: 'visible' as opposed to inherit while interactive graphs would get 'inherit'.
There was a problem hiding this comment.
Good to know about. That's the sort of obscure possibility for interaction that I thought maybe could exist.
There was a problem hiding this comment.
Feel free to pull this fix wherever you need or redo it elsewhere. I won't worry further about it.
with *visibility: inherit* on SVG export
rreusser
left a comment
There was a problem hiding this comment.
This all looks good to me. I assume tspans never have visibility set. I'm also assuming that there aren't other corner cases or obscure features that would cause potential issues.
| var txt = d3.select(this); | ||
| // hidden text is pre-formatting mathjax, | ||
| // the browser ignores it but it can still confuse batik |
There was a problem hiding this comment.
This seems to make sense 👍
| return; | ||
| } | ||
| else { | ||
| // force other visibility value to export as visible |
There was a problem hiding this comment.
This also makes sense. Presumably it overwrites visibility: inherit (or anything that's not hidden, for that matter), with visible. Which seems correct.
| textElements = svgDOM.getElementsByTagName('text'); | ||
| for(var i = 0; i < textElements.length; i++) { | ||
| expect(textElements[i].style.visibility).toEqual('visible'); |
rreusser
commented
Oct 12, 2016
Kinda transferred ownership of this PR. Feel free to merge if you're content, otherwise let me know if further action is required. |
fresheneesz
commented
Oct 12, 2016
Thanks! |
See #984
The goal of this PR is to get circle-ci to see what happens if the visibility default on text is
visibility: inheritinstead ofvisibility: visible.visiblehard-codes it and is difficult to override. I had an ominous feeling about this change leading to a rabbit hole of related fixes and compensations, but let's see what happens…