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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 2 commits into from
Sep 9, 2019
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bcc1aba
fix #4152 - convert <br> -> \n in scattermapbox text
etpinard Sep 9, 2019
b5a3f25
add <br> and \n in mapbox_bubble-tet mock
etpinard Sep 9, 2019
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: 1 addition & 1 deletion src/lib/svg_text_utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ var ZERO_WIDTH_SPACE = '\u200b';
*/
var PROTOCOLS = ['http:', 'https:', 'mailto:', '', undefined, ':'];

var NEWLINES = /(\r\n?|\n)/g;
var NEWLINES = exports.NEWLINES = /(\r\n?|\n)/g;

var SPLIT_TAGS = /(<[^<>]*>)/;

Expand Down
11 changes: 8 additions & 3 deletions src/traces/scattermapbox/convert.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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?


module.exports = function convert(gd, calcTrace) {
var trace = calcTrace[0].trace;

Expand Down Expand Up @@ -234,14 +237,13 @@ function makeSymbolGeoJSON(calcTrace, gd) {

var marker = trace.marker || {};
var symbol = marker.symbol;
var text = trace.text;

var fillSymbol = (symbol !== 'circle') ?
getFillFunc(symbol) :
blankFillFunc;

var fillText = subTypes.hasText(trace) ?
getFillFunc(text) :
getFillFunc(trace.text) :
blankFillFunc;

var features = [];
Expand All @@ -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

plotly.js/src/lib/svg_text_utils.js

Lines 449 to 626 in 4ef2f80

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.


features.push({
type: 'Feature',
geometry: {
Expand All @@ -269,7 +274,7 @@ function makeSymbolGeoJSON(calcTrace, gd) {
},
properties: {
symbol: fillSymbol(calcPt.mx),
text: txt ? calcPt.txt : fillText(calcPt.tx)
text: text
}
});
}
Expand Down
Binary file modified test/image/baselines/mapbox_bubbles-text.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 2 additions & 2 deletions test/image/mocks/mapbox_bubbles-text.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@
},
"text": [
"A",
"B",
"C"
"B \n b",
"C <br> c"
],
"textposition": "top left",
"textfont": {
Expand Down
26 changes: 26 additions & 0 deletions test/jasmine/tests/scattermapbox_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,32 @@ describe('scattermapbox convert', function() {
expect(actualText).toEqual(['A', 'B', 'C', 'F', '']);
});

it('should convert \\n to \'\' and <br> to \\n', function() {
var opts = _convert(Lib.extendFlat({}, base, {
mode: 'text',
text: ['one\nline', 'two<br>lines', 'three<BR>lines<br />yep']
}));

var actualText = opts.symbol.geojson.features.map(function(f) {
return f.properties.text;
});

expect(actualText).toEqual(['oneline', 'two\nlines', 'three\nlines\nyep', undefined, undefined]);
});

it('should convert \\n to \'\' and <br> to \\n - texttemplate case', function() {
var opts = _convert(Lib.extendFlat({}, base, {
mode: 'text',
texttemplate: ['%{lon}\none\nline', '%{lat}<br>two<br>lines', '%{lon}\n%{lat}<br>more<br>lines']
}));

var actualText = opts.symbol.geojson.features.map(function(f) {
return f.properties.text;
});

expect(actualText).toEqual(['10oneline', '20\ntwo\nlines', '3010\nmore\nlines', '', '']);
});

it('should generate correct output for texttemplate', function() {
var mock = {
'type': 'scattermapbox',
Expand Down