Skip to content

No innerHTML and pseudo-html cleanup #1792

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

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
3 changes: 2 additions & 1 deletion src/components/colorbar/draw.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ var Drawing = require('../drawing');
var Color = require('../color');
var Titles = require('../titles');
var svgTextUtils = require('../../lib/svg_text_utils');
var LINE_SPACING = require('../../constants/alignment').LINE_SPACING;

var handleAxisDefaults = require('../../plots/cartesian/axis_defaults');
var handleAxisPositionDefaults = require('../../plots/cartesian/position_defaults');
Expand Down Expand Up @@ -297,7 +298,7 @@ module.exports = function draw(gd, id) {
lineSize = 15.6;
if(titleText.node()) {
lineSize =
parseInt(titleText.style('font-size'), 10) * 1.3;
parseInt(titleText.style('font-size'), 10) * LINE_SPACING;
}
if(mathJaxNode) {
titleHeight = Drawing.bBox(mathJaxNode).height;
Expand Down
8 changes: 5 additions & 3 deletions src/components/drawing/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ var Lib = require('../../lib');
var svgTextUtils = require('../../lib/svg_text_utils');

var xmlnsNamespaces = require('../../constants/xmlns_namespaces');
var alignment = require('../../constants/alignment');
var LINE_SPACING = alignment.LINE_SPACING;

var subTypes = require('../../traces/scatter/subtypes');
var makeBubbleSizeFn = require('../../traces/scatter/make_bubble_size_func');

Expand Down Expand Up @@ -426,8 +429,7 @@ drawing.tryColorscale = function(marker, prefix) {
};

// draw text at points
var TEXTOFFSETSIGN = {start: 1, end: -1, middle: 0, bottom: 1, top: -1},
LINEEXPAND = 1.3;
var TEXTOFFSETSIGN = {start: 1, end: -1, middle: 0, bottom: 1, top: -1};
drawing.textPointStyle = function(s, trace, gd) {
s.each(function(d) {
var p = d3.select(this),
Expand Down Expand Up @@ -462,7 +464,7 @@ drawing.textPointStyle = function(s, trace, gd) {
.call(svgTextUtils.convertToTspans, gd);

var pgroup = d3.select(this.parentNode);
var numLines = (svgTextUtils.lineCount(p) - 1) * LINEEXPAND + 1;
var numLines = (svgTextUtils.lineCount(p) - 1) * LINE_SPACING + 1;
var dx = TEXTOFFSETSIGN[h] * r;
var dy = fontSize * 0.75 + TEXTOFFSETSIGN[v] * r +
(TEXTOFFSETSIGN[v] - 1) * numLines * fontSize / 2;
Expand Down
4 changes: 3 additions & 1 deletion src/components/legend/draw.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ var svgTextUtils = require('../../lib/svg_text_utils');

var constants = require('./constants');
var interactConstants = require('../../constants/interactions');
var LINE_SPACING = require('../../constants/alignment').LINE_SPACING;

var getLegendData = require('./get_legend_data');
var style = require('./style');
var helpers = require('./helpers');
Expand Down Expand Up @@ -561,7 +563,7 @@ function computeTextDimensions(g, gd) {
var mathjaxGroup = g.select('g[class*=math-group]');
var mathjaxNode = mathjaxGroup.node();
var opts = gd._fullLayout.legend;
var lineHeight = opts.font.size * 1.3;
var lineHeight = opts.font.size * LINE_SPACING;
var height, width;

if(mathjaxNode) {
Expand Down
6 changes: 4 additions & 2 deletions src/components/rangeselector/draw.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ var svgTextUtils = require('../../lib/svg_text_utils');
var axisIds = require('../../plots/cartesian/axis_ids');
var anchorUtils = require('../legend/anchor_utils');

var LINE_SPACING = require('../../constants/alignment').LINE_SPACING;

var constants = require('./constants');
var getUpdateObject = require('./get_update_object');

Expand Down Expand Up @@ -183,7 +185,7 @@ function reposition(gd, buttons, opts, axName) {
var button = d3.select(this);
var text = button.select('.selector-text');

var tHeight = opts.font.size * 1.3;
var tHeight = opts.font.size * LINE_SPACING;
var hEff = Math.max(tHeight * svgTextUtils.lineCount(text), 16) + 3;

opts.height = Math.max(opts.height, hEff);
Expand All @@ -195,7 +197,7 @@ function reposition(gd, buttons, opts, axName) {
var text = button.select('.selector-text');

var tWidth = text.node() && Drawing.bBox(text.node()).width;
var tHeight = opts.font.size * 1.3;
var tHeight = opts.font.size * LINE_SPACING;
var tLines = svgTextUtils.lineCount(text);

var wEff = Math.max(tWidth + 10, constants.minButtonWidth);
Expand Down
3 changes: 0 additions & 3 deletions src/components/sliders/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,6 @@ module.exports = {
// padding around item text
textPadX: 40,

// font size to height scale
fontSizeToHeight: 1.3,

// arrow offset off right edge
arrowOffsetX: 4,

Expand Down
5 changes: 3 additions & 2 deletions src/components/sliders/draw.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ var svgTextUtils = require('../../lib/svg_text_utils');
var anchorUtils = require('../legend/anchor_utils');

var constants = require('./constants');
var LINE_SPACING = require('../../constants/alignment').LINE_SPACING;


module.exports = function draw(gd) {
Expand Down Expand Up @@ -309,7 +310,7 @@ function drawCurrentValue(sliderGroup, sliderOpts, valueOverride) {
var lines = svgTextUtils.lineCount(text);

var y0 = (sliderOpts.currentValueMaxLines + 1 - lines) *
sliderOpts.currentvalue.font.size * constants.fontSizeToHeight;
sliderOpts.currentvalue.font.size * LINE_SPACING;

svgTextUtils.positionText(text, x0, y0);

Expand Down Expand Up @@ -381,7 +382,7 @@ function drawLabelGroup(sliderGroup, sliderOpts) {
sliderOpts.ticklen +
// position is the baseline of the top line of text only, even
// if the label spans multiple lines
sliderOpts.font.size * constants.fontSizeToHeight +
sliderOpts.font.size * LINE_SPACING +
constants.labelOffset +
sliderOpts.currentValueTotalHeight
);
Expand Down
3 changes: 0 additions & 3 deletions src/components/updatemenus/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,6 @@ module.exports = {
textPadX: 24,
arrowPadX: 16,

// font size to height scale
fontSizeToHeight: 1.3,

// item rect radii
rx: 2,
ry: 2,
Expand Down
6 changes: 4 additions & 2 deletions src/components/updatemenus/draw.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ var Drawing = require('../drawing');
var svgTextUtils = require('../../lib/svg_text_utils');
var anchorUtils = require('../legend/anchor_utils');

var LINE_SPACING = require('../../constants/alignment').LINE_SPACING;

var constants = require('./constants');
var ScrollBox = require('./scrollbox');

Expand Down Expand Up @@ -530,7 +532,7 @@ function findDimensions(gd, menuOpts) {
var wEff = Math.max(tWidth + constants.textPadX, constants.minWidth);

// height is determined by item text
var tHeight = menuOpts.font.size * constants.fontSizeToHeight;
var tHeight = menuOpts.font.size * LINE_SPACING;
var tLines = svgTextUtils.lineCount(text);
var hEff = Math.max(tHeight * tLines, constants.minHeight) + constants.textOffsetY;

Expand Down Expand Up @@ -643,7 +645,7 @@ function setItemPosition(item, menuOpts, posOpts, overrideOpts) {
height: finalHeight
});

var tHeight = menuOpts.font.size * constants.fontSizeToHeight;
var tHeight = menuOpts.font.size * LINE_SPACING;
var tLines = svgTextUtils.lineCount(text);
var spanOffset = ((tLines - 1) * tHeight / 2);

Expand Down
4 changes: 3 additions & 1 deletion src/constants/alignment.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,7 @@ module.exports = {
bottom: 1,
middle: 0.5,
top: 0
}
},
// multiple of fontSize to get the vertical offset between lines
LINE_SPACING: 1.3
Copy link
Contributor

Choose a reason for hiding this comment

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

🎉 🎉 🎉 🎉 🎉

};
3 changes: 2 additions & 1 deletion src/lib/svg_text_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ var d3 = require('d3');
var Lib = require('../lib');
var xmlnsNamespaces = require('../constants/xmlns_namespaces');
var stringMappings = require('../constants/string_mappings');
var LINE_SPACING = require('../constants/alignment').LINE_SPACING;

// text converter

Expand Down Expand Up @@ -326,7 +327,7 @@ function buildSVGText(containerNode, str) {
var lineNode = document.createElementNS(xmlnsNamespaces.svg, 'tspan');
d3.select(lineNode).attr({
class: 'line',
dy: (currentLine * 1.3) + 'em'
dy: (currentLine * LINE_SPACING) + 'em'
});
containerNode.appendChild(lineNode);

Expand Down