Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2k
Fix NaN positions for shapes when dragging them.#7470
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Changes from4 commits
abe1cc69cd52622f2c4a013e3cc1c9966995fa469258c811aee3106f7cce307File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
| @@ -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; | ||||
| @@ -84,10 +86,23 @@ function newShapes(outlines, dragOptions) { | ||||
| case 'line': | ||||
| case 'rect': | ||||
| case 'circle': | ||||
| var xaxis = axis_ids.getFromId(gd, beforeEdit.xref); | ||||
| if (beforeEdit.xref.includes('x') && xaxis.type.includes('category')) { | ||||
my-tien marked this conversation as resolved. OutdatedShow resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||||
| modifyItem('x0', afterEdit.x0 - (beforeEdit.x0shift || 0)); | ||||
| modifyItem('x1', afterEdit.x1 - (beforeEdit.x1shift || 0)); | ||||
| } else { | ||||
| modifyItem('x0', afterEdit.x0); | ||||
| modifyItem('x1', afterEdit.x1); | ||||
| } | ||||
| 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); | ||||
| } | ||||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Thanks@my-tien for the fix.
ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. I'll have a look at path and adding a test. However, my fix is not yet correct. Among other things, afterEdit can be a shape with xref and yref paper apparently even if the ref was something different before. I'll have to see, how I can correctly translate the shift variables to paper as well. I will probably push some changes later today. UPDATE: This is a separate bug. As mentioned further below, I'll open an issue for it later. ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. For the path case the change is not necessary, because the shift properties are ignored if path is used instead of x0, x1, y0, y1. Regarding the test, I added one with a test case that previously failed and now works. However, I am not sure how to test this locally because for some reason on my machine several Jasmine tests always fail. Could you please look at it on your end and make the necessary adjustments? | ||||
| break; | ||||
| case 'path': | ||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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) { | ||
my-tien marked this conversation as resolved. OutdatedShow resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
| Plotly.newPlot(gd, { | ||
| data: [ | ||
| { | ||
| x: ["2015-02-01", "2015-02-02", "2015-02-03"], | ||
| y: [14, 17, 8], | ||
| mode: "line", | ||
| }, | ||
| ], | ||
| layout: { | ||
archmoj marked this conversation as resolved. OutdatedShow resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
| 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'); | ||
archmoj marked this conversation as resolved. OutdatedShow resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
| }) | ||
| .then(done, done.fail); | ||
| }); | ||
| }); | ||
| }); | ||