From d3055645bc62f78d7eadabf2dfc88626602d1f34 Mon Sep 17 00:00:00 2001 From: etienne Date: Tue, 13 Mar 2018 16:24:24 -0400 Subject: [PATCH 01/16] perf makeSubplotData - replace Object.keys + for-loop with for-in - replace slow subplots.indexOf --- src/plots/cartesian/index.js | 31 ++++++++++++------------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/src/plots/cartesian/index.js b/src/plots/cartesian/index.js index 89a0e850f00..af386a9b406 100644 --- a/src/plots/cartesian/index.js +++ b/src/plots/cartesian/index.js @@ -347,29 +347,22 @@ exports.rangePlot = function(gd, plotinfo, cdSubplot) { }; function makeSubplotData(gd) { - var fullLayout = gd._fullLayout, - subplots = Object.keys(fullLayout._plots); - - var subplotData = [], - overlays = []; - - for(var i = 0; i < subplots.length; i++) { - var subplot = subplots[i], - plotinfo = fullLayout._plots[subplot]; - - var xa = plotinfo.xaxis; - var ya = plotinfo.yaxis; - var xa2 = xa._mainAxis; - var ya2 = ya._mainAxis; + var fullLayout = gd._fullLayout; + var subplotData = []; + var overlays = []; + for(var k in fullLayout._plots) { + var plotinfo = fullLayout._plots[k]; + var xa2 = plotinfo.xaxis._mainAxis; + var ya2 = plotinfo.yaxis._mainAxis; var mainplot = xa2._id + ya2._id; - if(mainplot !== subplot && subplots.indexOf(mainplot) !== -1) { + + if(mainplot !== k && fullLayout._plots[mainplot]) { plotinfo.mainplot = mainplot; plotinfo.mainplotinfo = fullLayout._plots[mainplot]; - overlays.push(subplot); - } - else { - subplotData.push(subplot); + overlays.push(k); + } else { + subplotData.push(k); } } From 7d881f6e3a8ab5f8ad3cb2024eaa7e0353bf78c7 Mon Sep 17 00:00:00 2001 From: etienne Date: Tue, 13 Mar 2018 16:26:51 -0400 Subject: [PATCH 02/16] replace selectAll().data([0]) with select() + .size() - to replace expensive querySelectorAll with plain querySelector --- src/plots/cartesian/index.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/plots/cartesian/index.js b/src/plots/cartesian/index.js index af386a9b406..17ad95b5126 100644 --- a/src/plots/cartesian/index.js +++ b/src/plots/cartesian/index.js @@ -303,8 +303,8 @@ exports.clean = function(newFullData, newFullLayout, oldFullData, oldFullLayout) }; exports.drawFramework = function(gd) { - var fullLayout = gd._fullLayout, - subplotData = makeSubplotData(gd); + var fullLayout = gd._fullLayout; + var subplotData = makeSubplotData(gd); var subplotLayers = fullLayout._cartesianlayer.selectAll('.subplot') .data(subplotData, Lib.identity); @@ -327,6 +327,7 @@ exports.drawFramework = function(gd) { plotinfo.overlays = []; makeSubplotLayer(plotinfo); + // fill in list of overlay subplots if(plotinfo.mainplot) { var mainplot = fullLayout._plots[plotinfo.mainplot]; @@ -506,12 +507,11 @@ function removeSubplotExtras(subplotId, fullLayout) { } function joinLayer(parent, nodeType, className, dataVal) { - var layer = parent.selectAll('.' + className) - .data([dataVal || 0]); - - layer.enter().append(nodeType) - .classed(className, true); - + var sel = parent.select('.' + className); + var layer = sel.size() ? + sel : + parent.append(nodeType).classed(className, true); + if(dataVal) layer.datum(dataVal); return layer; } From 5cc9af98bc9ecbfb246fafc0663269c7febe6c33 Mon Sep 17 00:00:00 2001 From: etienne Date: Wed, 14 Mar 2018 15:13:17 -0400 Subject: [PATCH 03/16] add ensureSingle & ensureSingleById lib methods - meant to replace all our `selectAll().data([0])` calls by something DRYer and faster. --- src/lib/index.js | 57 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/src/lib/index.js b/src/lib/index.js index 8ca656c82a2..d21cc0398a6 100644 --- a/src/lib/index.js +++ b/src/lib/index.js @@ -632,6 +632,63 @@ lib.isD3Selection = function(obj) { return obj && (typeof obj.classed === 'function'); }; +/** + * Append element to DOM only if not present. + * + * @param {d3 selection} parent : parent selection of the element in question + * @param {string} nodeType : node type of element to append + * @param {string} className : class name of element in question + * @param {fn} enterFn (optional) : optional fn applied to entering elements only + * @return {d3 selection} selection of new layer + * + * Previously, we were using the following pattern: + * + * ``` + * var sel = parent.selectAll('.' + className) + * .data([0]); + * + * sel.enter().append(nodeType) + * .classed(className, true); + * + * return sel; + * ``` + * + * in numerous places in our codebase to achieve the same behavior. + * + * The logic below performs much better, mostly as we are using + * `.select` instead `.selectAll` that is `querySelector` instead of + * `querySelectorAll`. + * + */ +lib.ensureSingle = function(parent, nodeType, className, enterFn) { + var sel = parent.select(nodeType + (className ? '.' + className : '')); + if(sel.size()) return sel; + + var layer = parent.append(nodeType).classed(className, true); + if(enterFn) layer.call(enterFn); + + return layer; +}; + +/** + * Same as Lib.ensureSingle, but using id as selector. + * This version is mostly used for clipPath nodes. + * + * @param {d3 selection} parent : parent selection of the element in question + * @param {string} nodeType : node type of element to append + * @param {string} id : id of element in question + * @param {fn} enterFn (optional) : optional fn applied to entering elements only + * @return {d3 selection} selection of new layer + */ +lib.ensureSingleById = function(parent, nodeType, id, enterFn) { + var sel = parent.select(nodeType + '#' + id); + if(sel.size()) return sel; + + var layer = parent.append(nodeType).attr('id', id); + if(enterFn) layer.call(enterFn); + + return layer; +}; /** * Converts a string path to an object. From e32434e1d85b4cd8304c6f016e5a2398552860ef Mon Sep 17 00:00:00 2001 From: etienne Date: Wed, 14 Mar 2018 15:17:10 -0400 Subject: [PATCH 04/16] replace selectAll().data([0]) with ensureSingle(ById) - note that one test suite had to be updated, simply because ensureSingle adds class names in a different order than previously. --- src/components/colorbar/draw.js | 9 +- src/components/drawing/index.js | 25 ++---- src/components/fx/hover.js | 20 ++--- src/components/legend/draw.js | 69 +++++---------- src/components/legend/style.js | 7 +- src/components/rangeselector/draw.js | 23 ++--- src/components/rangeslider/draw.js | 125 +++++++++------------------ src/components/sliders/draw.js | 73 ++++++---------- src/components/titles/index.js | 5 +- src/components/updatemenus/draw.js | 70 ++++++--------- src/plot_api/subroutines.js | 28 +++--- src/plots/cartesian/dragbox.js | 12 ++- src/plots/cartesian/index.js | 81 +++++++++-------- src/plots/plots.js | 15 ++-- src/plots/ternary/ternary.js | 21 ++--- src/traces/carpet/plot.js | 18 ++-- src/traces/contour/plot.js | 21 ++--- src/traces/contourcarpet/plot.js | 14 +-- src/traces/pie/plot.js | 9 +- test/jasmine/tests/click_test.js | 40 ++++----- 20 files changed, 256 insertions(+), 429 deletions(-) diff --git a/src/components/colorbar/draw.js b/src/components/colorbar/draw.js index 9ad1ac4017b..e580c0b8a7c 100644 --- a/src/components/colorbar/draw.js +++ b/src/components/colorbar/draw.js @@ -245,10 +245,9 @@ module.exports = function draw(gd, id) { cbAxisOut.setScale(); // now draw the elements - var container = fullLayout._infolayer.selectAll('g.' + id).data([0]); - container.enter().append('g').classed(id, true) - .classed(cn.colorbar, true) - .each(function() { + var container = Lib.ensureSingle(fullLayout._infolayer, 'g', id, function(s) { + s.classed(cn.colorbar, true); + s.each(function() { var s = d3.select(this); s.append('rect').classed(cn.cbbg, true); s.append('g').classed(cn.cbfills, true); @@ -259,6 +258,8 @@ module.exports = function draw(gd, id) { s.append('rect').classed(cn.cboutline, true); s.select('.cbtitle').datum(0); }); + }); + container.attr('transform', 'translate(' + Math.round(gs.l) + ',' + Math.round(gs.t) + ')'); // TODO: this opposite transform is a hack until we make it diff --git a/src/components/drawing/index.js b/src/components/drawing/index.js index 2f1baf0e55a..5146e656c63 100644 --- a/src/components/drawing/index.js +++ b/src/components/drawing/index.js @@ -415,9 +415,7 @@ drawing.gradient = function(sel, gd, gradientID, type, color1, color2) { * The upside of this is arbitrary points can share gradient defs */ drawing.initGradients = function(gd) { - var gradientsGroup = gd._fullLayout._defs.selectAll('.gradients').data([0]); - gradientsGroup.enter().append('g').classed('gradients', true); - + var gradientsGroup = Lib.ensureSingle(gd._fullLayout._defs, 'g', 'gradients'); gradientsGroup.selectAll('linearGradient,radialGradient').remove(); }; @@ -729,14 +727,9 @@ drawing.steps = function(shape) { // off-screen svg render testing element, shared by the whole page // uses the id 'js-plotly-tester' and stores it in drawing.tester drawing.makeTester = function() { - var tester = d3.select('body') - .selectAll('#js-plotly-tester') - .data([0]); - - tester.enter().append('svg') - .attr('id', 'js-plotly-tester') - .attr(xmlnsNamespaces.svgAttrs) - .style({ + var tester = Lib.ensureSingleById(d3.select('body'), 'svg', 'js-plotly-tester', function(s) { + s.attr(xmlnsNamespaces.svgAttrs); + s.style({ position: 'absolute', left: '-10000px', top: '-10000px', @@ -744,18 +737,18 @@ drawing.makeTester = function() { height: '9000px', 'z-index': '1' }); + }); // browsers differ on how they describe the bounding rect of // the svg if its contents spill over... so make a 1x1px // reference point we can measure off of. - var testref = tester.selectAll('.js-reference-point').data([0]); - testref.enter().append('path') - .classed('js-reference-point', true) - .attr('d', 'M0,0H1V1H0Z') - .style({ + var testref = Lib.ensureSingle(tester, 'path', 'js-reference-point', function(s) { + s.attr('d', 'M0,0H1V1H0Z'); + s.style({ 'stroke-width': 0, fill: 'black' }); + }); drawing.tester = tester; drawing.testref = testref; diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js index 32d7c09e077..c7db8a13d7f 100644 --- a/src/components/fx/hover.js +++ b/src/components/fx/hover.js @@ -696,23 +696,21 @@ function createHoverText(hoverData, opts, gd) { commonLabel.exit().remove(); commonLabel.each(function() { - var label = d3.select(this), - lpath = label.selectAll('path').data([0]), - ltext = label.selectAll('text').data([0]); - - lpath.enter().append('path') - .style({'stroke-width': '1px'}); + var label = d3.select(this); + var lpath = Lib.ensureSingle(label, 'path', '', function(s) { + s.style({'stroke-width': '1px'}); + }); + var ltext = Lib.ensureSingle(label, 'text', '', function(s) { + // prohibit tex interpretation until we can handle + // tex and regular text together + s.attr('data-notex', 1); + }); lpath.style({ fill: commonLabelOpts.bgcolor || Color.defaultLine, stroke: commonLabelOpts.bordercolor || Color.background, }); - ltext.enter().append('text') - // prohibit tex interpretation until we can handle - // tex and regular text together - .attr('data-notex', 1); - ltext.text(t0) .call(Drawing.font, commonLabelOpts.font.family || fontFamily, diff --git a/src/components/legend/draw.js b/src/components/legend/draw.js index e1a33b0eb6c..a6f1ab8872f 100644 --- a/src/components/legend/draw.js +++ b/src/components/legend/draw.js @@ -53,52 +53,35 @@ module.exports = function draw(gd) { return; } - var legend = fullLayout._infolayer.selectAll('g.legend') - .data([0]); - - legend.enter().append('g') - .attr({ - 'class': 'legend', - 'pointer-events': 'all' - }); - - var clipPath = fullLayout._topdefs.selectAll('#' + clipId) - .data([0]); - - clipPath.enter().append('clipPath') - .attr('id', clipId) - .append('rect'); + var firstRender = false; + var legend = Lib.ensureSingle(fullLayout._infolayer, 'g', 'legend', function(s) { + s.attr('pointer-events', 'all'); + firstRender = true; + }); - var bg = legend.selectAll('rect.bg') - .data([0]); + var clipPath = Lib.ensureSingleById(fullLayout._topdefs, 'clipPath', clipId, function(s) { + s.append('rect'); + }); - bg.enter().append('rect').attr({ - 'class': 'bg', - 'shape-rendering': 'crispEdges' + var bg = Lib.ensureSingle(legend, 'rect', 'bg', function(s) { + s.attr('shape-rendering', 'crispEdges'); }); bg.call(Color.stroke, opts.bordercolor) .call(Color.fill, opts.bgcolor) .style('stroke-width', opts.borderwidth + 'px'); - var scrollBox = legend.selectAll('g.scrollbox') - .data([0]); - - scrollBox.enter().append('g') - .attr('class', 'scrollbox'); - - var scrollBar = legend.selectAll('rect.scrollbar') - .data([0]); + var scrollBox = Lib.ensureSingle(legend, 'g', 'scrollbox'); - scrollBar.enter().append('rect') - .attr({ - 'class': 'scrollbar', + var scrollBar = Lib.ensureSingle(legend, 'rect', 'scrollbar', function(s) { + s.attr({ rx: 20, ry: 3, width: 0, height: 0 - }) - .call(Color.fill, '#808BA4'); + }); + s.call(Color.fill, '#808BA4'); + }); var groups = scrollBox.selectAll('g.groups') .data(legendData); @@ -129,7 +112,6 @@ module.exports = function draw(gd) { .call(setupTraceToggle, gd); }); - var firstRender = legend.enter().size() !== 0; if(firstRender) { computeLegendDimensions(gd, groups, traces); expandMargin(gd); @@ -378,10 +360,7 @@ function drawTexts(g, gd) { traceIndex = trace.index, name = isPie ? legendItem.label : trace.name; - var text = g.selectAll('text.legendtext') - .data([0]); - - text.enter().append('text').classed('legendtext', true); + var text = Lib.ensureSingle(g, 'text', 'legendtext'); text.attr('text-anchor', 'start') .classed('user-select-none', true) @@ -446,15 +425,11 @@ function setupTraceToggle(g, gd) { var newMouseDownTime, numClicks = 1; - var traceToggle = g.selectAll('rect') - .data([0]); - - traceToggle.enter().append('rect') - .classed('legendtoggle', true) - .style('cursor', 'pointer') - .attr('pointer-events', 'all') - .call(Color.fill, 'rgba(0,0,0,0)'); - + var traceToggle = Lib.ensureSingle(g, 'rect', 'legendtoggle', function(s) { + s.style('cursor', 'pointer'); + s.attr('pointer-events', 'all'); + s.call(Color.fill, 'rgba(0,0,0,0)'); + }); traceToggle.on('mousedown', function() { newMouseDownTime = (new Date()).getTime(); diff --git a/src/components/legend/style.js b/src/components/legend/style.js index 31f8661a882..b5a4878b9f7 100644 --- a/src/components/legend/style.js +++ b/src/components/legend/style.js @@ -6,7 +6,6 @@ * LICENSE file in the root directory of this source tree. */ - 'use strict'; var d3 = require('d3'); @@ -19,15 +18,11 @@ var Color = require('../color'); var subTypes = require('../../traces/scatter/subtypes'); var stylePie = require('../../traces/pie/style_one'); - module.exports = function style(s, gd) { s.each(function(d) { var traceGroup = d3.select(this); - var layers = traceGroup.selectAll('g.layers') - .data([0]); - layers.enter().append('g') - .classed('layers', true); + var layers = Lib.ensureSingle(traceGroup, 'g', 'layers'); layers.style('opacity', d[0].trace.opacity); var fill = layers diff --git a/src/components/rangeselector/draw.js b/src/components/rangeselector/draw.js index f729267eed8..5e270115b9e 100644 --- a/src/components/rangeselector/draw.js +++ b/src/components/rangeselector/draw.js @@ -14,6 +14,7 @@ var Registry = require('../../registry'); var Plots = require('../../plots/plots'); var Color = require('../color'); var Drawing = require('../drawing'); +var Lib = require('../../lib'); var svgTextUtils = require('../../lib/svg_text_utils'); var axisIds = require('../../plots/cartesian/axis_ids'); var anchorUtils = require('../legend/anchor_utils'); @@ -121,13 +122,9 @@ function isActive(axisLayout, opts, update) { } function drawButtonRect(button, selectorLayout, d) { - var rect = button.selectAll('rect') - .data([0]); - - rect.enter().append('rect') - .classed('selector-rect', true); - - rect.attr('shape-rendering', 'crispEdges'); + var rect = Lib.ensureSingle(button, 'rect', 'selector-rect', function(s) { + s.attr('shape-rendering', 'crispEdges'); + }); rect.attr({ 'rx': constants.rx, @@ -150,14 +147,10 @@ function drawButtonText(button, selectorLayout, d, gd) { svgTextUtils.convertToTspans(s, gd); } - var text = button.selectAll('text') - .data([0]); - - text.enter().append('text') - .classed('selector-text', true) - .classed('user-select-none', true); - - text.attr('text-anchor', 'middle'); + var text = Lib.ensureSingle(button, 'text', 'selector-text', function(s) { + s.classed('user-select-none', true); + s.attr('text-anchor', 'middle'); + }); text.call(Drawing.font, selectorLayout.font) .text(getLabel(d)) diff --git a/src/components/rangeslider/draw.js b/src/components/rangeslider/draw.js index 9fb61dfedd5..90d0e7ba62d 100644 --- a/src/components/rangeslider/draw.js +++ b/src/components/rangeslider/draw.js @@ -360,16 +360,13 @@ function setPixelRange(rangeSlider, gd, axisOpts, opts, oppAxisOpts, oppAxisRang } function drawBg(rangeSlider, gd, axisOpts, opts) { - var bg = rangeSlider.selectAll('rect.' + constants.bgClassName) - .data([0]); - - bg.enter().append('rect') - .classed(constants.bgClassName, true) - .attr({ + var bg = Lib.ensureSingle(rangeSlider, 'rect', constants.bgClassName, function(s) { + s.attr({ x: 0, y: 0, 'shape-rendering': 'crispEdges' }); + }); var borderCorrect = (opts.borderwidth % 2) === 0 ? opts.borderwidth : @@ -391,13 +388,9 @@ function drawBg(rangeSlider, gd, axisOpts, opts) { function addClipPath(rangeSlider, gd, axisOpts, opts) { var fullLayout = gd._fullLayout; - var clipPath = fullLayout._topdefs.selectAll('#' + opts._clipId) - .data([0]); - - clipPath.enter().append('clipPath') - .attr('id', opts._clipId) - .append('rect') - .attr({ x: 0, y: 0 }); + var clipPath = Lib.ensureSingleById(fullLayout._topdefs, 'clipPath', opts._clipId, function(s) { + s.append('rect').attr({ x: 0, y: 0 }); + }); clipPath.select('rect').attr({ width: opts._width, @@ -491,25 +484,19 @@ function filterRangePlotCalcData(calcData, subplotId) { } function drawMasks(rangeSlider, gd, axisOpts, opts, oppAxisRangeOpts) { - var maskMin = rangeSlider.selectAll('rect.' + constants.maskMinClassName) - .data([0]); - - maskMin.enter().append('rect') - .classed(constants.maskMinClassName, true) - .attr({ x: 0, y: 0 }) - .attr('shape-rendering', 'crispEdges'); + var maskMin = Lib.ensureSingle(rangeSlider, 'rect', constants.maskMinClassName, function(s) { + s.attr({ x: 0, y: 0 }); + s.attr('shape-rendering', 'crispEdges'); + }); maskMin .attr('height', opts._height) .call(Color.fill, constants.maskColor); - var maskMax = rangeSlider.selectAll('rect.' + constants.maskMaxClassName) - .data([0]); - - maskMax.enter().append('rect') - .classed(constants.maskMaxClassName, true) - .attr('y', 0) - .attr('shape-rendering', 'crispEdges'); + var maskMax = Lib.ensureSingle(rangeSlider, 'rect', constants.maskMaxClassName, function(s) { + s.attr('y', 0); + s.attr('shape-rendering', 'crispEdges'); + }); maskMax .attr('height', opts._height) @@ -517,25 +504,19 @@ function drawMasks(rangeSlider, gd, axisOpts, opts, oppAxisRangeOpts) { // masks used for oppAxis zoom if(oppAxisRangeOpts.rangemode !== 'match') { - var maskMinOppAxis = rangeSlider.selectAll('rect.' + constants.maskMinOppAxisClassName) - .data([0]); - - maskMinOppAxis.enter().append('rect') - .classed(constants.maskMinOppAxisClassName, true) - .attr('y', 0) - .attr('shape-rendering', 'crispEdges'); + var maskMinOppAxis = Lib.ensureSingle(rangeSlider, 'rect', constants.maskMinOppAxisClassName, function(s) { + s.attr('y', 0); + s.attr('shape-rendering', 'crispEdges'); + }); maskMinOppAxis .attr('width', opts._width) .call(Color.fill, constants.maskOppAxisColor); - var maskMaxOppAxis = rangeSlider.selectAll('rect.' + constants.maskMaxOppAxisClassName) - .data([0]); - - maskMaxOppAxis.enter().append('rect') - .classed(constants.maskMaxOppAxisClassName, true) - .attr('y', 0) - .attr('shape-rendering', 'crispEdges'); + var maskMaxOppAxis = Lib.ensureSingle(rangeSlider, 'rect', constants.maskMaxOppAxisClassName, function(s) { + s.attr('y', 0); + s.attr('shape-rendering', 'crispEdges'); + }); maskMaxOppAxis .attr('width', opts._width) @@ -547,14 +528,11 @@ function drawMasks(rangeSlider, gd, axisOpts, opts, oppAxisRangeOpts) { function drawSlideBox(rangeSlider, gd, axisOpts, opts) { if(gd._context.staticPlot) return; - var slideBox = rangeSlider.selectAll('rect.' + constants.slideBoxClassName) - .data([0]); - - slideBox.enter().append('rect') - .classed(constants.slideBoxClassName, true) - .attr('y', 0) - .attr('cursor', constants.slideBoxCursor) - .attr('shape-rendering', 'crispEdges'); + var slideBox = Lib.ensureSingle(rangeSlider, 'rect', constants.slideBoxClassName, function(s) { + s.attr('y', 0); + s.attr('cursor', constants.slideBoxCursor); + s.attr('shape-rendering', 'crispEdges'); + }); slideBox.attr({ height: opts._height, @@ -563,21 +541,11 @@ function drawSlideBox(rangeSlider, gd, axisOpts, opts) { } function drawGrabbers(rangeSlider, gd, axisOpts, opts) { - // - - var grabberMin = rangeSlider.selectAll('g.' + constants.grabberMinClassName) - .data([0]); - grabberMin.enter().append('g') - .classed(constants.grabberMinClassName, true); - - var grabberMax = rangeSlider.selectAll('g.' + constants.grabberMaxClassName) - .data([0]); - grabberMax.enter().append('g') - .classed(constants.grabberMaxClassName, true); + var grabberMin = Lib.ensureSingle(rangeSlider, 'g', constants.grabberMinClassName); + var grabberMax = Lib.ensureSingle(rangeSlider, 'g', constants.grabberMaxClassName); // - var handleFixAttrs = { x: 0, width: constants.handleWidth, @@ -587,28 +555,21 @@ function drawGrabbers(rangeSlider, gd, axisOpts, opts) { 'stroke-width': constants.handleStrokeWidth, 'shape-rendering': 'crispEdges' }; - var handleDynamicAttrs = { y: Math.round(opts._height / 4), height: Math.round(opts._height / 2), }; - - var handleMin = grabberMin.selectAll('rect.' + constants.handleMinClassName) - .data([0]); - handleMin.enter().append('rect') - .classed(constants.handleMinClassName, true) - .attr(handleFixAttrs); + var handleMin = Lib.ensureSingle(grabberMin, 'rect', constants.handleMinClassName, function(s) { + s.attr(handleFixAttrs); + }); handleMin.attr(handleDynamicAttrs); - var handleMax = grabberMax.selectAll('rect.' + constants.handleMaxClassName) - .data([0]); - handleMax.enter().append('rect') - .classed(constants.handleMaxClassName, true) - .attr(handleFixAttrs); + var handleMax = Lib.ensureSingle(grabberMax, 'rect', constants.handleMaxClassName, function(s) { + s.attr(handleFixAttrs); + }); handleMax.attr(handleDynamicAttrs); // - if(gd._context.staticPlot) return; var grabAreaFixAttrs = { @@ -619,18 +580,14 @@ function drawGrabbers(rangeSlider, gd, axisOpts, opts) { cursor: constants.grabAreaCursor }; - var grabAreaMin = grabberMin.selectAll('rect.' + constants.grabAreaMinClassName) - .data([0]); - grabAreaMin.enter().append('rect') - .classed(constants.grabAreaMinClassName, true) - .attr(grabAreaFixAttrs); + var grabAreaMin = Lib.ensureSingle(grabberMin, 'rect', constants.grabAreaMinClassName, function(s) { + s.attr(grabAreaFixAttrs); + }); grabAreaMin.attr('height', opts._height); - var grabAreaMax = grabberMax.selectAll('rect.' + constants.grabAreaMaxClassName) - .data([0]); - grabAreaMax.enter().append('rect') - .classed(constants.grabAreaMaxClassName, true) - .attr(grabAreaFixAttrs); + var grabAreaMax = Lib.ensureSingle(grabberMax, 'rect', constants.grabAreaMaxClassName, function(s) { + s.attr(grabAreaFixAttrs); + }); grabAreaMax.attr('height', opts._height); } diff --git a/src/components/sliders/draw.js b/src/components/sliders/draw.js index 1e7dff5caa4..adbe1dcd5d3 100644 --- a/src/components/sliders/draw.js +++ b/src/components/sliders/draw.js @@ -6,7 +6,6 @@ * LICENSE file in the root directory of this source tree. */ - 'use strict'; var d3 = require('d3'); @@ -14,6 +13,7 @@ var d3 = require('d3'); var Plots = require('../../plots/plots'); var Color = require('../color'); var Drawing = require('../drawing'); +var Lib = require('../../lib'); var svgTextUtils = require('../../lib/svg_text_utils'); var anchorUtils = require('../legend/anchor_utils'); @@ -23,7 +23,6 @@ var LINE_SPACING = alignmentConstants.LINE_SPACING; var FROM_TL = alignmentConstants.FROM_TL; var FROM_BR = alignmentConstants.FROM_BR; - module.exports = function draw(gd) { var fullLayout = gd._fullLayout, sliderData = makeSliderData(fullLayout, gd); @@ -264,10 +263,8 @@ function drawSlider(gd, sliderGroup, sliderOpts) { function drawCurrentValue(sliderGroup, sliderOpts, valueOverride) { if(!sliderOpts.currentvalue.visible) return; - var x0, textAnchor; - var text = sliderGroup.selectAll('text') - .data([0]); var dims = sliderOpts._dims; + var x0, textAnchor; switch(sliderOpts.currentvalue.xanchor) { case 'right': @@ -286,13 +283,13 @@ function drawCurrentValue(sliderGroup, sliderOpts, valueOverride) { textAnchor = 'left'; } - text.enter().append('text') - .classed(constants.labelClass, true) - .classed('user-select-none', true) - .attr({ + var text = Lib.ensureSingle(sliderGroup, 'text', constants.labelClass, function(s) { + s.classed('user-select-none', true); + s.attr({ 'text-anchor': textAnchor, 'data-notex': 1 }); + }); var str = sliderOpts.currentvalue.prefix ? sliderOpts.currentvalue.prefix : ''; @@ -322,13 +319,10 @@ function drawCurrentValue(sliderGroup, sliderOpts, valueOverride) { } function drawGrip(sliderGroup, gd, sliderOpts) { - var grip = sliderGroup.selectAll('rect.' + constants.gripRectClass) - .data([0]); - - grip.enter().append('rect') - .classed(constants.gripRectClass, true) - .call(attachGripEvents, gd, sliderGroup, sliderOpts) - .style('pointer-events', 'all'); + var grip = Lib.ensureSingle(sliderGroup, 'rect', constants.gripRectClass, function(s) { + s.call(attachGripEvents, gd, sliderGroup, sliderOpts); + s.style('pointer-events', 'all'); + }); grip.attr({ width: constants.gripWidth, @@ -336,22 +330,19 @@ function drawGrip(sliderGroup, gd, sliderOpts) { rx: constants.gripRadius, ry: constants.gripRadius, }) - .call(Color.stroke, sliderOpts.bordercolor) - .call(Color.fill, sliderOpts.bgcolor) - .style('stroke-width', sliderOpts.borderwidth + 'px'); + .call(Color.stroke, sliderOpts.bordercolor) + .call(Color.fill, sliderOpts.bgcolor) + .style('stroke-width', sliderOpts.borderwidth + 'px'); } function drawLabel(item, data, sliderOpts) { - var text = item.selectAll('text') - .data([0]); - - text.enter().append('text') - .classed(constants.labelClass, true) - .classed('user-select-none', true) - .attr({ + var text = Lib.ensureSingle(item, 'text', constants.labelClass, function(s) { + s.classed('user-select-none', true); + s.attr({ 'text-anchor': 'middle', 'data-notex': 1 }); + }); text.call(Drawing.font, sliderOpts.font) .text(data.step.label) @@ -361,13 +352,9 @@ function drawLabel(item, data, sliderOpts) { } function drawLabelGroup(sliderGroup, sliderOpts) { - var labels = sliderGroup.selectAll('g.' + constants.labelsClass) - .data([0]); + var labels = Lib.ensureSingle(sliderGroup, 'g', constants.labelsClass); var dims = sliderOpts._dims; - labels.enter().append('g') - .classed(constants.labelsClass, true); - var labelItems = labels.selectAll('g.' + constants.labelGroupClass) .data(dims.labelSteps); @@ -570,14 +557,11 @@ function positionToNormalizedValue(sliderOpts, position) { } function drawTouchRect(sliderGroup, gd, sliderOpts) { - var rect = sliderGroup.selectAll('rect.' + constants.railTouchRectClass) - .data([0]); var dims = sliderOpts._dims; - - rect.enter().append('rect') - .classed(constants.railTouchRectClass, true) - .call(attachGripEvents, gd, sliderGroup, sliderOpts) - .style('pointer-events', 'all'); + var rect = Lib.ensureSingle(sliderGroup, 'rect', constants.railTouchRectClass, function(s) { + s.call(attachGripEvents, gd, sliderGroup, sliderOpts); + s.style('pointer-events', 'all'); + }); rect.attr({ width: dims.inputAreaLength, @@ -590,14 +574,9 @@ function drawTouchRect(sliderGroup, gd, sliderOpts) { } function drawRail(sliderGroup, sliderOpts) { - var rect = sliderGroup.selectAll('rect.' + constants.railRectClass) - .data([0]); var dims = sliderOpts._dims; - - rect.enter().append('rect') - .classed(constants.railRectClass, true); - var computedLength = dims.inputAreaLength - constants.railInset * 2; + var rect = Lib.ensureSingle(sliderGroup, 'rect', constants.railRectClass); rect.attr({ width: computedLength, @@ -606,9 +585,9 @@ function drawRail(sliderGroup, sliderOpts) { ry: constants.railRadius, 'shape-rendering': 'crispEdges' }) - .call(Color.stroke, sliderOpts.bordercolor) - .call(Color.fill, sliderOpts.bgcolor) - .style('stroke-width', sliderOpts.borderwidth + 'px'); + .call(Color.stroke, sliderOpts.bordercolor) + .call(Color.fill, sliderOpts.bgcolor) + .style('stroke-width', sliderOpts.borderwidth + 'px'); Drawing.setTranslate(rect, constants.railInset, diff --git a/src/components/titles/index.js b/src/components/titles/index.js index b30bdc7f547..e2211dd88df 100644 --- a/src/components/titles/index.js +++ b/src/components/titles/index.js @@ -98,10 +98,7 @@ function draw(gd, titleClass, options) { var elShouldExist = txt || editable; if(!group) { - group = fullLayout._infolayer.selectAll('.g-' + titleClass) - .data([0]); - group.enter().append('g') - .classed('g-' + titleClass, true); + group = Lib.ensureSingle(fullLayout._infolayer, 'g', 'g-' + titleClass); } var el = group.selectAll('text') diff --git a/src/components/updatemenus/draw.js b/src/components/updatemenus/draw.js index ea916cef2f6..5852222daad 100644 --- a/src/components/updatemenus/draw.js +++ b/src/components/updatemenus/draw.js @@ -14,6 +14,7 @@ var d3 = require('d3'); var Plots = require('../../plots/plots'); var Color = require('../color'); var Drawing = require('../drawing'); +var Lib = require('../../lib'); var svgTextUtils = require('../../lib/svg_text_utils'); var anchorUtils = require('../legend/anchor_utils'); @@ -78,12 +79,9 @@ module.exports = function draw(gd) { .classed(constants.headerGroupClassName, true); // draw dropdown button container - var gButton = menus.selectAll('g.' + constants.dropdownButtonGroupClassName) - .data([0]); - - gButton.enter().append('g') - .classed(constants.dropdownButtonGroupClassName, true) - .style('pointer-events', 'all'); + var gButton = Lib.ensureSingle(menus, 'g', constants.dropdownButtonGroupClassName, function(s) { + s.style('pointer-events', 'all'); + }); // find dimensions before plotting anything (this mutates menuOpts) for(var i = 0; i < menuData.length; i++) { @@ -190,36 +188,30 @@ function setActive(gd, menuOpts, buttonOpts, gHeader, gButton, scrollBox, button } function drawHeader(gd, gHeader, gButton, scrollBox, menuOpts) { - var header = gHeader.selectAll('g.' + constants.headerClassName) - .data([0]); - var dims = menuOpts._dims; - - header.enter().append('g') - .classed(constants.headerClassName, true) - .style('pointer-events', 'all'); + var header = Lib.ensureSingle(gHeader, 'g', constants.headerClassName, function(s) { + s.style('pointer-events', 'all'); + }); - var active = menuOpts.active, - headerOpts = menuOpts.buttons[active] || constants.blankHeaderOpts, - posOpts = { y: menuOpts.pad.t, yPad: 0, x: menuOpts.pad.l, xPad: 0, index: 0 }, - positionOverrides = { - width: dims.headerWidth, - height: dims.headerHeight - }; + var dims = menuOpts._dims; + var active = menuOpts.active; + var headerOpts = menuOpts.buttons[active] || constants.blankHeaderOpts; + var posOpts = { y: menuOpts.pad.t, yPad: 0, x: menuOpts.pad.l, xPad: 0, index: 0 }; + var positionOverrides = { + width: dims.headerWidth, + height: dims.headerHeight + }; header .call(drawItem, menuOpts, headerOpts, gd) .call(setItemPosition, menuOpts, posOpts, positionOverrides); // draw drop arrow at the right edge - var arrow = gHeader.selectAll('text.' + constants.headerArrowClassName) - .data([0]); - - arrow.enter().append('text') - .classed(constants.headerArrowClassName, true) - .classed('user-select-none', true) - .attr('text-anchor', 'end') - .call(Drawing.font, menuOpts.font) - .text(constants.arrowSymbol[menuOpts.direction]); + var arrow = Lib.ensureSingle(gHeader, 'text', constants.headerArrowClassName, function(s) { + s.classed('user-select-none', true) + .attr('text-anchor', 'end') + .call(Drawing.font, menuOpts.font) + .text(constants.arrowSymbol[menuOpts.direction]); + }); arrow.attr({ x: dims.headerWidth - constants.arrowOffsetX + menuOpts.pad.l, @@ -447,16 +439,13 @@ function drawItem(item, menuOpts, itemOpts, gd) { } function drawItemRect(item, menuOpts) { - var rect = item.selectAll('rect') - .data([0]); - - rect.enter().append('rect') - .classed(constants.itemRectClassName, true) - .attr({ + var rect = Lib.ensureSingle(item, 'rect', constants.itemRectClassName, function(s) { + s.attr({ rx: constants.rx, ry: constants.ry, 'shape-rendering': 'crispEdges' }); + }); rect.call(Color.stroke, menuOpts.bordercolor) .call(Color.fill, menuOpts.bgcolor) @@ -464,16 +453,13 @@ function drawItemRect(item, menuOpts) { } function drawItemText(item, menuOpts, itemOpts, gd) { - var text = item.selectAll('text') - .data([0]); - - text.enter().append('text') - .classed(constants.itemTextClassName, true) - .classed('user-select-none', true) - .attr({ + var text = Lib.ensureSingle(item, 'text', constants.itemTextClassName, function(s) { + s.classed('user-select-none', true); + s.attr({ 'text-anchor': 'start', 'data-notex': 1 }); + }); text.call(Drawing.font, menuOpts.font) .text(itemOpts.label) diff --git a/src/plot_api/subroutines.js b/src/plot_api/subroutines.js index c92faea1f8f..128f02d81a2 100644 --- a/src/plot_api/subroutines.js +++ b/src/plot_api/subroutines.js @@ -180,23 +180,17 @@ exports.lsInner = function(gd) { } // Clip so that data only shows up on the plot area. - plotinfo.clipId = 'clip' + fullLayout._uid + subplot + 'plot'; + var clipId = plotinfo.clipId = 'clip' + fullLayout._uid + subplot + 'plot'; - var plotClip = fullLayout._clips.selectAll('#' + plotinfo.clipId) - .data([0]); - - plotClip.enter().append('clipPath') - .attr({ - 'class': 'plotclip', - 'id': plotinfo.clipId - }) - .append('rect'); + var plotClip = Lib.ensureSingleById(fullLayout._clips, 'clipPath', clipId, function(s) { + s.classed('plotclip', true); + s.append('rect'); + }); - plotClip.selectAll('rect') - .attr({ - 'width': xa._length, - 'height': ya._length - }); + plotClip.select('rect').attr({ + width: xa._length, + height: ya._length + }); Drawing.setTranslate(plotinfo.plot, xa._offset, ya._offset); @@ -205,9 +199,9 @@ exports.lsInner = function(gd) { if(plotinfo._hasClipOnAxisFalse) { plotClipId = null; - layerClipId = plotinfo.clipId; + layerClipId = clipId; } else { - plotClipId = plotinfo.clipId; + plotClipId = clipId; layerClipId = null; } diff --git a/src/plots/cartesian/dragbox.js b/src/plots/cartesian/dragbox.js index 6b36f98c82d..2153c8de1df 100644 --- a/src/plots/cartesian/dragbox.js +++ b/src/plots/cartesian/dragbox.js @@ -802,13 +802,11 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) { } function makeDragger(plotinfo, nodeName, dragClass, cursor) { - var dragger3 = plotinfo.draglayer.selectAll('.' + dragClass).data([0]); - - dragger3.enter().append(nodeName) - .classed('drag', true) - .classed(dragClass, true) - .style({fill: 'transparent', 'stroke-width': 0}) - .attr('data-subplot', plotinfo.id); + var dragger3 = Lib.ensureSingle(plotinfo.draglayer, nodeName, dragClass, function(s) { + s.classed('drag', true) + .style({fill: 'transparent', 'stroke-width': 0}) + .attr('data-subplot', plotinfo.id); + }); dragger3.call(setCursor, cursor); diff --git a/src/plots/cartesian/index.js b/src/plots/cartesian/index.js index 17ad95b5126..07af2b3654b 100644 --- a/src/plots/cartesian/index.js +++ b/src/plots/cartesian/index.js @@ -18,6 +18,14 @@ var axisIds = require('./axis_ids'); var constants = require('./constants'); var xmlnsNamespaces = require('../../constants/xmlns_namespaces'); +var ensureSingle = Lib.ensureSingle; + +function ensureSingleAndAddDatum(parent, nodeType, className) { + return Lib.ensureSingle(parent, nodeType, className, function(s) { + s.datum(className); + }); +} + exports.name = 'cartesian'; exports.attr = ['xaxis', 'yaxis']; @@ -337,7 +345,7 @@ exports.drawFramework = function(gd) { // make separate drag layers for each subplot, // but append them to paper rather than the plot groups, // so they end up on top of the rest - plotinfo.draglayer = joinLayer(fullLayout._draggers, 'g', name); + plotinfo.draglayer = ensureSingle(fullLayout._draggers, 'g', name); }); }; @@ -380,32 +388,32 @@ function makeSubplotLayer(plotinfo) { var yLayer = constants.layerValue2layerClass[plotinfo.yaxis.layer]; if(!plotinfo.mainplot) { - var backLayer = joinLayer(plotgroup, 'g', 'layer-subplot'); - plotinfo.shapelayer = joinLayer(backLayer, 'g', 'shapelayer'); - plotinfo.imagelayer = joinLayer(backLayer, 'g', 'imagelayer'); + var backLayer = ensureSingle(plotgroup, 'g', 'layer-subplot'); + plotinfo.shapelayer = ensureSingle(backLayer, 'g', 'shapelayer'); + plotinfo.imagelayer = ensureSingle(backLayer, 'g', 'imagelayer'); - plotinfo.gridlayer = joinLayer(plotgroup, 'g', 'gridlayer'); + plotinfo.gridlayer = ensureSingle(plotgroup, 'g', 'gridlayer'); - plotinfo.zerolinelayer = joinLayer(plotgroup, 'g', 'zerolinelayer'); + plotinfo.zerolinelayer = ensureSingle(plotgroup, 'g', 'zerolinelayer'); - joinLayer(plotgroup, 'path', 'xlines-below'); - joinLayer(plotgroup, 'path', 'ylines-below'); - plotinfo.overlinesBelow = joinLayer(plotgroup, 'g', 'overlines-below'); + ensureSingle(plotgroup, 'path', 'xlines-below'); + ensureSingle(plotgroup, 'path', 'ylines-below'); + plotinfo.overlinesBelow = ensureSingle(plotgroup, 'g', 'overlines-below'); - joinLayer(plotgroup, 'g', 'xaxislayer-below'); - joinLayer(plotgroup, 'g', 'yaxislayer-below'); - plotinfo.overaxesBelow = joinLayer(plotgroup, 'g', 'overaxes-below'); + ensureSingle(plotgroup, 'g', 'xaxislayer-below'); + ensureSingle(plotgroup, 'g', 'yaxislayer-below'); + plotinfo.overaxesBelow = ensureSingle(plotgroup, 'g', 'overaxes-below'); - plotinfo.plot = joinLayer(plotgroup, 'g', 'plot'); - plotinfo.overplot = joinLayer(plotgroup, 'g', 'overplot'); + plotinfo.plot = ensureSingle(plotgroup, 'g', 'plot'); + plotinfo.overplot = ensureSingle(plotgroup, 'g', 'overplot'); - joinLayer(plotgroup, 'path', 'xlines-above'); - joinLayer(plotgroup, 'path', 'ylines-above'); - plotinfo.overlinesAbove = joinLayer(plotgroup, 'g', 'overlines-above'); + ensureSingle(plotgroup, 'path', 'xlines-above'); + ensureSingle(plotgroup, 'path', 'ylines-above'); + plotinfo.overlinesAbove = ensureSingle(plotgroup, 'g', 'overlines-above'); - joinLayer(plotgroup, 'g', 'xaxislayer-above'); - joinLayer(plotgroup, 'g', 'yaxislayer-above'); - plotinfo.overaxesAbove = joinLayer(plotgroup, 'g', 'overaxes-above'); + ensureSingle(plotgroup, 'g', 'xaxislayer-above'); + ensureSingle(plotgroup, 'g', 'yaxislayer-above'); + plotinfo.overaxesAbove = ensureSingle(plotgroup, 'g', 'overaxes-above'); // set refs to correct layers as determined by 'axis.layer' plotinfo.xlines = plotgroup.select('.xlines-' + xLayer); @@ -427,17 +435,17 @@ function makeSubplotLayer(plotinfo) { plotinfo.gridlayer = mainplotinfo.gridlayer; plotinfo.zerolinelayer = mainplotinfo.zerolinelayer; - joinLayer(mainplotinfo.overlinesBelow, 'path', xId); - joinLayer(mainplotinfo.overlinesBelow, 'path', yId); - joinLayer(mainplotinfo.overaxesBelow, 'g', xId); - joinLayer(mainplotinfo.overaxesBelow, 'g', yId); + ensureSingle(mainplotinfo.overlinesBelow, 'path', xId); + ensureSingle(mainplotinfo.overlinesBelow, 'path', yId); + ensureSingle(mainplotinfo.overaxesBelow, 'g', xId); + ensureSingle(mainplotinfo.overaxesBelow, 'g', yId); - plotinfo.plot = joinLayer(mainplotinfo.overplot, 'g', id); + plotinfo.plot = ensureSingle(mainplotinfo.overplot, 'g', id); - joinLayer(mainplotinfo.overlinesAbove, 'path', xId); - joinLayer(mainplotinfo.overlinesAbove, 'path', yId); - joinLayer(mainplotinfo.overaxesAbove, 'g', xId); - joinLayer(mainplotinfo.overaxesAbove, 'g', yId); + ensureSingle(mainplotinfo.overlinesAbove, 'path', xId); + ensureSingle(mainplotinfo.overlinesAbove, 'path', yId); + ensureSingle(mainplotinfo.overaxesAbove, 'g', xId); + ensureSingle(mainplotinfo.overaxesAbove, 'g', yId); // set refs to correct layers as determined by 'abovetraces' plotinfo.xlines = mainplotgroup.select('.overlines-' + xLayer).select('.' + xId); @@ -446,14 +454,14 @@ function makeSubplotLayer(plotinfo) { plotinfo.yaxislayer = mainplotgroup.select('.overaxes-' + yLayer).select('.' + yId); } - joinLayer(plotinfo.gridlayer, 'g', plotinfo.xaxis._id, plotinfo.xaxis._id); - joinLayer(plotinfo.gridlayer, 'g', plotinfo.yaxis._id, plotinfo.yaxis._id); + ensureSingleAndAddDatum(plotinfo.gridlayer, 'g', plotinfo.xaxis._id); + ensureSingleAndAddDatum(plotinfo.gridlayer, 'g', plotinfo.yaxis._id); plotinfo.gridlayer.selectAll('g').sort(axisIds.idSort); // common attributes for all subplots, overlays or not for(var i = 0; i < constants.traceLayerClasses.length; i++) { - joinLayer(plotinfo.plot, 'g', constants.traceLayerClasses[i]); + ensureSingle(plotinfo.plot, 'g', constants.traceLayerClasses[i]); } plotinfo.xlines @@ -506,15 +514,6 @@ function removeSubplotExtras(subplotId, fullLayout) { fullLayout._defs.select('#clip' + fullLayout._uid + subplotId + 'plot').remove(); } -function joinLayer(parent, nodeType, className, dataVal) { - var sel = parent.select('.' + className); - var layer = sel.size() ? - sel : - parent.append(nodeType).classed(className, true); - if(dataVal) layer.datum(dataVal); - return layer; -} - exports.toSVG = function(gd) { var imageRoot = gd._fullLayout._glimages; var root = d3.select(gd).selectAll('.svg-container'); diff --git a/src/plots/plots.js b/src/plots/plots.js index d567aa09371..08d311e6792 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -132,12 +132,8 @@ plots.addLinks = function(gd) { var fullLayout = gd._fullLayout; - var linkContainer = fullLayout._paper - .selectAll('text.js-plot-link-container').data([0]); - - linkContainer.enter().append('text') - .classed('js-plot-link-container', true) - .style({ + var linkContainer = Lib.ensureSingle(fullLayout._paper, 'text', 'js-plot-link-container', function(s) { + s.style({ 'font-family': '"Open Sans", Arial, sans-serif', 'font-size': '12px', 'fill': Color.defaultLine, @@ -149,12 +145,11 @@ plots.addLinks = function(gd) { links.append('tspan').classed('js-link-spacer', true); links.append('tspan').classed('js-sourcelinks', true); }); + }); // The text node inside svg - var text = linkContainer.node(), - attrs = { - y: fullLayout._paper.attr('height') - 9 - }; + var text = linkContainer.node(); + var attrs = {y: fullLayout._paper.attr('height') - 9}; // If text's width is bigger than the layout // Check that text is a child node or document.body diff --git a/src/plots/ternary/ternary.js b/src/plots/ternary/ternary.js index d5501223230..ae6ef1cca4d 100644 --- a/src/plots/ternary/ternary.js +++ b/src/plots/ternary/ternary.js @@ -70,27 +70,20 @@ proto.plot = function(ternaryCalcData, fullLayout) { proto.makeFramework = function(fullLayout) { var _this = this; var ternaryLayout = fullLayout[_this.id]; + var clipId = _this.clipId = 'clip' + _this.layoutId + _this.id; + var clipIdRelative = _this.clipIdRelative = 'clip-relative' + _this.layoutId + _this.id; // clippath for this ternary subplot - _this.clipDef = fullLayout._clips.selectAll('#' + clipId) - .data([0]); - _this.clipDef.enter().append('clipPath').attr('id', clipId) - .append('path').attr('d', 'M0,0Z'); + _this.clipDef = Lib.ensureSingleById(fullLayout._clips, 'clipPath', clipId); + _this.clipDef.append('path').attr('d', 'M0,0Z'); // 'relative' clippath (i.e. no translation) for this ternary subplot - var clipIdRelative = _this.clipIdRelative = 'clip-relative' + _this.layoutId + _this.id; - _this.clipDefRelative = fullLayout._clips.selectAll('#' + clipIdRelative) - .data([0]); - _this.clipDefRelative.enter().append('clipPath').attr('id', clipIdRelative) - .append('path').attr('d', 'M0,0Z'); + _this.clipDefRelative = Lib.ensureSingleById(fullLayout._clips, 'clipPath', clipIdRelative); + _this.clipDefRelative.append('path').attr('d', 'M0,0Z'); // container for everything in this ternary subplot - _this.plotContainer = _this.container.selectAll('g.' + _this.id) - .data([0]); - _this.plotContainer.enter().append('g') - .classed(_this.id, true); - + _this.plotContainer = Lib.ensureSingle(_this.container, 'g', _this.id); _this.updateLayers(ternaryLayout); Drawing.setClipUrl(_this.layers.backplot, clipId); diff --git a/src/traces/carpet/plot.js b/src/traces/carpet/plot.js index eb7b957985e..ce728562e93 100644 --- a/src/traces/carpet/plot.js +++ b/src/traces/carpet/plot.js @@ -24,12 +24,6 @@ module.exports = function plot(gd, plotinfo, cdcarpet) { } }; -function makeg(el, type, klass) { - var join = el.selectAll(type + '.' + klass).data([0]); - join.enter().append(type).classed(klass, true); - return join; -} - function plotOne(gd, plotinfo, cd) { var t = cd[0]; var trace = cd[0].trace, @@ -42,11 +36,11 @@ function plotOne(gd, plotinfo, cd) { var gridLayer = plotinfo.plot.selectAll('.carpetlayer'); var clipLayer = fullLayout._clips; - var axisLayer = makeg(gridLayer, 'g', 'carpet' + trace.uid).classed('trace', true); - var minorLayer = makeg(axisLayer, 'g', 'minorlayer'); - var majorLayer = makeg(axisLayer, 'g', 'majorlayer'); - var boundaryLayer = makeg(axisLayer, 'g', 'boundarylayer'); - var labelLayer = makeg(axisLayer, 'g', 'labellayer'); + var axisLayer = Lib.ensureSingle(gridLayer, 'g', 'carpet' + trace.uid).classed('trace', true); + var minorLayer = Lib.ensureSingle(axisLayer, 'g', 'minorlayer'); + var majorLayer = Lib.ensureSingle(axisLayer, 'g', 'majorlayer'); + var boundaryLayer = Lib.ensureSingle(axisLayer, 'g', 'boundarylayer'); + var labelLayer = Lib.ensureSingle(axisLayer, 'g', 'labellayer'); axisLayer.style('opacity', trace.opacity); @@ -78,7 +72,7 @@ function drawClipPath(trace, t, layer, xaxis, yaxis) { .classed('carpetclip', true); } - var path = makeg(clip, 'path', 'carpetboundary'); + var path = Lib.ensureSingle(clip, 'path', 'carpetboundary'); var segments = t.clipsegments; var segs = []; diff --git a/src/traces/contour/plot.js b/src/traces/contour/plot.js index be126d43e59..325a54c83c7 100644 --- a/src/traces/contour/plot.js +++ b/src/traces/contour/plot.js @@ -112,8 +112,7 @@ exports.makeContourGroup = function(plotinfo, cd, id) { }; function makeBackground(plotgroup, perimeter, contours) { - var bggroup = plotgroup.selectAll('g.contourbg').data([0]); - bggroup.enter().append('g').classed('contourbg', true); + var bggroup = Lib.ensureSingle(plotgroup, 'g', 'contourbg'); var bgfill = bggroup.selectAll('path') .data(contours.coloring === 'fill' ? [0] : []); @@ -125,10 +124,7 @@ function makeBackground(plotgroup, perimeter, contours) { } function makeFills(plotgroup, pathinfo, perimeter, contours) { - var fillgroup = plotgroup.selectAll('g.contourfill') - .data([0]); - fillgroup.enter().append('g') - .classed('contourfill', true); + var fillgroup = Lib.ensureSingle(plotgroup, 'g', 'contourfill'); var fillitems = fillgroup.selectAll('path') .data(contours.coloring === 'fill' || (contours.type === 'constraint' && contours._operation !== '=') ? pathinfo : []); @@ -252,11 +248,7 @@ function joinAllPaths(pi, perimeter) { } function makeLinesAndLabels(plotgroup, pathinfo, gd, cd0, contours, perimeter) { - var lineContainer = plotgroup.selectAll('g.contourlines').data([0]); - - lineContainer.enter().append('g') - .classed('contourlines', true); - + var lineContainer = Lib.ensureSingle(plotgroup, 'g', 'contourlines'); var showLines = contours.showlines !== false; var showLabels = contours.showlabels; var clipLinesForLabels = showLines && showLabels; @@ -626,8 +618,7 @@ exports.drawLabels = function(labelGroup, labelData, gd, lineClip, labelClipPath clipPath += 'M' + labelClipPathData[i].join('L') + 'Z'; } - var lineClipPath = lineClip.selectAll('path').data([0]); - lineClipPath.enter().append('path'); + var lineClipPath = Lib.ensureSingle(lineClip, 'path', ''); lineClipPath.attr('d', clipPath); } }; @@ -666,9 +657,7 @@ function clipGaps(plotGroup, plotinfo, clips, cd0, perimeter) { findAllPaths([clipPathInfo]); var fullpath = joinAllPaths(clipPathInfo, perimeter); - var path = clipPath.selectAll('path') - .data([0]); - path.enter().append('path'); + var path = Lib.ensureSingle(clipPath, 'path', ''); path.attr('d', fullpath); } else clipId = null; diff --git a/src/traces/contourcarpet/plot.js b/src/traces/contourcarpet/plot.js index 8b5974e49dd..b14b4b4db96 100644 --- a/src/traces/contourcarpet/plot.js +++ b/src/traces/contourcarpet/plot.js @@ -129,11 +129,7 @@ function plotOne(gd, plotinfo, cd) { } function makeLinesAndLabels(plotgroup, pathinfo, gd, cd0, contours, plotinfo, carpet) { - var lineContainer = plotgroup.selectAll('g.contourlines').data([0]); - - lineContainer.enter().append('g') - .classed('contourlines', true); - + var lineContainer = Lib.ensureSingle(plotgroup, 'g', 'contourlines'); var showLines = contours.showlines !== false; var showLabels = contours.showlabels; var clipLinesForLabels = showLines && showLabels; @@ -298,8 +294,7 @@ function vectorTan(v0, v1) { function makeBackground(plotgroup, clipsegments, xaxis, yaxis, isConstraint, coloring) { var seg, xp, yp, i; - var bggroup = plotgroup.selectAll('g.contourbg').data([0]); - bggroup.enter().append('g').classed('contourbg', true); + var bggroup = Lib.ensureSingle(plotgroup, 'g', 'contourbg'); var bgfill = bggroup.selectAll('path') .data((coloring === 'fill' && !isConstraint) ? [0] : []); @@ -320,10 +315,7 @@ function makeBackground(plotgroup, clipsegments, xaxis, yaxis, isConstraint, col } function makeFills(trace, plotgroup, xa, ya, pathinfo, perimeter, ab2p, carpet, carpetcd, coloring, boundaryPath) { - var fillgroup = plotgroup.selectAll('g.contourfill') - .data([0]); - fillgroup.enter().append('g') - .classed('contourfill', true); + var fillgroup = Lib.ensureSingle(plotgroup, 'g', 'contourfill'); var fillitems = fillgroup.selectAll('path') .data(coloring === 'fill' ? pathinfo : []); diff --git a/src/traces/pie/plot.js b/src/traces/pie/plot.js index dc755080378..a1efb11946d 100644 --- a/src/traces/pie/plot.js +++ b/src/traces/pie/plot.js @@ -13,6 +13,7 @@ var d3 = require('d3'); var Fx = require('../../components/fx'); var Color = require('../../components/color'); var Drawing = require('../../components/drawing'); +var Lib = require('../../lib'); var svgTextUtils = require('../../lib/svg_text_utils'); var helpers = require('./helpers'); @@ -256,13 +257,11 @@ module.exports = function plot(gd, cdpie) { sliceTextGroup.exit().remove(); sliceTextGroup.each(function() { - var sliceText = d3.select(this).selectAll('text').data([0]); - - sliceText.enter().append('text') + var sliceText = Lib.ensureSingle(d3.select(this), 'text', '', function(s) { // prohibit tex interpretation until we can handle // tex and regular text together - .attr('data-notex', 1); - sliceText.exit().remove(); + s.attr('data-notex', 1); + }); sliceText.text(pt.text) .attr({ diff --git a/test/jasmine/tests/click_test.js b/test/jasmine/tests/click_test.js index 5d22ee67951..a7f2e206f76 100644 --- a/test/jasmine/tests/click_test.js +++ b/test/jasmine/tests/click_test.js @@ -418,8 +418,8 @@ describe('Test click interactions:', function() { var node = document.querySelector('rect.nwdrag'); var pos = getRectCenter(node); - expect(node.classList[0]).toBe('drag'); - expect(node.classList[1]).toBe('nwdrag'); + expect(node.classList[1]).toBe('drag'); + expect(node.classList[0]).toBe('nwdrag'); expect(node.classList[2]).toBe('cursor-nw-resize'); expect(gd.layout.xaxis.range).toBeCloseToArray(autoRangeX); @@ -442,8 +442,8 @@ describe('Test click interactions:', function() { var node = document.querySelector('rect.nedrag'); var pos = getRectCenter(node); - expect(node.classList[0]).toBe('drag'); - expect(node.classList[1]).toBe('nedrag'); + expect(node.classList[1]).toBe('drag'); + expect(node.classList[0]).toBe('nedrag'); expect(node.classList[2]).toBe('cursor-ne-resize'); expect(gd.layout.xaxis.range).toBeCloseToArray(autoRangeX); @@ -466,8 +466,8 @@ describe('Test click interactions:', function() { var node = document.querySelector('rect.swdrag'); var pos = getRectCenter(node); - expect(node.classList[0]).toBe('drag'); - expect(node.classList[1]).toBe('swdrag'); + expect(node.classList[1]).toBe('drag'); + expect(node.classList[0]).toBe('swdrag'); expect(node.classList[2]).toBe('cursor-sw-resize'); expect(gd.layout.xaxis.range).toBeCloseToArray(autoRangeX); @@ -490,8 +490,8 @@ describe('Test click interactions:', function() { var node = document.querySelector('rect.sedrag'); var pos = getRectCenter(node); - expect(node.classList[0]).toBe('drag'); - expect(node.classList[1]).toBe('sedrag'); + expect(node.classList[1]).toBe('drag'); + expect(node.classList[0]).toBe('sedrag'); expect(node.classList[2]).toBe('cursor-se-resize'); expect(gd.layout.xaxis.range).toBeCloseToArray(autoRangeX); @@ -514,8 +514,8 @@ describe('Test click interactions:', function() { var node = document.querySelector('rect.ewdrag'); var pos = getRectCenter(node); - expect(node.classList[0]).toBe('drag'); - expect(node.classList[1]).toBe('ewdrag'); + expect(node.classList[1]).toBe('drag'); + expect(node.classList[0]).toBe('ewdrag'); expect(node.classList[2]).toBe('cursor-ew-resize'); expect(gd.layout.xaxis.range).toBeCloseToArray(autoRangeX); @@ -538,8 +538,8 @@ describe('Test click interactions:', function() { var node = document.querySelector('rect.wdrag'); var pos = getRectCenter(node); - expect(node.classList[0]).toBe('drag'); - expect(node.classList[1]).toBe('wdrag'); + expect(node.classList[1]).toBe('drag'); + expect(node.classList[0]).toBe('wdrag'); expect(node.classList[2]).toBe('cursor-w-resize'); expect(gd.layout.xaxis.range).toBeCloseToArray(autoRangeX); @@ -562,8 +562,8 @@ describe('Test click interactions:', function() { var node = document.querySelector('rect.edrag'); var pos = getRectCenter(node); - expect(node.classList[0]).toBe('drag'); - expect(node.classList[1]).toBe('edrag'); + expect(node.classList[1]).toBe('drag'); + expect(node.classList[0]).toBe('edrag'); expect(node.classList[2]).toBe('cursor-e-resize'); expect(gd.layout.xaxis.range).toBeCloseToArray(autoRangeX); @@ -586,8 +586,8 @@ describe('Test click interactions:', function() { var node = document.querySelector('rect.nsdrag'); var pos = getRectCenter(node); - expect(node.classList[0]).toBe('drag'); - expect(node.classList[1]).toBe('nsdrag'); + expect(node.classList[1]).toBe('drag'); + expect(node.classList[0]).toBe('nsdrag'); expect(node.classList[2]).toBe('cursor-ns-resize'); expect(gd.layout.xaxis.range).toBeCloseToArray(autoRangeX); @@ -610,8 +610,8 @@ describe('Test click interactions:', function() { var node = document.querySelector('rect.sdrag'); var pos = getRectCenter(node); - expect(node.classList[0]).toBe('drag'); - expect(node.classList[1]).toBe('sdrag'); + expect(node.classList[1]).toBe('drag'); + expect(node.classList[0]).toBe('sdrag'); expect(node.classList[2]).toBe('cursor-s-resize'); expect(gd.layout.xaxis.range).toBeCloseToArray(autoRangeX); @@ -634,8 +634,8 @@ describe('Test click interactions:', function() { var node = document.querySelector('rect.ndrag'); var pos = getRectCenter(node); - expect(node.classList[0]).toBe('drag'); - expect(node.classList[1]).toBe('ndrag'); + expect(node.classList[1]).toBe('drag'); + expect(node.classList[0]).toBe('ndrag'); expect(node.classList[2]).toBe('cursor-n-resize'); expect(gd.layout.xaxis.range).toBeCloseToArray(autoRangeX); From e810c1ee55900132caa009b5f96e1644d272634b Mon Sep 17 00:00:00 2001 From: etienne Date: Wed, 14 Mar 2018 16:05:29 -0400 Subject: [PATCH 05/16] :hocho: layoutStyles call in drawFramework step - this appear to be no longer needed - adapt plot_api test to relied on layoutStyles spy counts --- src/plot_api/plot_api.js | 4 +--- test/jasmine/tests/plot_api_test.js | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index 51d5bf5c514..1805321221e 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -245,9 +245,7 @@ exports.plot = function(gd, data, layout, config) { .attr('height', fullLayout.height); } - return Lib.syncOrAsync([ - subroutines.layoutStyles - ], gd); + return Plots.previousPromises(gd); } // draw anything that can affect margins. diff --git a/test/jasmine/tests/plot_api_test.js b/test/jasmine/tests/plot_api_test.js index 8af0b1e3820..1924111d1a0 100644 --- a/test/jasmine/tests/plot_api_test.js +++ b/test/jasmine/tests/plot_api_test.js @@ -2440,9 +2440,7 @@ describe('Test plot api', function() { function countCalls(counts) { var callsFinal = Lib.extendFlat({}, counts); - // TODO: do we really need to do layoutStyles twice in Plotly.plot? - // We get one from drawFramework and another directly from Plotly.plot. - callsFinal.layoutStyles = (counts.layoutStyles || 0) + 2 * (counts.plot || 0); + callsFinal.layoutStyles = (counts.layoutStyles || 0) + (counts.plot || 0); mockedMethods.forEach(function(m) { expect(subroutines[m]).toHaveBeenCalledTimes(callsFinal[m] || 0); From 8e6032d0d221d650f646878172780472540fc16c Mon Sep 17 00:00:00 2001 From: etienne Date: Wed, 14 Mar 2018 17:04:47 -0400 Subject: [PATCH 06/16] bring back d3-esque method chains ... even though don't look bad to etpinard's :eyes: --- src/components/colorbar/draw.js | 24 ++++++++++---------- src/components/drawing/index.js | 28 +++++++++++------------ src/components/legend/draw.js | 10 ++++----- src/components/rangeselector/draw.js | 4 ++-- src/components/rangeslider/draw.js | 33 ++++++++++++++++++---------- src/components/sliders/draw.js | 28 +++++++++++------------ src/components/updatemenus/draw.js | 10 ++++----- src/plot_api/subroutines.js | 4 ++-- src/plots/ternary/ternary.js | 10 +++++---- 9 files changed, 82 insertions(+), 69 deletions(-) diff --git a/src/components/colorbar/draw.js b/src/components/colorbar/draw.js index e580c0b8a7c..f879e3661ad 100644 --- a/src/components/colorbar/draw.js +++ b/src/components/colorbar/draw.js @@ -246,18 +246,18 @@ module.exports = function draw(gd, id) { // now draw the elements var container = Lib.ensureSingle(fullLayout._infolayer, 'g', id, function(s) { - s.classed(cn.colorbar, true); - s.each(function() { - var s = d3.select(this); - s.append('rect').classed(cn.cbbg, true); - s.append('g').classed(cn.cbfills, true); - s.append('g').classed(cn.cblines, true); - s.append('g').classed(cn.cbaxis, true).classed(cn.crisp, true); - s.append('g').classed(cn.cbtitleunshift, true) - .append('g').classed(cn.cbtitle, true); - s.append('rect').classed(cn.cboutline, true); - s.select('.cbtitle').datum(0); - }); + s.classed(cn.colorbar, true) + .each(function() { + var s = d3.select(this); + s.append('rect').classed(cn.cbbg, true); + s.append('g').classed(cn.cbfills, true); + s.append('g').classed(cn.cblines, true); + s.append('g').classed(cn.cbaxis, true).classed(cn.crisp, true); + s.append('g').classed(cn.cbtitleunshift, true) + .append('g').classed(cn.cbtitle, true); + s.append('rect').classed(cn.cboutline, true); + s.select('.cbtitle').datum(0); + }); }); container.attr('transform', 'translate(' + Math.round(gs.l) + diff --git a/src/components/drawing/index.js b/src/components/drawing/index.js index 5146e656c63..50f5d4e2ddf 100644 --- a/src/components/drawing/index.js +++ b/src/components/drawing/index.js @@ -728,26 +728,26 @@ drawing.steps = function(shape) { // uses the id 'js-plotly-tester' and stores it in drawing.tester drawing.makeTester = function() { var tester = Lib.ensureSingleById(d3.select('body'), 'svg', 'js-plotly-tester', function(s) { - s.attr(xmlnsNamespaces.svgAttrs); - s.style({ - position: 'absolute', - left: '-10000px', - top: '-10000px', - width: '9000px', - height: '9000px', - 'z-index': '1' - }); + s.attr(xmlnsNamespaces.svgAttrs) + .style({ + position: 'absolute', + left: '-10000px', + top: '-10000px', + width: '9000px', + height: '9000px', + 'z-index': '1' + }); }); // browsers differ on how they describe the bounding rect of // the svg if its contents spill over... so make a 1x1px // reference point we can measure off of. var testref = Lib.ensureSingle(tester, 'path', 'js-reference-point', function(s) { - s.attr('d', 'M0,0H1V1H0Z'); - s.style({ - 'stroke-width': 0, - fill: 'black' - }); + s.attr('d', 'M0,0H1V1H0Z') + .style({ + 'stroke-width': 0, + fill: 'black' + }); }); drawing.tester = tester; diff --git a/src/components/legend/draw.js b/src/components/legend/draw.js index a6f1ab8872f..98c1e766652 100644 --- a/src/components/legend/draw.js +++ b/src/components/legend/draw.js @@ -79,8 +79,8 @@ module.exports = function draw(gd) { ry: 3, width: 0, height: 0 - }); - s.call(Color.fill, '#808BA4'); + }) + .call(Color.fill, '#808BA4'); }); var groups = scrollBox.selectAll('g.groups') @@ -426,9 +426,9 @@ function setupTraceToggle(g, gd) { numClicks = 1; var traceToggle = Lib.ensureSingle(g, 'rect', 'legendtoggle', function(s) { - s.style('cursor', 'pointer'); - s.attr('pointer-events', 'all'); - s.call(Color.fill, 'rgba(0,0,0,0)'); + s.style('cursor', 'pointer') + .attr('pointer-events', 'all') + .call(Color.fill, 'rgba(0,0,0,0)'); }); traceToggle.on('mousedown', function() { diff --git a/src/components/rangeselector/draw.js b/src/components/rangeselector/draw.js index 5e270115b9e..b21f73f6a08 100644 --- a/src/components/rangeselector/draw.js +++ b/src/components/rangeselector/draw.js @@ -148,8 +148,8 @@ function drawButtonText(button, selectorLayout, d, gd) { } var text = Lib.ensureSingle(button, 'text', 'selector-text', function(s) { - s.classed('user-select-none', true); - s.attr('text-anchor', 'middle'); + s.classed('user-select-none', true) + .attr('text-anchor', 'middle'); }); text.call(Drawing.font, selectorLayout.font) diff --git a/src/components/rangeslider/draw.js b/src/components/rangeslider/draw.js index 90d0e7ba62d..8aff445e6a0 100644 --- a/src/components/rangeslider/draw.js +++ b/src/components/rangeslider/draw.js @@ -485,8 +485,11 @@ function filterRangePlotCalcData(calcData, subplotId) { function drawMasks(rangeSlider, gd, axisOpts, opts, oppAxisRangeOpts) { var maskMin = Lib.ensureSingle(rangeSlider, 'rect', constants.maskMinClassName, function(s) { - s.attr({ x: 0, y: 0 }); - s.attr('shape-rendering', 'crispEdges'); + s.attr({ + x: 0, + y: 0, + 'shape-rendering': 'crispEdges' + }); }); maskMin @@ -494,8 +497,10 @@ function drawMasks(rangeSlider, gd, axisOpts, opts, oppAxisRangeOpts) { .call(Color.fill, constants.maskColor); var maskMax = Lib.ensureSingle(rangeSlider, 'rect', constants.maskMaxClassName, function(s) { - s.attr('y', 0); - s.attr('shape-rendering', 'crispEdges'); + s.attr({ + y: 0, + 'shape-rendering': 'crispEdges' + }); }); maskMax @@ -505,8 +510,10 @@ function drawMasks(rangeSlider, gd, axisOpts, opts, oppAxisRangeOpts) { // masks used for oppAxis zoom if(oppAxisRangeOpts.rangemode !== 'match') { var maskMinOppAxis = Lib.ensureSingle(rangeSlider, 'rect', constants.maskMinOppAxisClassName, function(s) { - s.attr('y', 0); - s.attr('shape-rendering', 'crispEdges'); + s.attr({ + y: 0, + 'shape-rendering': 'crispEdges' + }); }); maskMinOppAxis @@ -514,8 +521,10 @@ function drawMasks(rangeSlider, gd, axisOpts, opts, oppAxisRangeOpts) { .call(Color.fill, constants.maskOppAxisColor); var maskMaxOppAxis = Lib.ensureSingle(rangeSlider, 'rect', constants.maskMaxOppAxisClassName, function(s) { - s.attr('y', 0); - s.attr('shape-rendering', 'crispEdges'); + s.attr({ + y: 0, + 'shape-rendering': 'crispEdges' + }); }); maskMaxOppAxis @@ -529,9 +538,11 @@ function drawSlideBox(rangeSlider, gd, axisOpts, opts) { if(gd._context.staticPlot) return; var slideBox = Lib.ensureSingle(rangeSlider, 'rect', constants.slideBoxClassName, function(s) { - s.attr('y', 0); - s.attr('cursor', constants.slideBoxCursor); - s.attr('shape-rendering', 'crispEdges'); + s.attr({ + y: 0, + cursor: constants.slideBoxCursor, + 'shape-rendering': 'crispEdges' + }); }); slideBox.attr({ diff --git a/src/components/sliders/draw.js b/src/components/sliders/draw.js index adbe1dcd5d3..15d60bacbe5 100644 --- a/src/components/sliders/draw.js +++ b/src/components/sliders/draw.js @@ -284,11 +284,11 @@ function drawCurrentValue(sliderGroup, sliderOpts, valueOverride) { } var text = Lib.ensureSingle(sliderGroup, 'text', constants.labelClass, function(s) { - s.classed('user-select-none', true); - s.attr({ - 'text-anchor': textAnchor, - 'data-notex': 1 - }); + s.classed('user-select-none', true) + .attr({ + 'text-anchor': textAnchor, + 'data-notex': 1 + }); }); var str = sliderOpts.currentvalue.prefix ? sliderOpts.currentvalue.prefix : ''; @@ -320,8 +320,8 @@ function drawCurrentValue(sliderGroup, sliderOpts, valueOverride) { function drawGrip(sliderGroup, gd, sliderOpts) { var grip = Lib.ensureSingle(sliderGroup, 'rect', constants.gripRectClass, function(s) { - s.call(attachGripEvents, gd, sliderGroup, sliderOpts); - s.style('pointer-events', 'all'); + s.call(attachGripEvents, gd, sliderGroup, sliderOpts) + .style('pointer-events', 'all'); }); grip.attr({ @@ -337,11 +337,11 @@ function drawGrip(sliderGroup, gd, sliderOpts) { function drawLabel(item, data, sliderOpts) { var text = Lib.ensureSingle(item, 'text', constants.labelClass, function(s) { - s.classed('user-select-none', true); - s.attr({ - 'text-anchor': 'middle', - 'data-notex': 1 - }); + s.classed('user-select-none', true) + .attr({ + 'text-anchor': 'middle', + 'data-notex': 1 + }); }); text.call(Drawing.font, sliderOpts.font) @@ -559,8 +559,8 @@ function positionToNormalizedValue(sliderOpts, position) { function drawTouchRect(sliderGroup, gd, sliderOpts) { var dims = sliderOpts._dims; var rect = Lib.ensureSingle(sliderGroup, 'rect', constants.railTouchRectClass, function(s) { - s.call(attachGripEvents, gd, sliderGroup, sliderOpts); - s.style('pointer-events', 'all'); + s.call(attachGripEvents, gd, sliderGroup, sliderOpts) + .style('pointer-events', 'all'); }); rect.attr({ diff --git a/src/components/updatemenus/draw.js b/src/components/updatemenus/draw.js index 5852222daad..faee1578a6e 100644 --- a/src/components/updatemenus/draw.js +++ b/src/components/updatemenus/draw.js @@ -454,11 +454,11 @@ function drawItemRect(item, menuOpts) { function drawItemText(item, menuOpts, itemOpts, gd) { var text = Lib.ensureSingle(item, 'text', constants.itemTextClassName, function(s) { - s.classed('user-select-none', true); - s.attr({ - 'text-anchor': 'start', - 'data-notex': 1 - }); + s.classed('user-select-none', true) + .attr({ + 'text-anchor': 'start', + 'data-notex': 1 + }); }); text.call(Drawing.font, menuOpts.font) diff --git a/src/plot_api/subroutines.js b/src/plot_api/subroutines.js index 128f02d81a2..8a883cf5d89 100644 --- a/src/plot_api/subroutines.js +++ b/src/plot_api/subroutines.js @@ -183,8 +183,8 @@ exports.lsInner = function(gd) { var clipId = plotinfo.clipId = 'clip' + fullLayout._uid + subplot + 'plot'; var plotClip = Lib.ensureSingleById(fullLayout._clips, 'clipPath', clipId, function(s) { - s.classed('plotclip', true); - s.append('rect'); + s.classed('plotclip', true) + .append('rect'); }); plotClip.select('rect').attr({ diff --git a/src/plots/ternary/ternary.js b/src/plots/ternary/ternary.js index ae6ef1cca4d..e115361d351 100644 --- a/src/plots/ternary/ternary.js +++ b/src/plots/ternary/ternary.js @@ -75,12 +75,14 @@ proto.makeFramework = function(fullLayout) { var clipIdRelative = _this.clipIdRelative = 'clip-relative' + _this.layoutId + _this.id; // clippath for this ternary subplot - _this.clipDef = Lib.ensureSingleById(fullLayout._clips, 'clipPath', clipId); - _this.clipDef.append('path').attr('d', 'M0,0Z'); + _this.clipDef = Lib.ensureSingleById(fullLayout._clips, 'clipPath', clipId, function(s) { + s.append('path').attr('d', 'M0,0Z'); + }); // 'relative' clippath (i.e. no translation) for this ternary subplot - _this.clipDefRelative = Lib.ensureSingleById(fullLayout._clips, 'clipPath', clipIdRelative); - _this.clipDefRelative.append('path').attr('d', 'M0,0Z'); + _this.clipDefRelative = Lib.ensureSingleById(fullLayout._clips, 'clipPath', clipIdRelative, function(s) { + s.clipDefRelative.append('path').attr('d', 'M0,0Z'); + }); // container for everything in this ternary subplot _this.plotContainer = Lib.ensureSingle(_this.container, 'g', _this.id); From b974eab43b204a5ff3c897697d78d700641ba7da Mon Sep 17 00:00:00 2001 From: etienne Date: Wed, 14 Mar 2018 17:05:52 -0400 Subject: [PATCH 07/16] add toBeClassed custom matcher - to easily test all items in class list while not having to worry about class name ordering. --- test/jasmine/assets/custom_matchers.js | 25 ++++++++++++++++ test/jasmine/tests/cartesian_test.js | 15 ++++------ test/jasmine/tests/click_test.js | 40 +++++++------------------- 3 files changed, 40 insertions(+), 40 deletions(-) diff --git a/test/jasmine/assets/custom_matchers.js b/test/jasmine/assets/custom_matchers.js index b29f7be4f88..23deb14ef4e 100644 --- a/test/jasmine/assets/custom_matchers.js +++ b/test/jasmine/assets/custom_matchers.js @@ -149,6 +149,31 @@ var matchers = { msgExtra ].join(' '); + return { + pass: passed, + message: message + }; + } + }; + }, + + toBeClassed: function() { + return { + compare: function(node, _expected, msgExtra) { + var actual = node.classList; + var expected = Array.isArray(_expected) ? _expected : [_expected]; + + var passed = ( + actual.length === expected.length && + expected.every(function(e) { return actual.contains(e); }) + ); + + var message = [ + 'Expected classList', '[' + actual + ']', + 'to have classes', expected, + msgExtra + ].join(' '); + return { pass: passed, message: message diff --git a/test/jasmine/tests/cartesian_test.js b/test/jasmine/tests/cartesian_test.js index ab307a2e697..15c6c896f67 100644 --- a/test/jasmine/tests/cartesian_test.js +++ b/test/jasmine/tests/cartesian_test.js @@ -28,10 +28,8 @@ describe('restyle', function() { // First is tozero, second is tonext: expect(d3.selectAll('g.trace.scatter .js-fill').size()).toEqual(2); - expect(fills[0].classList.contains('js-tozero')).toBe(true); - expect(fills[0].classList.contains('js-tonext')).toBe(false); - expect(fills[1].classList.contains('js-tozero')).toBe(false); - expect(fills[1].classList.contains('js-tonext')).toBe(true); + expect(fills[0]).toBeClassed(['js-fill', 'js-tozero']); + expect(fills[1]).toBeClassed(['js-fill', 'js-tonext']); firstToZero = fills[0]; firstToNext = fills[1]; @@ -40,8 +38,7 @@ describe('restyle', function() { }).then(function() { // Trace 1 hidden leaves only trace zero's tozero fill: expect(d3.selectAll('g.trace.scatter .js-fill').size()).toEqual(1); - expect(fills[0].classList.contains('js-tozero')).toBe(true); - expect(fills[0].classList.contains('js-tonext')).toBe(false); + expect(fills[0]).toBeClassed(['js-fill', 'js-tozero']); }).then(function() { return Plotly.restyle(gd, {visible: [true]}, [1]); }).then(function() { @@ -50,10 +47,8 @@ describe('restyle', function() { // First is tozero, second is tonext: expect(d3.selectAll('g.trace.scatter .js-fill').size()).toEqual(2); - expect(fills[0].classList.contains('js-tozero')).toBe(true); - expect(fills[0].classList.contains('js-tonext')).toBe(false); - expect(fills[1].classList.contains('js-tozero')).toBe(false); - expect(fills[1].classList.contains('js-tonext')).toBe(true); + expect(fills[0]).toBeClassed(['js-fill', 'js-tozero']); + expect(fills[1]).toBeClassed(['js-fill', 'js-tonext']); secondToZero = fills[0]; secondToNext = fills[1]; diff --git a/test/jasmine/tests/click_test.js b/test/jasmine/tests/click_test.js index a7f2e206f76..2f65fe50668 100644 --- a/test/jasmine/tests/click_test.js +++ b/test/jasmine/tests/click_test.js @@ -418,9 +418,7 @@ describe('Test click interactions:', function() { var node = document.querySelector('rect.nwdrag'); var pos = getRectCenter(node); - expect(node.classList[1]).toBe('drag'); - expect(node.classList[0]).toBe('nwdrag'); - expect(node.classList[2]).toBe('cursor-nw-resize'); + expect(node).toBeClassed(['drag', 'nwdrag', 'cursor-nw-resize']); expect(gd.layout.xaxis.range).toBeCloseToArray(autoRangeX); expect(gd.layout.yaxis.range).toBeCloseToArray(autoRangeY); @@ -442,9 +440,7 @@ describe('Test click interactions:', function() { var node = document.querySelector('rect.nedrag'); var pos = getRectCenter(node); - expect(node.classList[1]).toBe('drag'); - expect(node.classList[0]).toBe('nedrag'); - expect(node.classList[2]).toBe('cursor-ne-resize'); + expect(node).toBeClassed(['drag', 'nedrag', 'cursor-ne-resize']); expect(gd.layout.xaxis.range).toBeCloseToArray(autoRangeX); expect(gd.layout.yaxis.range).toBeCloseToArray(autoRangeY); @@ -466,9 +462,7 @@ describe('Test click interactions:', function() { var node = document.querySelector('rect.swdrag'); var pos = getRectCenter(node); - expect(node.classList[1]).toBe('drag'); - expect(node.classList[0]).toBe('swdrag'); - expect(node.classList[2]).toBe('cursor-sw-resize'); + expect(node).toBeClassed(['drag', 'swdrag', 'cursor-sw-resize']); expect(gd.layout.xaxis.range).toBeCloseToArray(autoRangeX); expect(gd.layout.yaxis.range).toBeCloseToArray(autoRangeY); @@ -490,9 +484,7 @@ describe('Test click interactions:', function() { var node = document.querySelector('rect.sedrag'); var pos = getRectCenter(node); - expect(node.classList[1]).toBe('drag'); - expect(node.classList[0]).toBe('sedrag'); - expect(node.classList[2]).toBe('cursor-se-resize'); + expect(node).toBeClassed(['drag', 'sedrag', 'cursor-se-resize']); expect(gd.layout.xaxis.range).toBeCloseToArray(autoRangeX); expect(gd.layout.yaxis.range).toBeCloseToArray(autoRangeY); @@ -514,9 +506,7 @@ describe('Test click interactions:', function() { var node = document.querySelector('rect.ewdrag'); var pos = getRectCenter(node); - expect(node.classList[1]).toBe('drag'); - expect(node.classList[0]).toBe('ewdrag'); - expect(node.classList[2]).toBe('cursor-ew-resize'); + expect(node).toBeClassed(['drag', 'ewdrag', 'cursor-ew-resize']); expect(gd.layout.xaxis.range).toBeCloseToArray(autoRangeX); expect(gd.layout.yaxis.range).toBeCloseToArray(autoRangeY); @@ -538,9 +528,7 @@ describe('Test click interactions:', function() { var node = document.querySelector('rect.wdrag'); var pos = getRectCenter(node); - expect(node.classList[1]).toBe('drag'); - expect(node.classList[0]).toBe('wdrag'); - expect(node.classList[2]).toBe('cursor-w-resize'); + expect(node).toBeClassed(['drag', 'wdrag', 'cursor-w-resize']); expect(gd.layout.xaxis.range).toBeCloseToArray(autoRangeX); expect(gd.layout.yaxis.range).toBeCloseToArray(autoRangeY); @@ -562,9 +550,7 @@ describe('Test click interactions:', function() { var node = document.querySelector('rect.edrag'); var pos = getRectCenter(node); - expect(node.classList[1]).toBe('drag'); - expect(node.classList[0]).toBe('edrag'); - expect(node.classList[2]).toBe('cursor-e-resize'); + expect(node).toBeClassed(['drag', 'edrag', 'cursor-e-resize']); expect(gd.layout.xaxis.range).toBeCloseToArray(autoRangeX); expect(gd.layout.yaxis.range).toBeCloseToArray(autoRangeY); @@ -586,9 +572,7 @@ describe('Test click interactions:', function() { var node = document.querySelector('rect.nsdrag'); var pos = getRectCenter(node); - expect(node.classList[1]).toBe('drag'); - expect(node.classList[0]).toBe('nsdrag'); - expect(node.classList[2]).toBe('cursor-ns-resize'); + expect(node).toBeClassed(['drag', 'nsdrag', 'cursor-ns-resize']); expect(gd.layout.xaxis.range).toBeCloseToArray(autoRangeX); expect(gd.layout.yaxis.range).toBeCloseToArray(autoRangeY); @@ -610,9 +594,7 @@ describe('Test click interactions:', function() { var node = document.querySelector('rect.sdrag'); var pos = getRectCenter(node); - expect(node.classList[1]).toBe('drag'); - expect(node.classList[0]).toBe('sdrag'); - expect(node.classList[2]).toBe('cursor-s-resize'); + expect(node).toBeClassed(['drag', 'sdrag', 'cursor-s-resize']); expect(gd.layout.xaxis.range).toBeCloseToArray(autoRangeX); expect(gd.layout.yaxis.range).toBeCloseToArray(autoRangeY); @@ -634,9 +616,7 @@ describe('Test click interactions:', function() { var node = document.querySelector('rect.ndrag'); var pos = getRectCenter(node); - expect(node.classList[1]).toBe('drag'); - expect(node.classList[0]).toBe('ndrag'); - expect(node.classList[2]).toBe('cursor-n-resize'); + expect(node).toBeClassed(['drag', 'ndrag', 'cursor-n-resize']); expect(gd.layout.xaxis.range).toBeCloseToArray(autoRangeX); expect(gd.layout.yaxis.range).toBeCloseToArray(autoRangeY); From fc8c3868a1d3e26eab653cab491e103324ab2f20 Mon Sep 17 00:00:00 2001 From: etienne Date: Wed, 14 Mar 2018 17:24:40 -0400 Subject: [PATCH 08/16] fixup typo --- src/plots/ternary/ternary.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plots/ternary/ternary.js b/src/plots/ternary/ternary.js index e115361d351..a20e706db8d 100644 --- a/src/plots/ternary/ternary.js +++ b/src/plots/ternary/ternary.js @@ -81,7 +81,7 @@ proto.makeFramework = function(fullLayout) { // 'relative' clippath (i.e. no translation) for this ternary subplot _this.clipDefRelative = Lib.ensureSingleById(fullLayout._clips, 'clipPath', clipIdRelative, function(s) { - s.clipDefRelative.append('path').attr('d', 'M0,0Z'); + s.append('path').attr('d', 'M0,0Z'); }); // container for everything in this ternary subplot From 5887104139256934bbf554bf62685fbec62585d2 Mon Sep 17 00:00:00 2001 From: etienne Date: Wed, 14 Mar 2018 21:49:30 -0400 Subject: [PATCH 09/16] stash base url on Drawing module object ... so that we don't have to traverse the DOM whenever we add a clipPath url :racehorse: --- src/components/drawing/index.js | 20 +++++++++++++------- test/jasmine/tests/drawing_test.js | 4 +++- test/jasmine/tests/plot_interact_test.js | 6 ++++++ 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/components/drawing/index.js b/src/components/drawing/index.js index 50f5d4e2ddf..2b1c733df98 100644 --- a/src/components/drawing/index.js +++ b/src/components/drawing/index.js @@ -910,15 +910,21 @@ drawing.setClipUrl = function(s, localId) { return; } - var url = '#' + localId, - base = d3.select('base'); - - // add id to location href w/o hashes if any) - if(base.size() && base.attr('href')) { - url = window.location.href.split('#')[0] + url; + if(drawing.baseUrl === undefined) { + var base = d3.select('base'); + + // Stash base url once and for all! + // We may have to stash this elsewhere when + // we'll try to support for child windows + // more info -> https://github.com/plotly/plotly.js/issues/702 + if(base.size() && base.attr('href')) { + drawing.baseUrl = window.location.href.split('#')[0]; + } else { + drawing.baseUrl = ''; + } } - s.attr('clip-path', 'url(' + url + ')'); + s.attr('clip-path', 'url(' + drawing.baseUrl + '#' + localId + ')'); }; drawing.getTranslate = function(element) { diff --git a/test/jasmine/tests/drawing_test.js b/test/jasmine/tests/drawing_test.js index 5b5049857f5..d62d2fdea73 100644 --- a/test/jasmine/tests/drawing_test.js +++ b/test/jasmine/tests/drawing_test.js @@ -19,6 +19,9 @@ describe('Drawing', function() { afterEach(function() { this.svg.remove(); this.g.remove(); + + // unstash base url from Drawing module object + delete Drawing.baseUrl; }); it('should set the clip-path attribute', function() { @@ -38,7 +41,6 @@ describe('Drawing', function() { }); it('should append window URL to clip-path if is present', function() { - // append with href var base = d3.select('body') .append('base') diff --git a/test/jasmine/tests/plot_interact_test.js b/test/jasmine/tests/plot_interact_test.js index 640b57c8908..03e0338607f 100644 --- a/test/jasmine/tests/plot_interact_test.js +++ b/test/jasmine/tests/plot_interact_test.js @@ -2,6 +2,7 @@ var d3 = require('d3'); var Plotly = require('@lib/index'); var Lib = require('@src/lib'); +var Drawing = require('@src/components/drawing'); var createGraphDiv = require('../assets/create_graph_div'); var destroyGraphDiv = require('../assets/destroy_graph_div'); @@ -551,6 +552,7 @@ describe('plot svg clip paths', function() { d3.selectAll('[clip-path]').each(function() { var cp = d3.select(this).attr('clip-path'); + expect(Drawing.baseUrl).toBe(''); expect(cp.substring(0, 5)).toEqual('url(#'); expect(cp.substring(cp.length - 1)).toEqual(')'); }); @@ -560,6 +562,8 @@ describe('plot svg clip paths', function() { }); it('should set clip path url to ids appended to window url', function(done) { + // unstash base url from Drawing module object + delete Drawing.baseUrl; // this case occurs in some past versions of AngularJS // https://github.com/angular/angular.js/issues/8934 @@ -577,11 +581,13 @@ describe('plot svg clip paths', function() { d3.selectAll('[clip-path]').each(function() { var cp = d3.select(this).attr('clip-path'); + expect(Drawing.baseUrl).toBe(href); expect(cp.substring(0, 5 + href.length)).toEqual('url(' + href + '#'); expect(cp.substring(cp.length - 1)).toEqual(')'); }); base.remove(); + delete Drawing.baseUrl; }) .catch(failTest) .then(done); From 42ec6a0081a5dec480ed5cad04752fc597e7119e Mon Sep 17 00:00:00 2001 From: etienne Date: Fri, 16 Mar 2018 11:48:33 -0400 Subject: [PATCH 10/16] clear stashed base url off Drawing module on replot .. so that in the event where window.location changes on pages with a , our clip path urls will be properly updated. --- src/plot_api/plot_api.js | 3 +++ test/jasmine/tests/plot_interact_test.js | 4 ---- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index 1805321221e..d382240bf73 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -113,6 +113,9 @@ exports.plot = function(gd, data, layout, config) { // so we can share cached text across tabs Drawing.makeTester(); + // clear stashed base url + delete Drawing.baseUrl; + // collect promises for any async actions during plotting // any part of the plotting code can push to gd._promises, then // before we move to the next step, we check that they're all diff --git a/test/jasmine/tests/plot_interact_test.js b/test/jasmine/tests/plot_interact_test.js index 03e0338607f..06e5b692b49 100644 --- a/test/jasmine/tests/plot_interact_test.js +++ b/test/jasmine/tests/plot_interact_test.js @@ -562,9 +562,6 @@ describe('plot svg clip paths', function() { }); it('should set clip path url to ids appended to window url', function(done) { - // unstash base url from Drawing module object - delete Drawing.baseUrl; - // this case occurs in some past versions of AngularJS // https://github.com/angular/angular.js/issues/8934 @@ -587,7 +584,6 @@ describe('plot svg clip paths', function() { }); base.remove(); - delete Drawing.baseUrl; }) .catch(failTest) .then(done); From 928ff6dc7f6af328d9b1f28b6a7d545fd347e6c2 Mon Sep 17 00:00:00 2001 From: etienne Date: Mon, 19 Mar 2018 16:18:22 -0400 Subject: [PATCH 11/16] do not try to clear ax layers on first render - this can speed up Axes.doTicks by 50ms on 50x50 subplots grids --- src/plot_api/plot_api.js | 2 +- src/plots/cartesian/axes.js | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index d382240bf73..283efa08ce2 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -347,7 +347,7 @@ exports.plot = function(gd, data, layout, config) { // draw ticks, titles, and calculate axis scaling (._b, ._m) function drawAxes() { - return Axes.doTicks(gd, 'redraw'); + return Axes.doTicks(gd, graphWasEmpty ? '' : 'redraw'); } // Now plot the data diff --git a/src/plots/cartesian/axes.js b/src/plots/cartesian/axes.js index 5b6b766fac9..0f5801dc193 100644 --- a/src/plots/cartesian/axes.js +++ b/src/plots/cartesian/axes.js @@ -1539,10 +1539,8 @@ axes.doTicks = function(gd, axid, skipTitle) { return function() { if(!ax._id) return; var axDone = axes.doTicks(gd, ax._id); - if(axid === 'redraw') { - ax._r = ax.range.slice(); - ax._rl = Lib.simpleMap(ax._r, ax.r2l); - } + ax._r = ax.range.slice(); + ax._rl = Lib.simpleMap(ax._r, ax.r2l); return axDone; }; })); From a2c5c3d8a9c6ea54f9525b86719d40aee472014e Mon Sep 17 00:00:00 2001 From: etienne Date: Tue, 20 Mar 2018 11:11:10 -0400 Subject: [PATCH 12/16] use ensureSingle in plot bg lsInner routine --- src/plot_api/subroutines.js | 29 +++++++++-------------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/src/plot_api/subroutines.js b/src/plot_api/subroutines.js index 8a883cf5d89..9c4b322b3e9 100644 --- a/src/plot_api/subroutines.js +++ b/src/plot_api/subroutines.js @@ -124,31 +124,20 @@ exports.lsInner = function(gd) { var xDomain = plotinfo.xaxis.domain; var yDomain = plotinfo.yaxis.domain; - var plotgroupBgData = []; + var plotgroup = plotinfo.plotgroup; + var plotgroupBg; if(overlappingDomain(xDomain, yDomain, lowerDomains)) { - plotgroupBgData = [0]; - } - else { + var pgNode = plotgroup.node(); + plotgroupBg = plotinfo.bg = Lib.ensureSingle(plotgroup, 'rect', 'bg'); + pgNode.insertBefore(plotgroupBg.node(), pgNode.childNodes[0]); + } else { + plotgroupBg = plotgroup.select('rect,bg'); + if(plotgroupBg.size()) plotgroupBg.remove(); + lowerBackgroundIDs.push(subplot); lowerDomains.push([xDomain, yDomain]); } - - // create the plot group backgrounds now, since - // they're all independent selections - var plotgroupBg = plotinfo.plotgroup.selectAll('.bg') - .data(plotgroupBgData); - - plotgroupBg.enter().append('rect') - .classed('bg', true); - - plotgroupBg.exit().remove(); - - plotgroupBg.each(function() { - plotinfo.bg = plotgroupBg; - var pgNode = plotinfo.plotgroup.node(); - pgNode.insertBefore(this, pgNode.childNodes[0]); - }); }); // now create all the lower-layer backgrounds at once now that From 49177a2a4aae87ad78ca67c0a313e14e7afe3c07 Mon Sep 17 00:00:00 2001 From: etienne Date: Tue, 20 Mar 2018 11:11:43 -0400 Subject: [PATCH 13/16] lint in axes.js --- src/plots/cartesian/axes.js | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/src/plots/cartesian/axes.js b/src/plots/cartesian/axes.js index 0f5801dc193..a7aae8d4ca0 100644 --- a/src/plots/cartesian/axes.js +++ b/src/plots/cartesian/axes.js @@ -1550,21 +1550,21 @@ axes.doTicks = function(gd, axid, skipTitle) { // set scaling to pixels ax.setScale(); - var axLetter = axid.charAt(0), - counterLetter = axes.counterLetter(axid), - vals = axes.calcTicks(ax), - datafn = function(d) { return [d.text, d.x, ax.mirror, d.font, d.fontSize, d.fontColor].join('_'); }, - tcls = axid + 'tick', - gcls = axid + 'grid', - zcls = axid + 'zl', - pad = (ax.linewidth || 1) / 2, - labelStandoff = (ax.ticks === 'outside' ? ax.ticklen : 0), - labelShift = 0, - gridWidth = Drawing.crispRound(gd, ax.gridwidth, 1), - zeroLineWidth = Drawing.crispRound(gd, ax.zerolinewidth, gridWidth), - tickWidth = Drawing.crispRound(gd, ax.tickwidth, 1), - sides, transfn, tickpathfn, subplots, - i; + var axLetter = axid.charAt(0); + var counterLetter = axes.counterLetter(axid); + var vals = axes.calcTicks(ax); + var datafn = function(d) { return [d.text, d.x, ax.mirror, d.font, d.fontSize, d.fontColor].join('_'); }; + var tcls = axid + 'tick'; + var gcls = axid + 'grid'; + var zcls = axid + 'zl'; + var pad = (ax.linewidth || 1) / 2; + var labelStandoff = (ax.ticks === 'outside' ? ax.ticklen : 0); + var labelShift = 0; + var gridWidth = Drawing.crispRound(gd, ax.gridwidth, 1); + var zeroLineWidth = Drawing.crispRound(gd, ax.zerolinewidth, gridWidth); + var tickWidth = Drawing.crispRound(gd, ax.tickwidth, 1); + var sides, transfn, tickpathfn, subplots; + var i; if(ax._counterangle && ax.ticks === 'outside') { var caRad = ax._counterangle * Math.PI / 180; @@ -1614,10 +1614,11 @@ axes.doTicks = function(gd, axid, skipTitle) { Lib.warn('Unrecognized doTicks axis:', axid); return; } - var axside = ax.side || sides[0], + + var axside = ax.side || sides[0]; // which direction do the side[0], side[1], and free ticks go? // then we flip if outside XOR y axis - ticksign = [-1, 1, axside === sides[1] ? 1 : -1]; + var ticksign = [-1, 1, axside === sides[1] ? 1 : -1]; if((ax.ticks !== 'inside') === (axLetter === 'x')) { ticksign = ticksign.map(function(v) { return -v; }); } From 8cf99a6662b341328b92f2b56c3c51fabcbe4771 Mon Sep 17 00:00:00 2001 From: etienne Date: Tue, 20 Mar 2018 11:12:57 -0400 Subject: [PATCH 14/16] stash tickLabels selection in doTicks scope - so that drawTitle() can reuse it - this can speed up doTicks by 200ms at 50x50 subplots --- src/plots/cartesian/axes.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/plots/cartesian/axes.js b/src/plots/cartesian/axes.js index a7aae8d4ca0..c44db0c9856 100644 --- a/src/plots/cartesian/axes.js +++ b/src/plots/cartesian/axes.js @@ -1564,6 +1564,7 @@ axes.doTicks = function(gd, axid, skipTitle) { var zeroLineWidth = Drawing.crispRound(gd, ax.zerolinewidth, gridWidth); var tickWidth = Drawing.crispRound(gd, ax.tickwidth, 1); var sides, transfn, tickpathfn, subplots; + var tickLabels; var i; if(ax._counterangle && ax.ticks === 'outside') { @@ -1646,6 +1647,7 @@ axes.doTicks = function(gd, axid, skipTitle) { function drawTicks(container, tickpath) { var ticks = container.selectAll('path.' + tcls) .data(ax.ticks === 'inside' ? valsClipped : vals, datafn); + if(tickpath && ax.ticks) { ticks.enter().append('path').classed(tcls, 1).classed('ticks', 1) .classed('crisp', 1) @@ -1661,7 +1663,7 @@ axes.doTicks = function(gd, axid, skipTitle) { function drawLabels(container, position) { // tick labels - for now just the main labels. // TODO: mirror labels, esp for subplots - var tickLabels = container.selectAll('g.' + tcls).data(vals, datafn); + tickLabels = container.selectAll('g.' + tcls).data(vals, datafn); if(!isNumeric(position)) { tickLabels.remove(); @@ -2012,14 +2014,12 @@ axes.doTicks = function(gd, axid, skipTitle) { // now this only applies to regular cartesian axes; colorbars and // others ALWAYS call doTicks with skipTitle=true so they can // configure their own titles. - var ax = axisIds.getFromId(gd, axid); // rangeslider takes over a bottom title so drop it here if(ax.rangeslider && ax.rangeslider.visible && ax._boundingBox && ax.side === 'bottom') return; - var avoidSelection = d3.select(gd).selectAll('g.' + axid + 'tick'); var avoid = { - selection: avoidSelection, + selection: tickLabels, side: ax.side }; var axLetter = axid.charAt(0); @@ -2029,8 +2029,8 @@ axes.doTicks = function(gd, axid, skipTitle) { var transform, counterAxis, x, y; - if(avoidSelection.size()) { - var translation = Drawing.getTranslate(avoidSelection.node().parentNode); + if(tickLabels.size()) { + var translation = Drawing.getTranslate(tickLabels.node().parentNode); avoid.offsetLeft = translation.x; avoid.offsetTop = translation.y; } From 5307125ef9655bec0c2e24ba444b3cfc4c1c4f48 Mon Sep 17 00:00:00 2001 From: etienne Date: Mon, 26 Mar 2018 16:41:39 -0400 Subject: [PATCH 15/16] fixup typo in select query & remove neccessary branch --- src/plot_api/subroutines.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/plot_api/subroutines.js b/src/plot_api/subroutines.js index 9c4b322b3e9..6227536d273 100644 --- a/src/plot_api/subroutines.js +++ b/src/plot_api/subroutines.js @@ -132,9 +132,7 @@ exports.lsInner = function(gd) { plotgroupBg = plotinfo.bg = Lib.ensureSingle(plotgroup, 'rect', 'bg'); pgNode.insertBefore(plotgroupBg.node(), pgNode.childNodes[0]); } else { - plotgroupBg = plotgroup.select('rect,bg'); - if(plotgroupBg.size()) plotgroupBg.remove(); - + plotgroup.select('rect.bg').remove(); lowerBackgroundIDs.push(subplot); lowerDomains.push([xDomain, yDomain]); } From 4059764bf0c6aafcd533bb2f69e8a78f4ab1a6eb Mon Sep 17 00:00:00 2001 From: etienne Date: Mon, 26 Mar 2018 16:53:29 -0400 Subject: [PATCH 16/16] :hocho: unnecessary scoped var --- src/plot_api/subroutines.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/plot_api/subroutines.js b/src/plot_api/subroutines.js index 6227536d273..fa74da9e967 100644 --- a/src/plot_api/subroutines.js +++ b/src/plot_api/subroutines.js @@ -125,11 +125,10 @@ exports.lsInner = function(gd) { var xDomain = plotinfo.xaxis.domain; var yDomain = plotinfo.yaxis.domain; var plotgroup = plotinfo.plotgroup; - var plotgroupBg; if(overlappingDomain(xDomain, yDomain, lowerDomains)) { var pgNode = plotgroup.node(); - plotgroupBg = plotinfo.bg = Lib.ensureSingle(plotgroup, 'rect', 'bg'); + var plotgroupBg = plotinfo.bg = Lib.ensureSingle(plotgroup, 'rect', 'bg'); pgNode.insertBefore(plotgroupBg.node(), pgNode.childNodes[0]); } else { plotgroup.select('rect.bg').remove();