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/5] 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/5] - 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/5] 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/5] 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/5] 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 {