From abe1cc6c424f715406e094d28cde4bca204e6fbb Mon Sep 17 00:00:00 2001 From: My-Tien Nguyen Date: Wed, 16 Jul 2025 17:11:16 +0200 Subject: [PATCH 1/6] Fix NaN positions for shapes when dragging them. --- .../shapes/draw_newshape/newshapes.js | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/components/shapes/draw_newshape/newshapes.js b/src/components/shapes/draw_newshape/newshapes.js index 831a270e9ab..3353c523517 100644 --- a/src/components/shapes/draw_newshape/newshapes.js +++ b/src/components/shapes/draw_newshape/newshapes.js @@ -67,6 +67,7 @@ function newShapes(outlines, dragOptions) { clearOutline(gd); var editHelpers = dragOptions.editHelpers; + var plotinfo = dragOptions.plotinfo; var modifyItem = (editHelpers || {}).modifyItem; var allShapes = []; @@ -84,10 +85,21 @@ function newShapes(outlines, dragOptions) { case 'line': case 'rect': case 'circle': - modifyItem('x0', afterEdit.x0 - (beforeEdit.x0shift || 0)); - modifyItem('x1', afterEdit.x1 - (beforeEdit.x1shift || 0)); - modifyItem('y0', afterEdit.y0 - (beforeEdit.y0shift || 0)); - modifyItem('y1', afterEdit.y1 - (beforeEdit.y1shift || 0)); + if (beforeEdit.xref.includes("x") && plotinfo.xaxis.type.includes("category")) { + plotinfo.xaxis.r2c(afterEdit.x0) + modifyItem('x0', plotinfo.xaxis.r2c(afterEdit.x0) - (beforeEdit.x0shift || 0)); + modifyItem('x1', plotinfo.xaxis.r2c(afterEdit.x1) - (beforeEdit.x1shift || 0)); + } else { + modifyItem('x0', afterEdit.x0); + modifyItem('x1', afterEdit.x1); + } + if (beforeEdit.yref.includes("y") && plotinfo.yaxis.type.includes("category")) { + modifyItem('y0', plotinfo.xaxis.r2c(afterEdit.y0) - (beforeEdit.y0shift || 0)); + modifyItem('y1', plotinfo.xaxis.r2c(afterEdit.y1) - (beforeEdit.y1shift || 0)); + } else { + modifyItem('y0', afterEdit.y0); + modifyItem('y1', afterEdit.y1); + } break; case 'path': From 9cd526273cad2f385ab83c012962be513d256346 Mon Sep 17 00:00:00 2001 From: My-Tien Nguyen Date: Thu, 17 Jul 2025 13:10:09 +0200 Subject: [PATCH 2/6] - Remove r2c call. Unnecessary, since category references seem to have been converted to numerical before this point already. - Don't use plotinfo to retrieve the axis, somehow it doesn't always contain the correct axis information. Use `axis_ids.getFromId` instead. - Fix formatting --- .../shapes/draw_newshape/newshapes.js | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/components/shapes/draw_newshape/newshapes.js b/src/components/shapes/draw_newshape/newshapes.js index 3353c523517..2c03da4d9bc 100644 --- a/src/components/shapes/draw_newshape/newshapes.js +++ b/src/components/shapes/draw_newshape/newshapes.js @@ -1,5 +1,7 @@ 'use strict'; +var axis_ids = require('../../../plots/cartesian/axis_ids'); + var dragHelpers = require('../../dragelement/helpers'); var drawMode = dragHelpers.drawMode; var openMode = dragHelpers.openMode; @@ -67,7 +69,6 @@ function newShapes(outlines, dragOptions) { clearOutline(gd); var editHelpers = dragOptions.editHelpers; - var plotinfo = dragOptions.plotinfo; var modifyItem = (editHelpers || {}).modifyItem; var allShapes = []; @@ -85,17 +86,19 @@ function newShapes(outlines, dragOptions) { case 'line': case 'rect': case 'circle': - if (beforeEdit.xref.includes("x") && plotinfo.xaxis.type.includes("category")) { - plotinfo.xaxis.r2c(afterEdit.x0) - modifyItem('x0', plotinfo.xaxis.r2c(afterEdit.x0) - (beforeEdit.x0shift || 0)); - modifyItem('x1', plotinfo.xaxis.r2c(afterEdit.x1) - (beforeEdit.x1shift || 0)); + + var xaxis = axis_ids.getFromId(gd, beforeEdit.xref); + if (beforeEdit.xref.includes('x') && xaxis.type.includes("category")) { + modifyItem('x0', afterEdit.x0 - (beforeEdit.x0shift || 0)); + modifyItem('x1', afterEdit.x1 - (beforeEdit.x1shift || 0)); } else { modifyItem('x0', afterEdit.x0); modifyItem('x1', afterEdit.x1); } - if (beforeEdit.yref.includes("y") && plotinfo.yaxis.type.includes("category")) { - modifyItem('y0', plotinfo.xaxis.r2c(afterEdit.y0) - (beforeEdit.y0shift || 0)); - modifyItem('y1', plotinfo.xaxis.r2c(afterEdit.y1) - (beforeEdit.y1shift || 0)); + var yaxis = axis_ids.getFromId(gd, beforeEdit.yref); + if (beforeEdit.yref.includes('y') && yaxis.type.includes("category")) { + modifyItem('y0', afterEdit.y0 - (beforeEdit.y0shift || 0)); + modifyItem('y1', afterEdit.y1 - (beforeEdit.y1shift || 0)); } else { modifyItem('y0', afterEdit.y0); modifyItem('y1', afterEdit.y1); From 2f2c4a069f080aac842aa66535af3bb7f347e89c Mon Sep 17 00:00:00 2001 From: My-Tien Nguyen Date: Thu, 17 Jul 2025 13:49:42 +0200 Subject: [PATCH 3/6] Fix formatting for category string --- src/components/shapes/draw_newshape/newshapes.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/shapes/draw_newshape/newshapes.js b/src/components/shapes/draw_newshape/newshapes.js index 2c03da4d9bc..7b58a782ff0 100644 --- a/src/components/shapes/draw_newshape/newshapes.js +++ b/src/components/shapes/draw_newshape/newshapes.js @@ -88,7 +88,7 @@ function newShapes(outlines, dragOptions) { case 'circle': var xaxis = axis_ids.getFromId(gd, beforeEdit.xref); - if (beforeEdit.xref.includes('x') && xaxis.type.includes("category")) { + if (beforeEdit.xref.includes('x') && xaxis.type.includes('category')) { modifyItem('x0', afterEdit.x0 - (beforeEdit.x0shift || 0)); modifyItem('x1', afterEdit.x1 - (beforeEdit.x1shift || 0)); } else { @@ -96,7 +96,7 @@ function newShapes(outlines, dragOptions) { modifyItem('x1', afterEdit.x1); } var yaxis = axis_ids.getFromId(gd, beforeEdit.yref); - if (beforeEdit.yref.includes('y') && yaxis.type.includes("category")) { + if (beforeEdit.yref.includes('y') && yaxis.type.includes('category')) { modifyItem('y0', afterEdit.y0 - (beforeEdit.y0shift || 0)); modifyItem('y1', afterEdit.y1 - (beforeEdit.y1shift || 0)); } else { From 13e3cc18ea153246210c569d30327cb981b51fcc Mon Sep 17 00:00:00 2001 From: My-Tien Nguyen Date: Thu, 17 Jul 2025 13:50:18 +0200 Subject: [PATCH 4/6] WIP draw_newshape_test --- test/jasmine/tests/draw_newshape_test.js | 37 ++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/test/jasmine/tests/draw_newshape_test.js b/test/jasmine/tests/draw_newshape_test.js index 657df5b2c0f..55e6a7554cb 100644 --- a/test/jasmine/tests/draw_newshape_test.js +++ b/test/jasmine/tests/draw_newshape_test.js @@ -1426,6 +1426,43 @@ describe('Activate and edit editable shapes', function() { .then(done, done.fail); }); + + it('should be possible to drag shapes referencing non-categorical axes', function(done) { + Plotly.newPlot(gd, { + data: [ + { + x: ["2015-02-01", "2015-02-02", "2015-02-03"], + y: [14, 17, 8], + mode: "line", + }, + ], + layout: { + xaxis: { title: { text: "Date" }, type: "date" }, + dragmode: "drawline", + shapes: [ + { + type: "rect", + xref: "x", + yref: "paper", + x0: "2015-02-02", + y0: 0, + x1: "2015-02-08", + y1: 1, + opacity: 0.2, + editable: true, + }, + ], + }, + }) + .then(function() { drag([[257.64, 370], [250, 300]]); }) // move lower left corner up and left + .then(function () { + var shapes = gd._fullLayout.shapes; + var obj = shapes[0]._input; + print(obj); + assertPos(obj.path, 'M250,300H1019V100H257.64Z'); + }) + .then(done, done.fail); + }); }); }); From c996699355d8e315ed6a8802d46d8892ae9044da Mon Sep 17 00:00:00 2001 From: My-Tien Nguyen Date: Thu, 17 Jul 2025 21:25:43 +0200 Subject: [PATCH 5/6] Use .charAt(0) === 'x'/'y' to test for xref/yref pointing to an axis. (as is being done at other locations to test this) --- src/components/shapes/draw_newshape/newshapes.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/shapes/draw_newshape/newshapes.js b/src/components/shapes/draw_newshape/newshapes.js index 7b58a782ff0..7145838c04d 100644 --- a/src/components/shapes/draw_newshape/newshapes.js +++ b/src/components/shapes/draw_newshape/newshapes.js @@ -88,7 +88,7 @@ function newShapes(outlines, dragOptions) { case 'circle': var xaxis = axis_ids.getFromId(gd, beforeEdit.xref); - if (beforeEdit.xref.includes('x') && xaxis.type.includes('category')) { + if (beforeEdit.xref.charAt(0) === 'x' && xaxis.type.includes('category')) { modifyItem('x0', afterEdit.x0 - (beforeEdit.x0shift || 0)); modifyItem('x1', afterEdit.x1 - (beforeEdit.x1shift || 0)); } else { @@ -96,7 +96,7 @@ function newShapes(outlines, dragOptions) { modifyItem('x1', afterEdit.x1); } var yaxis = axis_ids.getFromId(gd, beforeEdit.yref); - if (beforeEdit.yref.includes('y') && yaxis.type.includes('category')) { + if (beforeEdit.yref.charAt(0) === 'y' && yaxis.type.includes('category')) { modifyItem('y0', afterEdit.y0 - (beforeEdit.y0shift || 0)); modifyItem('y1', afterEdit.y1 - (beforeEdit.y1shift || 0)); } else { From 58c811acd9ab04263299f1243abda3895956ce91 Mon Sep 17 00:00:00 2001 From: My-Tien Nguyen Date: Mon, 21 Jul 2025 09:47:20 +0200 Subject: [PATCH 6/6] Add jasmine tests for dragging shape attached to date-axis (thx Mojtaba, for the code) --- test/jasmine/tests/draw_newshape_test.js | 316 ++++++++++++++++++++--- 1 file changed, 279 insertions(+), 37 deletions(-) diff --git a/test/jasmine/tests/draw_newshape_test.js b/test/jasmine/tests/draw_newshape_test.js index 55e6a7554cb..421509265bc 100644 --- a/test/jasmine/tests/draw_newshape_test.js +++ b/test/jasmine/tests/draw_newshape_test.js @@ -1426,43 +1426,6 @@ describe('Activate and edit editable shapes', function() { .then(done, done.fail); }); - - it('should be possible to drag shapes referencing non-categorical axes', function(done) { - Plotly.newPlot(gd, { - data: [ - { - x: ["2015-02-01", "2015-02-02", "2015-02-03"], - y: [14, 17, 8], - mode: "line", - }, - ], - layout: { - xaxis: { title: { text: "Date" }, type: "date" }, - dragmode: "drawline", - shapes: [ - { - type: "rect", - xref: "x", - yref: "paper", - x0: "2015-02-02", - y0: 0, - x1: "2015-02-08", - y1: 1, - opacity: 0.2, - editable: true, - }, - ], - }, - }) - .then(function() { drag([[257.64, 370], [250, 300]]); }) // move lower left corner up and left - .then(function () { - var shapes = gd._fullLayout.shapes; - var obj = shapes[0]._input; - print(obj); - assertPos(obj.path, 'M250,300H1019V100H257.64Z'); - }) - .then(done, done.fail); - }); }); }); @@ -1695,3 +1658,282 @@ describe('Activate and edit editable shapes', function() { .then(done, done.fail); }); }); + +describe('Activate and edit editable shapes - date axes', function() { + var fig = { + data: [ + { + x: [ + 0, + 50 + ], + y: [ + 0, + 50 + ] + } + ], + layout: { + width: 800, + height: 600, + margin: { + t: 100, + b: 50, + l: 100, + r: 50 + }, + xaxis: { + type: 'date', + range: ["1975-07-01", "2380-07-01"] + }, + yaxis: { + range: [301.78041543026706, -18.694362017804156] + }, + shapes: [ + { + editable: true, + layer: 'below', + type: 'rect', + line: { + width: 5 + }, + fillcolor: 'red', + opacity: 0.5, + xref: 'xaxis', + yref: 'yaxis', + x0: '2025-01-01', + y0: 25, + x1: '2075-01-01', + y1: 75 + }, + { + editable: true, + layer: 'top', + type: 'circle', + line: { + width: 5 + }, + fillcolor: 'green', + opacity: 0.5, + xref: 'xaxis', + yref: 'yaxis', + x0: '2125-01-01', + y0: 25, + x1: '2175-01-01', + y1: 75 + } + ] + }, + config: { + editable: false, + modeBarButtonsToAdd: [ + 'drawline', + 'drawopenpath', + 'drawclosedpath', + 'drawcircle', + 'drawrect', + 'eraseshape' + ] + } + }; + + var gd; + + beforeEach(function() { + gd = createGraphDiv(); + }); + + afterEach(destroyGraphDiv); + + ['mouse'].forEach(function(device) { + it('reactangle using ' + device, function(done) { + var i = 0; // shape index + + Plotly.newPlot(gd, { + data: fig.data, + layout: fig.layout, + config: fig.config + }) + + // shape between 175, 160 and 255, 230 + .then(function() { click(200, 160); }) // activate shape + .then(function() { + var id = gd._fullLayout._activeShapeIndex; + expect(id).toEqual(i, 'activate shape by clicking border'); + + var shapes = gd._fullLayout.shapes; + var obj = shapes[id]._input; + expect(obj.type).toEqual('rect'); + print(obj); + assertPos({ + x0: obj.x0, + y0: obj.y0, + x1: obj.x1, + y1: obj.y1 + }, { + x0: '2025-01-01', + y0: 25, + x1: '2075-01-01', + y1: 75 + }); + }) + .then(function() { drag([[255, 230], [300, 200]]); }) // move vertex + .then(function() { + var id = gd._fullLayout._activeShapeIndex; + expect(id).toEqual(i, 'keep shape active after drag corner'); + + var shapes = gd._fullLayout.shapes; + var obj = shapes[id]._input; + expect(obj.type).toEqual('rect'); + print(obj); + assertPos({ + x0: obj.x0, + y0: obj.y0, + x1: obj.x1, + y1: obj.y1 + }, { + x0: '2024-12-30 20:44:36.1846', + y0: 24.997032640949552, + x1: '2103-01-15 16:20:58.3385', + y1: 53.63323442136499 + }); + }) + .then(function() { drag([[300, 200], [255, 230]]); }) // move vertex back + .then(function() { + var id = gd._fullLayout._activeShapeIndex; + expect(id).toEqual(i, 'keep shape active after drag corner'); + + var shapes = gd._fullLayout.shapes; + var obj = shapes[id]._input; + expect(obj.type).toEqual('rect'); + print(obj); + assertPos({ + x0: obj.x0, + y0: obj.y0, + x1: obj.x1, + y1: obj.y1 + }, { + x0: '2024-12-30 20:44:36.1846', + y0: 25, + x1: '2074-12-31 18:56:02.9538', + y1: 75 + }); + }) + .then(function() { drag([[215, 195], [300, 200]]); }) // move shape + .then(function() { + var id = gd._fullLayout._activeShapeIndex; + expect(id).toEqual(i, 'keep shape active after drag corner'); + + var shapes = gd._fullLayout.shapes; + var obj = shapes[id]._input; + expect(obj.type).toEqual('rect'); + print(obj); + assertPos({ + x0: obj.x0, + y0: obj.y0, + x1: obj.x1, + y1: obj.y1 + }, { + x0: '2077-12-16 18:31:40.8', + y0: 24.997032640949552, + x1: '2127-12-18 16:43:07.5692', + y1: 74.99821958456974 + }); + }) + .then(function() { drag([[300, 200], [215, 195]]); }) // move shape back + .then(function() { + var id = gd._fullLayout._activeShapeIndex; + expect(id).toEqual(i, 'keep shape active after drag corner'); + + var shapes = gd._fullLayout.shapes; + var obj = shapes[id]._input; + expect(obj.type).toEqual('rect'); + print(obj); + assertPos({ + x0: obj.x0, + y0: obj.y0, + x1: obj.x1, + y1: obj.y1 + }, { + x0: '2024-12-30 20:44:36.1846', + y0: 25, + x1: '2074-12-31 18:56:02.9538', + y1: 75 + }); + }) + .then(function() { click(100, 100); }) + .then(function() { + var id = gd._fullLayout._activeShapeIndex; + expect(id).toEqual(undefined, 'deactivate shape by clicking outside'); + }) + .then(function() { click(255, 230); }) + .then(function() { + var id = gd._fullLayout._activeShapeIndex; + expect(id).toEqual(i, 'activate shape by clicking on corner'); + }) + .then(function() { click(215, 195); }) + .then(function() { + var id = gd._fullLayout._activeShapeIndex; + expect(id).toEqual(undefined, 'deactivate shape by clicking inside'); + }) + + .then(done, done.fail); + }); + + it('circle using ' + device, function(done) { + var i = 1; // shape index + + Plotly.newPlot(gd, { + data: fig.data, + layout: fig.layout, + config: fig.config + }) + + // next shape + .then(function() { click(355, 225); }) // activate shape + .then(function() { + var id = gd._fullLayout._activeShapeIndex; + expect(id).toEqual(i, 'activate shape by clicking border'); + + var shapes = gd._fullLayout.shapes; + var obj = shapes[id]._input; + expect(obj.type).toEqual('circle'); + print(obj); + assertPos({ + x0: obj.x0, + y0: obj.y0, + x1: obj.x1, + y1: obj.y1 + }, { + x0: '2125-01-01', + x1: '2175-01-01', + y0: 25, + y1: 75 + }); + }) + .then(function() { drag([[338, 196], [300, 175]]); }) // move vertex + .then(function() { + var id = gd._fullLayout._activeShapeIndex; + expect(id).toEqual(i, 'keep shape active after drag corner'); + + var shapes = gd._fullLayout.shapes; + var obj = shapes[id]._input; + expect(obj.type).toEqual('circle'); + print(obj); + assertPos({ + x0: obj.x0, + y0: obj.y0, + x1: obj.x1, + y1: obj.y1 + }, { + x0: '2186-11-02 07:04:22.7446', + y0: 74.99821958456971, + x1: '2113-03-01 18:44:58.3385', + y1: 10.04154302670623 + }); + }) + + .then(done, done.fail); + }); + }); +});