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

Properly handle <br> and \n in scattermapbox text #4176

Merged
merged 2 commits into from
Sep 9, 2019

Conversation

etpinard
Copy link
Contributor

@etpinard etpinard commented Sep 9, 2019

fixes #4152

In mapbox-gl, users have to input \n to break lines in text while we expect <br> to do that. This PR fixes the inconsistencies between scattermapbox text and all other plotly.js text attributes.

before: https://codepen.io/etpinard/pen/vYBdoeJ
after: https://codepen.io/etpinard/pen/OJLQKxZ

cc @plotly/plotly_js

@etpinard etpinard added bug something broken status: reviewable labels Sep 9, 2019
@@ -20,6 +20,9 @@ var makeBubbleSizeFn = require('../scatter/make_bubble_size_func');
var subTypes = require('../scatter/subtypes');
var convertTextOpts = require('../../plots/mapbox/convert_text_opts');

var NEWLINES = require('../../lib/svg_text_utils').NEWLINES;
var BR_TAG_ALL = require('../../lib/svg_text_utils').BR_TAG_ALL;
Copy link
Contributor

Choose a reason for hiding this comment

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

Non-blocking: @etpinard would you mind requiring svg_text_utils once?

@@ -261,6 +263,9 @@ function makeSymbolGeoJSON(calcTrace, gd) {
calcPt.txt = Lib.texttemplateString(txti, {}, gd._fullLayout._d3locale, calcPt, trace._meta || {});
}

var text = txt ? calcPt.txt : fillText(calcPt.tx);
if(text) text = text.replace(NEWLINES, '').replace(BR_TAG_ALL, '\n');
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting.
Wondering this logic may be useful in other places.
If so, it would be nice to have it in Lib.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's part of

function buildSVGText(containerNode, str) {
/*
* Normalize behavior between IE and others wrt newlines and whitespace:pre
* this combination makes IE barf https://github.com/plotly/plotly.js/issues/746
* Chrome and FF display \n, \r, or \r\n as a space in this mode.
* I feel like at some point we turned these into <br> but currently we don't so
* I'm just going to cement what we do now in Chrome and FF
*/
str = str.replace(NEWLINES, ' ');
var hasLink = false;
// as we're building the text, keep track of what elements we're nested inside
// nodeStack will be an array of {node, type, style, href, target, popup}
// where only type: 'a' gets the last 3 and node is only added when it's created
var nodeStack = [];
var currentNode;
var currentLine = -1;
function newLine() {
currentLine++;
var lineNode = document.createElementNS(xmlnsNamespaces.svg, 'tspan');
d3.select(lineNode).attr({
class: 'line',
dy: (currentLine * LINE_SPACING) + 'em'
});
containerNode.appendChild(lineNode);
currentNode = lineNode;
var oldNodeStack = nodeStack;
nodeStack = [{node: lineNode}];
if(oldNodeStack.length > 1) {
for(var i = 1; i < oldNodeStack.length; i++) {
enterNode(oldNodeStack[i]);
}
}
}
function enterNode(nodeSpec) {
var type = nodeSpec.type;
var nodeAttrs = {};
var nodeType;
if(type === 'a') {
nodeType = 'a';
var target = nodeSpec.target;
var href = nodeSpec.href;
var popup = nodeSpec.popup;
if(href) {
nodeAttrs = {
'xlink:xlink:show': (target === '_blank' || target.charAt(0) !== '_') ? 'new' : 'replace',
target: target,
'xlink:xlink:href': href
};
if(popup) {
// security: href and target are not inserted as code but
// as attributes. popup is, but limited to /[A-Za-z0-9_=,]/
nodeAttrs.onclick = 'window.open(this.href.baseVal,this.target.baseVal,"' +
popup + '");return false;';
}
}
} else nodeType = 'tspan';
if(nodeSpec.style) nodeAttrs.style = nodeSpec.style;
var newNode = document.createElementNS(xmlnsNamespaces.svg, nodeType);
if(type === 'sup' || type === 'sub') {
addTextNode(currentNode, ZERO_WIDTH_SPACE);
currentNode.appendChild(newNode);
var resetter = document.createElementNS(xmlnsNamespaces.svg, 'tspan');
addTextNode(resetter, ZERO_WIDTH_SPACE);
d3.select(resetter).attr('dy', RESET_DY[type]);
nodeAttrs.dy = SHIFT_DY[type];
currentNode.appendChild(newNode);
currentNode.appendChild(resetter);
} else {
currentNode.appendChild(newNode);
}
d3.select(newNode).attr(nodeAttrs);
currentNode = nodeSpec.node = newNode;
nodeStack.push(nodeSpec);
}
function addTextNode(node, text) {
node.appendChild(document.createTextNode(text));
}
function exitNode(type) {
// A bare closing tag can't close the root node. If we encounter this it
// means there's an extra closing tag that can just be ignored:
if(nodeStack.length === 1) {
Lib.log('Ignoring unexpected end tag </' + type + '>.', str);
return;
}
var innerNode = nodeStack.pop();
if(type !== innerNode.type) {
Lib.log('Start tag <' + innerNode.type + '> doesnt match end tag <' +
type + '>. Pretending it did match.', str);
}
currentNode = nodeStack[nodeStack.length - 1].node;
}
var hasLines = BR_TAG.test(str);
if(hasLines) newLine();
else {
currentNode = containerNode;
nodeStack = [{node: containerNode}];
}
var parts = str.split(SPLIT_TAGS);
for(var i = 0; i < parts.length; i++) {
var parti = parts[i];
var match = parti.match(ONE_TAG);
var tagType = match && match[2].toLowerCase();
var tagStyle = TAG_STYLES[tagType];
if(tagType === 'br') {
newLine();
} else if(tagStyle === undefined) {
addTextNode(currentNode, convertEntities(parti));
} else {
// tag - open or close
if(match[1]) {
exitNode(tagType);
} else {
var extra = match[4];
var nodeSpec = {type: tagType};
// now add style, from both the tag name and any extra css
// Most of the svg css that users will care about is just like html,
// but font color is different (uses fill). Let our users ignore this.
var css = getQuotedMatch(extra, STYLEMATCH);
if(css) {
css = css.replace(COLORMATCH, '$1 fill:');
if(tagStyle) css += ';' + tagStyle;
} else if(tagStyle) css = tagStyle;
if(css) nodeSpec.style = css;
if(tagType === 'a') {
hasLink = true;
var href = getQuotedMatch(extra, HREFMATCH);
if(href) {
// check safe protocols
var dummyAnchor = document.createElement('a');
dummyAnchor.href = href;
if(PROTOCOLS.indexOf(dummyAnchor.protocol) !== -1) {
// Decode href to allow both already encoded and not encoded
// URIs. Without decoding prior encoding, an already encoded
// URI would be encoded twice producing a semantically different URI.
nodeSpec.href = encodeURI(decodeURI(href));
nodeSpec.target = getQuotedMatch(extra, TARGETMATCH) || '_blank';
nodeSpec.popup = getQuotedMatch(extra, POPUPMATCH);
}
}
}
enterNode(nodeSpec);
}
}
}
return hasLink;
}

used for all SVG text.

@archmoj
Copy link
Contributor

archmoj commented Sep 9, 2019

@etpinard nicely done.
💃

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.

Scattermapbox, Choroplethmapbox- '<br>' tag is not respected on displaying a text
2 participants