diff --git a/src/lib/index.js b/src/lib/index.js index a6086e300ce..8b9d36d1e11 100644 --- a/src/lib/index.js +++ b/src/lib/index.js @@ -478,6 +478,17 @@ lib.extractOption = function(calcPt, trace, calcKey, traceKey) { if(!Array.isArray(traceVal)) return traceVal; }; +function makePtIndex2PtNumber(indexToPoints) { + var ptIndex2ptNumber = {}; + for(var k in indexToPoints) { + var pts = indexToPoints[k]; + for(var j = 0; j < pts.length; j++) { + ptIndex2ptNumber[pts[j]] = +k; + } + } + return ptIndex2ptNumber; +} + /** Tag selected calcdata items * * N.B. note that point 'index' corresponds to input data array index @@ -498,13 +509,7 @@ lib.tagSelected = function(calcTrace, trace, ptNumber2cdIndex) { // make pt index-to-number map object, which takes care of transformed traces if(indexToPoints) { - ptIndex2ptNumber = {}; - for(var k in indexToPoints) { - var pts = indexToPoints[k]; - for(var j = 0; j < pts.length; j++) { - ptIndex2ptNumber[pts[j]] = k; - } - } + ptIndex2ptNumber = makePtIndex2PtNumber(indexToPoints); } function isCdIndexValid(v) { @@ -525,6 +530,30 @@ lib.tagSelected = function(calcTrace, trace, ptNumber2cdIndex) { } }; +lib.selIndices2selPoints = function(trace) { + var selectedpoints = trace.selectedpoints; + var indexToPoints = trace._indexToPoints; + + if(indexToPoints) { + var ptIndex2ptNumber = makePtIndex2PtNumber(indexToPoints); + var out = []; + + for(var i = 0; i < selectedpoints.length; i++) { + var ptIndex = selectedpoints[i]; + if(lib.isIndex(ptIndex)) { + var ptNumber = ptIndex2ptNumber[ptIndex]; + if(lib.isIndex(ptNumber)) { + out.push(ptNumber); + } + } + } + + return out; + } else { + return selectedpoints; + } +}; + /** Returns target as set by 'target' transform attribute * * @param {object} trace : full trace object diff --git a/src/plots/plots.js b/src/plots/plots.js index 5be17ecdfbc..c49f5cb3ae7 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -2342,7 +2342,15 @@ plots.doCalcdata = function(gd, traces) { // we need one round of trace module calc before // the calc transform to 'fill in' the categories list // used for example in the data-to-coordinate method - if(_module && _module.calc) _module.calc(gd, trace); + if(_module && _module.calc) { + var cdi = _module.calc(gd, trace); + + // must clear scene 'batches', so that 2nd + // _module.calc call starts from scratch + if(cdi[0] && cdi[0].t && cdi[0].t._scene) { + delete cdi[0].t._scene.dirty; + } + } for(j = 0; j < trace.transforms.length; j++) { var transform = trace.transforms[j]; diff --git a/src/traces/scattergl/convert.js b/src/traces/scattergl/convert.js index 83788abe95a..ba3878f1833 100644 --- a/src/traces/scattergl/convert.js +++ b/src/traces/scattergl/convert.js @@ -192,10 +192,10 @@ function convertMarkerStyle(trace) { // See https://github.com/plotly/plotly.js/pull/1781#discussion_r121820798 if(multiLineWidth) { for(i = 0; i < count; i++) { - borderSizes[i] = markerSizeFunc(optsIn.line.width[i]); + borderSizes[i] = optsIn.line.width[i] / 2; } } else { - s = markerSizeFunc(optsIn.line.width); + s = optsIn.line.width / 2; for(i = 0; i < count; i++) { borderSizes[i] = s; } @@ -219,7 +219,7 @@ function convertMarkerSelection(trace, target) { if(target.marker && target.marker.symbol) { optsOut = convertMarkerStyle(Lib.extendFlat({}, optsIn, target.marker)); } else if(target.marker) { - if(target.marker.size) optsOut.sizes = target.marker.size; + if(target.marker.size) optsOut.sizes = target.marker.size / 2; if(target.marker.color) optsOut.colors = target.marker.color; if(target.marker.opacity !== undefined) optsOut.opacity = target.marker.opacity; } diff --git a/src/traces/scattergl/index.js b/src/traces/scattergl/index.js index 934b60c9ca8..17b2096e37c 100644 --- a/src/traces/scattergl/index.js +++ b/src/traces/scattergl/index.js @@ -492,12 +492,11 @@ function plot(gd, subplot, cdata) { // regenerate scene batch, if traces number changed during selection if(trace.selectedpoints) { - scene.selectBatch[id] = trace.selectedpoints; + var selPts = scene.selectBatch[id] = Lib.selIndices2selPoints(trace); - var selPts = trace.selectedpoints; var selDict = {}; for(i = 0; i < selPts.length; i++) { - selDict[selPts[i]] = true; + selDict[selPts[i]] = 1; } var unselPts = []; for(i = 0; i < stash.count; i++) { diff --git a/test/image/baselines/gl2d_point-selection.png b/test/image/baselines/gl2d_point-selection.png new file mode 100644 index 00000000000..f4e67323d98 Binary files /dev/null and b/test/image/baselines/gl2d_point-selection.png differ diff --git a/test/image/baselines/gl2d_transforms.png b/test/image/baselines/gl2d_transforms.png new file mode 100644 index 00000000000..a2f9769dfaa Binary files /dev/null and b/test/image/baselines/gl2d_transforms.png differ diff --git a/test/image/mocks/gl2d_point-selection.json b/test/image/mocks/gl2d_point-selection.json new file mode 100644 index 00000000000..a7427865108 --- /dev/null +++ b/test/image/mocks/gl2d_point-selection.json @@ -0,0 +1,74 @@ +{ + "data": [{ + "type": "scattergl", + "mode": "lines+markers+text", + "x": [1, 2, 3, 4, 5, 6], + "y": [1, 3, 2, 4, 5, 7], + "ids": ["a", "b", "c", "d", "e", "f"], + "text": ["a", "b", "c", "d", "e", "f"], + "marker": { + "color": "#67353E", + "size": 12 + }, + "textposition": "top left", + "selectedpoints": [1, 2, 3], + "selected": { + "marker": { + "color": "blue", + "size": 20 + } + }, + "unselected": { + "marker": { + "color": "green", + "opacity": 0.5 + } + }, + "transforms": [{ + "type": "filter", + "target": "x", + "operation": "][", + "value": [3.5, 4.5] + }] + }, { + "type": "scattergl", + "mode": "lines+markers+text", + "x": [1, 2, 3, 4, 5, 6], + "y": [6, 5, 6, 5, 4, 3], + "ids": ["a", "b", "c", "d", "e", "f"], + "text": ["a", "b", "c", "d", "e", "f"], + "marker": { + "color": "#34ABA2", + "size": 12 + }, + "textposition": "top left", + "selectedpoints": [], + "selected": { + "marker": { + "color": "green" + } + }, + "unselected": { + "marker": { + "color": "blue", + "opacity": 0.5 + } + }, + "transforms": [{ + "type": "filter", + "target": "x", + "operation": "][", + "value": [3.5, 4.5] + }] + }], + "layout": { + "dragmode": "lasso", + "legend": { + "x": 0, + "y": 1, + "xanchor": "left", + "yanchor": "bottom" + }, + "width": 600 + } +} diff --git a/test/image/mocks/gl2d_transforms.json b/test/image/mocks/gl2d_transforms.json new file mode 100644 index 00000000000..ce6a128ae92 --- /dev/null +++ b/test/image/mocks/gl2d_transforms.json @@ -0,0 +1,75 @@ +{ + "data": [{ + "type": "scattergl", + "mode": "lines+markers", + "x": [1, -1, -2, 0, 1, 3, 3], + "y": [2, 1, 0, 1, 3, 4, 3], + "transforms": [{ + "type": "groupby", + "groups": ["a", "a", "b", "a", "b", "b", "a"], + "styles": [ + {"target": "a", "value": {"marker": {"color": "orange"}}}, + {"target": "b", "value": {"marker": {"color": "blue"}}} + ] + }, { + "type": "filter", + "target": "x", + "operation": ">=", + "value": 0, + "preservegaps": true + }], + "name": "Groupby+filter" + }, + { + "type": "scattergl", + "x": [1, 2, 3, 4, -3], + "y": [1.1, 2.2, 3.3, 4.4, 5.5], + "marker": { + "size": [0.3, 0.2, 0.1, 0.4, 0.5], + "sizeref": 0.01, + "color": [2, 4, 6, 10, 8], + "opacity": [0.9, 0.6, 0.2, 0.8, 1.0], + "line": { + "color": [2.2, 3.3, 4.4, 5.5, 1.1] + } + }, + "transforms": [{ + "type": "aggregate", + "groups": ["a", "b", "a", "a", "a"], + "aggregations": [ + {"target": "x", "func": "sum"}, + {"target": "y", "func": "avg"}, + {"target": "marker.size", "func": "min"}, + {"target": "marker.color", "func": "max"}, + {"target": "marker.line.color", "func": "last"}, + {"target": "marker.line.width", "func": "count"} + ] + }], + "name": "Aggregate" + }, + { + "type": "scattergl", + "x": [1, 2, 3, 4, 5, 6], + "y": [1, 4, 2, 6, 5, 3], + "transforms": [{ + "type": "sort", + "target": [1, 6, 2, 5, 3, 4] + }], + "name": "Sort" + }, + { + "type": "scattergl", + "x":[4, 5, 6, 4, 5, 6], + "y": [1, 1, 1, 2, 2, 2], + "marker": {"color": [1, 2, 3, -1, -2, -3], "size": 20}, + "mode": "lines+markers", + "transforms": [ + {"type": "groupby", "groups": [1, 1, 1, 2, 2, 2]} + ] + }], + "layout": { + "width": 600, + "height": 400, + "title": "Transforms on scattergl traces" + } +} diff --git a/test/jasmine/tests/gl2d_click_test.js b/test/jasmine/tests/gl2d_click_test.js index 3a6f5635fa8..1bade67fc6a 100644 --- a/test/jasmine/tests/gl2d_click_test.js +++ b/test/jasmine/tests/gl2d_click_test.js @@ -4,7 +4,7 @@ var Lib = require('@src/lib'); var d3 = require('d3'); var createGraphDiv = require('../assets/create_graph_div'); var destroyGraphDiv = require('../assets/destroy_graph_div'); -var fail = require('../assets/fail_test.js'); +var failTest = require('../assets/fail_test.js'); var customAssertions = require('../assets/custom_assertions'); var assertHoverLabelStyle = customAssertions.assertHoverLabelStyle; @@ -217,7 +217,7 @@ describe('@gl @flaky Test hover and click interactions', function() { Plotly.plot(gd, _mock) .then(run) - .catch(fail) + .catch(failTest) .then(done); }); @@ -256,7 +256,7 @@ describe('@gl @flaky Test hover and click interactions', function() { Plotly.plot(gd, _mock) .then(run) - .catch(fail) + .catch(failTest) .then(done); }); @@ -295,7 +295,7 @@ describe('@gl @flaky Test hover and click interactions', function() { Plotly.plot(gd, _mock) .then(run) - .catch(fail) + .catch(failTest) .then(done); }); @@ -315,7 +315,7 @@ describe('@gl @flaky Test hover and click interactions', function() { Plotly.plot(gd, _mock) .then(run) - .catch(fail) + .catch(failTest) .then(done); }); @@ -343,7 +343,7 @@ describe('@gl @flaky Test hover and click interactions', function() { Plotly.plot(gd, _mock) .then(run) - .catch(fail) + .catch(failTest) .then(done); }); @@ -376,7 +376,7 @@ describe('@gl @flaky Test hover and click interactions', function() { Plotly.plot(gd, _mock) .then(run) - .catch(fail) + .catch(failTest) .then(done); }); @@ -409,7 +409,7 @@ describe('@gl @flaky Test hover and click interactions', function() { Plotly.plot(gd, _mock) .then(run) - .catch(fail) + .catch(failTest) .then(done); }); @@ -451,7 +451,7 @@ describe('@gl @flaky Test hover and click interactions', function() { return Plotly.restyle(gd, 'visible', false, [1]); }) .then(run2) - .catch(fail) + .catch(failTest) .then(done); }); @@ -499,7 +499,7 @@ describe('@gl @flaky Test hover and click interactions', function() { return Plotly.restyle(gd, 'visible', false, [1]); }) .then(run2) - .catch(fail) + .catch(failTest) .then(done); }); @@ -527,7 +527,7 @@ describe('@gl @flaky Test hover and click interactions', function() { Plotly.plot(gd, _mock) .then(run) - .catch(fail) + .catch(failTest) .then(done); }); }); @@ -623,7 +623,7 @@ describe('@noCI @gl Test gl2d lasso/select:', function() { }); }) - .catch(fail) + .catch(failTest) .then(done); }); @@ -647,7 +647,7 @@ describe('@noCI @gl Test gl2d lasso/select:', function() { ] }); }) - .catch(fail) + .catch(failTest) .then(done); }); @@ -667,7 +667,7 @@ describe('@noCI @gl Test gl2d lasso/select:', function() { points: [{x: 0.004, y: 12.5}] }); }) - .catch(fail) + .catch(failTest) .then(done); }); @@ -686,7 +686,30 @@ describe('@noCI @gl Test gl2d lasso/select:', function() { points: [{ x: 0.099, y: 2.75 }] }); }) - .catch(fail) + .catch(failTest) + .then(done); + }); + + it('should work on trace with enabled transforms', function(done) { + var fig = Lib.extendDeep({}, require('@mocks/gl2d_transforms.json')); + fig.layout.dragmode = 'select'; + fig.layout.margin = {t: 0, b: 0, l: 0, r: 0}; + fig.layout.height = 500; + fig.layout.width = 500; + gd = createGraphDiv(); + + Plotly.plot(gd, fig) + .then(delay(100)) + .then(function() { return select([[100, 100], [250, 250]]); }) + .then(function(eventData) { + assertEventData(eventData, { + points: [ + { x: 3, y: 4 }, + { x: 2, y: 4 } + ] + }); + }) + .catch(failTest) .then(done); }); }); diff --git a/test/jasmine/tests/gl2d_plot_interact_test.js b/test/jasmine/tests/gl2d_plot_interact_test.js index 15f5c92169d..f2be65b1fa9 100644 --- a/test/jasmine/tests/gl2d_plot_interact_test.js +++ b/test/jasmine/tests/gl2d_plot_interact_test.js @@ -962,6 +962,72 @@ describe('@gl Test gl2d plots', function() { .catch(failTest) .then(done); }); + + it('should handle transform traces properly (calcTransform case)', function(done) { + spyOn(ScatterGl, 'calc').and.callThrough(); + + Plotly.plot(gd, [{ + type: 'scattergl', + x: [1, 2, 3], + y: [1, 2, 1], + transforms: [{ + type: 'filter', + target: 'x', + operation: '>', + value: 1 + }] + }]) + .then(function() { + expect(ScatterGl.calc).toHaveBeenCalledTimes(2); + + var opts = gd.calcdata[0][0].t._scene.markerOptions; + // length === 2 before #2677 + expect(opts.length).toBe(1); + + return Plotly.restyle(gd, 'selectedpoints', [[1]]); + }) + .then(function() { + // was === 1 before #2677 + var scene = gd.calcdata[0][0].t._scene; + expect(scene.selectBatch[0]).toEqual([0]); + }) + .catch(failTest) + .then(done); + }); + + it('should handle transform traces properly (default transform case)', function(done) { + spyOn(ScatterGl, 'calc').and.callThrough(); + + Plotly.plot(gd, [{ + type: 'scattergl', + x: [1, 2, 3], + y: [1, 2, 1], + transforms: [{ + type: 'groupby', + groups: ['a', 'b', 'a'] + }] + }]) + .then(function() { + // twice per 'expanded' trace + expect(ScatterGl.calc).toHaveBeenCalledTimes(4); + + // 'scene' from opts0 and opts1 is linked to the same object, + // which has two items, one for each 'expanded' trace + var opts0 = gd.calcdata[0][0].t._scene.markerOptions; + expect(opts0.length).toBe(2); + + var opts1 = gd.calcdata[1][0].t._scene.markerOptions; + expect(opts1.length).toBe(2); + + return Plotly.restyle(gd, 'selectedpoints', [[1]]); + }) + .then(function() { + var scene = gd.calcdata[0][0].t._scene; + expect(scene.selectBatch).toEqual([[], [0]]); + }) + .catch(failTest) + .then(done); + }); }); describe('Test scattergl autorange:', function() {