property x0shift, x1shift, y0shift, y1shift for adjusting the shape/selection coordinates

[my-tien]

trafficstars

This property adjusts the shape/selection coordinates if xref/yref references a (multi-)category axis.

A shape at position x0/x1 = "A"

with x_shift = 0.5

Disclaimer

I am required to add that…

the software is provided "as is", without warranty of any kind, express or implied, including but not limited to the warranties of merchantability, fitness for a particular purpose and noninfringement. in no event shall the authors or copyright holders be liable for any claim, damages or other liability, whether in an action of contract, tort or otherwise, arising from, out of or in connection with the software or the use or other dealings in the software.

[archmoj]

Interesting PR for shapes and selections. But I started wondering perhaps we should add a categoryshift option to Cartesian axes instead?

[archmoj]

I am wondering if adding a categoryselectionstart, categoryselectionend, categoryshapestart and categoryshapeend to cartesian axes is a better option?

[archmoj]

Let's ask @alexcjohnson to review :mag_right:

[my-tien]

I am wondering if adding a categoryselectionstart, categoryselectionend, categoryshapestart and categoryshapeend to cartesian axes is a better option?

Here is a PR with this suggestion for shapes only: #7010 (I actually only included selection in this PR, because it reuses shape properties.)

[archmoj]

Considering https://github.com/plotly/plotly.js/pull/7010#issuecomment-2147726890 discussion, two attributes are needed for start and the end i.e. instead of xshift which is already implemented we need x0shift and x1shift.

[archmoj]

#6347 seems related to this PR. No?

[stephprobst]

#6347 seems related to this PR. No?

Yes. The current PR may be part of the groundwork required for #6347. Do you think any additional changes are required to merge the current PR though?

[archmoj]

#6347 seems related to this PR. No?

Yes. The current PR may be part of the groundwork required for #6347. Do you think any additional changes are required to merge the current PR though?

@stephprobst In your use cases do you want to move the coordinates by either -0.5 or +0.5 only? Or you want to have control for any ratio e.g. between -1 and +1? Also we need to know if you are interested to snap to those values or just moving without any rounding?

[stephprobst]

#6347 seems related to this PR. No?

Yes. The current PR may be part of the groundwork required for #6347. Do you think any additional changes are required to merge the current PR though?

@stephprobst In your use cases do you want to move the coordinates by either -0.5 or +0.5 only? Or you want to have control for any ratio e.g. between -1 and +1? Also we need to know if you are interested to snap to those values or just moving without any rounding?

In our use case moving the coordinates by either -0.5 or +0.5 is sufficient. The rationale is, that users want to add a vertical line separating two categories and they want the line to move with the preceding category in case the category order changes.

We also don't have a need for snap. In our use case the provided values would be exact.

[archmoj]

Thanks for the feedback @stephprobst. It's good to know that -0.5 and 0.5 values are sufficient in your use cases. With that I think rounding start and end values gives us a better API. Also in case of selection (and new selection) I think rounding would be useful both for rectangular selections as well as lasso selection. What do you think?

[my-tien]

Do you mean that plotly should internally round the shift values provided by the user to the closest 0.5 increment/decrement? Wouldn't it then be a little confusing to allow any float value between -1 and 1 in the schema instead of only allowing -1, -0.5, 0, 0.5 and 1?

[stephprobst]

On the discussion of floating point precision vs. rounding: We currently only have a use case for +/- 0.5, but I'm not sure other floating point values won't be useful in the future. At the same time I see the value of rounding. Would it be possible to say we do both? Finish the current PR with the floating point precision and add additional keywords roundup and rounddown at a later stage?

On the discussion of "add property to axis" vs. "add property to shape": We would strongly prefer this property to be available on the individual shape, as it may very well be, that the user wants to combine a vertical line that separates categories with other shapes, that should be linked to the center of the category. Thanks!

[my-tien]

As far as I understand it, it only makes sense to add rounding logic for interactively created shapes/selections whereas for shapes/selections defined via code I would expect the exact value to be used.

[archmoj]

@my-tien There are not conflicts with master; but please fetch & merge upstream/master into this branch.

[archmoj]

@stephprobst Do you have a use case for adding these options to selections? If no, I'd suggest not to implement it in this PR.

[stephprobst]

@stephprobst Do you have a use case for adding these options to selections? If no, I'd suggest not to implement it in this PR.

@archmoj: No. We're not using selections at the moment and probably won't in the foreseeable future. I'm ok with skipping the implementation in this PR.

[archmoj]

@stephprobst Do you have a use case for adding these options to selections? If no, I'd suggest not to implement it in this PR.

@archmoj: No. We're not using selections at the moment and probably won't in the foreseeable future. I'm ok with skipping the implementation in this PR.

@my-tien Let's simplify this PR by adding shift options to shapes only i.e. no newshape and no selections with no rounding. Thank you!

[archmoj]

Thanks for the simplification. Please update the PR title and description. Also zzz_shape_shift_vertical image test is failing now.

[archmoj]

@stephprobst Do you have a use case for date axes? If so it could be possible to add these options for other axis types.

[stephprobst]

@stephprobst Do you have a use case for date axes? If so it could be possible to add these options for other axis types.

@archmoj No, I don't think this is relevant for other axes. The rationale for this property is to allow exact positioning of a shape based on the axis value. For categorical axes this is needed, because it allows the combination of categorical and numerical values (e.g. country "Canada" plus a shift of 0.5 to position the shape at the end of the categories interval on the axis). On numerical or date axes this isn't really needed, since the user can directly select the precise numerical or date value (e.g. "2023-12-31" to position a shape at the end of year 2023).

My proposal would be to skip the implementation for date and numeric axes.

[archmoj]

When shape.editable is set to true, clicking the shape and editing the position of a vertex result in the wrong positioning of the updated shape.

[archmoj]

Please also test texttemplate on your mocks to show the slopes similar to those in the text_on_shapes_texttemplate mock. Thank you!

[my-tien]

Please also test texttemplate on your mocks to show the slopes similar to those in the text_on_shapes_texttemplate mock. Thank you!

I noticed that I didn't account for the shift for the texttemplate. Fixed that and added a test shape with slope and xcenter.

[my-tien]

When shape.editable is set to true, clicking the shape and editing the position of a vertex result in the wrong positioning of the updated shape.

Fixed