Skip to content
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

No innerHTML and pseudo-html cleanup #1792

Merged
merged 37 commits into from
Jun 19, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
3ad11f6
standardize on attr over style for text-anchor
alexcjohnson Jun 15, 2017
9949092
centralize data-unformatted and use it for bBox cache key
alexcjohnson Jun 15, 2017
951a75e
let Drawing.bBox handle tspan.line position reset
alexcjohnson Jun 15, 2017
639878e
fix pseudo-html rendering in rangeselectors and updatemenus
alexcjohnson Jun 15, 2017
16d7515
call bBox on text node when possible
alexcjohnson Jun 15, 2017
0b2a7b2
clean up legend sizing and fallback on missing node
alexcjohnson Jun 15, 2017
a5a578f
minor lint to bar text
alexcjohnson Jun 15, 2017
5d3648c
cleaner data-math usage
alexcjohnson Jun 15, 2017
d29159a
fix sliders with pseudo-HTML line breaks
alexcjohnson Jun 15, 2017
6eedaa5
make `positionText` and `lineCount` fns
alexcjohnson Jun 16, 2017
76fc8b8
linting related to tspan.line centralization
alexcjohnson Jun 16, 2017
61dd3b1
use display:none instead of visibility:hidden
alexcjohnson Jun 16, 2017
936936b
remove Drawing.measureText and support pseudo-HTML in carpets
alexcjohnson Jun 16, 2017
4ae49f5
prohibit Math.sign and .innerHTML in syntax test
alexcjohnson Jun 16, 2017
9f4ec66
remove Math.sign
alexcjohnson Jun 16, 2017
1a5679e
fix title alignment
alexcjohnson Jun 16, 2017
af5b7f2
update snapshot test
alexcjohnson Jun 16, 2017
4886985
oops, we need y=0 on tspan.line after all
alexcjohnson Jun 16, 2017
717c7d8
hover labels need explicit position reset for proper positioning later
alexcjohnson Jun 16, 2017
7c3dc3b
get correct bbox for carpet tick labels
alexcjohnson Jun 16, 2017
378c805
fixed range selector mocks
alexcjohnson Jun 16, 2017
f8101b0
update binding mock - new label positions are slightly different
alexcjohnson Jun 16, 2017
3b293ac
update updatemenus test
alexcjohnson Jun 16, 2017
c908ed4
lint axes.js
alexcjohnson Jun 16, 2017
0bb38e9
reorder sliders findDimensions so dummy calculations always succeed
alexcjohnson Jun 16, 2017
6c72c77
test #984 - ensure we never override visibility
alexcjohnson Jun 16, 2017
30b9fa5
remove some obsolete code and comments from sliders/draw
alexcjohnson Jun 16, 2017
930e58e
updatemenus point in the direction they'll open
alexcjohnson Jun 16, 2017
301fba7
update updatemenus_positioning mock for new arrows
alexcjohnson Jun 16, 2017
4b87b2e
fix #1782 (second half: arrowheads broken) - :hocho: parentElement
alexcjohnson Jun 16, 2017
a68f861
note about innerHTML prohibition
alexcjohnson Jun 16, 2017
c5fa44b
pull 1.3 out into alignment constants
alexcjohnson Jun 16, 2017
82ba7ee
null transform, rather than empty string
alexcjohnson Jun 16, 2017
27b4be3
selectAll -> select because it can only be one node
alexcjohnson Jun 16, 2017
46d6e44
lint titles var statements
alexcjohnson Jun 17, 2017
8305c02
let drawing.bBox cache simple nested elements
alexcjohnson Jun 17, 2017
8e3d4fa
don't recalculate nodehash for recursive bBox
alexcjohnson Jun 17, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions src/components/annotations/draw.js
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,6 @@ function drawRaw(gd, options, index, subplotId, xa, ya) {

var annText = annTextGroupInner.append('text')
.classed('annotation-text', true)
.attr('data-unformatted', options.text)
.text(options.text);

function textLayout(s) {
Expand Down Expand Up @@ -693,7 +692,6 @@ function drawRaw(gd, options, index, subplotId, xa, ya) {
.call(textLayout)
.on('edit', function(_text) {
options.text = _text;
this.attr({'data-unformatted': options.text});
this.call(textLayout);

var update = {};
Expand Down
19 changes: 14 additions & 5 deletions src/components/drawing/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -615,11 +615,17 @@ var savedBBoxesCount = 0;
var maxSavedBBoxes = 10000;

drawing.bBox = function(node) {
// cache elements we've already measured so we don't have to
// Cache elements we've already measured so we don't have to
// remeasure the same thing many times
// We have a few bBox callers though who pass a node larger than
// a <text> or a MathJax <g>, such as an axis group containing many labels.
// These will not generate a hash (unless we figure out an appropriate
// hash key for them) and thus we will not hash them.
var hash = nodeHash(node);
var out = drawing.savedBBoxes[hash];
if(out) return Lib.extendFlat({}, out);
if(hash) {
var out = drawing.savedBBoxes[hash];
if(out) return Lib.extendFlat({}, out);
}

var tester = drawing.tester.node();

Expand Down Expand Up @@ -659,7 +665,7 @@ drawing.bBox = function(node) {
}

// cache this bbox
drawing.savedBBoxes[hash] = bb;
if(hash) drawing.savedBBoxes[hash] = bb;
savedBBoxesCount++;

return Lib.extendFlat({}, bb);
Expand All @@ -683,7 +689,10 @@ drawing.bBox = function(node) {
// return h;
// }
function nodeHash(node) {
return node.innerHTML +
var inputText = node.getAttribute('data-unformatted');
if(inputText === null) return;
return inputText + '~' +
node.getAttribute('data-math') +
node.getAttribute('text-anchor') +
node.getAttribute('style');
}
Expand Down
2 changes: 0 additions & 2 deletions src/components/legend/draw.js
Original file line number Diff line number Diff line change
Expand Up @@ -390,8 +390,6 @@ function drawTexts(g, gd) {
text.call(svgTextUtils.makeEditable, {gd: gd})
.call(textLayout)
.on('edit', function(text) {
this.attr({'data-unformatted': text});

this.text(text)
.call(textLayout);

Expand Down
6 changes: 2 additions & 4 deletions src/components/titles/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -193,17 +193,15 @@ Titles.draw = function(gd, titleClass, options) {
}
}

el.attr({'data-unformatted': txt})
.call(titleLayout);
el.call(titleLayout);

var placeholderText = 'Click to enter ' + name + ' title';

function setPlaceholder() {
opacity = 0;
isplaceholder = true;
txt = placeholderText;
el.attr({'data-unformatted': txt})
.text(txt)
el.text(txt)
.on('mouseover.opacity', function() {
d3.select(this).transition()
.duration(interactConstants.SHOW_PLACEHOLDER).style('opacity', 1);
Expand Down
21 changes: 15 additions & 6 deletions src/lib/svg_text_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,18 +40,23 @@ exports.convertToTspans = function(_context, gd, _callback) {
svgClass += '-math';
parent.selectAll('svg.' + svgClass).remove();
parent.selectAll('g.' + svgClass + '-group').remove();
_context.style({visibility: null});
_context.style({visibility: null})
.attr({
// some callers use data-unformatted *from the <text> element* in 'cancel'
// so we need it here even if we're going to turn it into math
// these two (plus style and text-anchor attributes) form the key we're
// going to use for Drawing.bBox
'data-unformatted': str,
'data-math': ''
});

function showText() {
if(!parent.empty()) {
svgClass = _context.attr('class') + '-math';
parent.select('svg.' + svgClass).remove();
}
_context.text('')
.style({
visibility: 'inherit',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. Why did this line get 🔪ed ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I didn't notice the original issue #984 that led to this change... assumed it was a remnant of a much older time. My goal was to remove inline styles whenever possible, rather than setting an explicit reset value, but anyway I moved from visibility:hidden to display:none because only the latter takes the item out of the bounding box, and there are still a couple of places (most notably titles/index:146 & 171 that I'm still working on) we get the bounding box of the combined text+MathJax element. I'll see if I can come up with a test of #984 to include with this PR.

'white-space': 'pre'
});
.style('white-space', 'pre');

var hasLink = buildSVGText(_context.node(), str);

Expand Down Expand Up @@ -84,7 +89,11 @@ exports.convertToTspans = function(_context, gd, _callback) {

var mathjaxGroup = parent.append('g')
.classed(svgClass + '-group', true)
.attr({'pointer-events': 'none'});
.attr({
'pointer-events': 'none',
'data-unformatted': str,
'data-math': 'Y'
});

mathjaxGroup.node().appendChild(newSvg.node());

Expand Down
6 changes: 3 additions & 3 deletions src/plot_api/plot_api.js
Original file line number Diff line number Diff line change
Expand Up @@ -480,9 +480,10 @@ function plotPolar(gd, data, layout) {
.call(titleLayout);

if(gd._context.editable) {
title.attr({'data-unformatted': txt});
if(!txt || txt === placeholderText) {
opacity = 0.2;
// placeholder is not going through convertToTspans
// so needs explicit data-unformatted
title.attr({'data-unformatted': placeholderText})
.text(placeholderText)
.style({opacity: opacity})
Expand All @@ -500,8 +501,7 @@ function plotPolar(gd, data, layout) {
this.call(svgTextUtils.makeEditable, {gd: gd})
.on('edit', function(text) {
gd.framework({layout: {title: text}});
this.attr({'data-unformatted': text})
.text(text)
this.text(text)
.call(titleLayout);
this.call(setContenteditable);
})
Expand Down
2 changes: 1 addition & 1 deletion src/snapshot/tosvg.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ module.exports = function toSVG(gd, format) {
svg.node().style.background = '';

svg.selectAll('text')
.attr('data-unformatted', null)
.attr({'data-unformatted': null, 'data-math': null})
.each(function() {
var txt = d3.select(this);

Expand Down