Add `ticklabelstandoff` and `ticklabelshift` to cartesian axes #7006

my-tien · 2024-05-27T15:03:20Z

These properties shift the axis tick labels in parallel or orthogonally to the axis (in pixels).

Resolves #1673

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.

Also modifies mock date_axes_period2 to test the new properties

src/plots/cartesian/axes.js

Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>

…polar, smith, ternary

my-tien · 2024-05-28T11:18:39Z

Help appreciated for these failing bundle tests (I failed to get the debugger running for this).
Is "ticks" not the correct editType for my new properties?

Firefox 126.0 (Windows 10) plot schema has valid `editType` in all attributes and containers FAILED
        Expected false to be true, 'carpet.aaxis.ticklabelshiftx: "ticks"'.
        <Jasmine>
        @webpack://plotly.js/./test/jasmine/bundle_tests/plotschema_test.js?:185:62
        exports.crawl/<@webpack://plotly.js/./src/plot_api/plot_schema.js?:102:13
        exports.crawl@webpack://plotly.js/./src/plot_api/plot_schema.js?:98:22
        exports.crawl/<@webpack://plotly.js/./src/plot_api/plot_schema.js?:105:15
        exports.crawl@webpack://plotly.js/./src/plot_api/plot_schema.js?:98:22
        assertTraceSchema/<@webpack://plotly.js/./test/jasmine/bundle_tests/plotschema_test.js?:27:25
        assertTraceSchema@webpack://plotly.js/./test/jasmine/bundle_tests/plotschema_test.js?:26:25
        @webpack://plotly.js/./test/jasmine/bundle_tests/plotschema_test.js?:183:22
        <Jasmine>

my-tien · 2024-05-28T11:47:45Z

Help appreciated for these failing bundle tests (I failed to get the debugger running for this). Is "ticks" not the correct editType for my new properties?

Firefox 126.0 (Windows 10) plot schema has valid `editType` in all attributes and containers FAILED
        Expected false to be true, 'carpet.aaxis.ticklabelshiftx: "ticks"'.
        <Jasmine>
        @webpack://plotly.js/./test/jasmine/bundle_tests/plotschema_test.js?:185:62
        exports.crawl/<@webpack://plotly.js/./src/plot_api/plot_schema.js?:102:13
        exports.crawl@webpack://plotly.js/./src/plot_api/plot_schema.js?:98:22
        exports.crawl/<@webpack://plotly.js/./src/plot_api/plot_schema.js?:105:15
        exports.crawl@webpack://plotly.js/./src/plot_api/plot_schema.js?:98:22
        assertTraceSchema/<@webpack://plotly.js/./test/jasmine/bundle_tests/plotschema_test.js?:27:25
        assertTraceSchema@webpack://plotly.js/./test/jasmine/bundle_tests/plotschema_test.js?:26:25
        @webpack://plotly.js/./test/jasmine/bundle_tests/plotschema_test.js?:183:22
        <Jasmine>

Nvm, I figured it out, print debugging ftw :)

…ks → calc carpet is a trace and therefore doesn't support editType ticks

archmoj · 2024-05-30T12:48:53Z

src/plots/cartesian/layout_attributes.js

@@ -698,6 +698,22 @@ module.exports = {
            'In other cases the default is *hide past div*.'
        ].join(' ')
    },
+    ticklabelshiftx: {


Wondering where we should put the x or y in the attribute names?
In #7005 we have xshift, here we have shiftx.

Hm, true. in #7005 my reasoning was that in the shape properties we have x0, x1 and xref. So I added xshift.

Here I had a similar reasoning, there were already properties starting with ticklabel*. And since in ticklabelxshift it is maybe hard to make out the 'x', I moved it to the end…

No strong opinion here.

IMHo we shouldn't use x and y in these attribute names.
Instead something like standoff and runoff could be used so that when switching the orientation e.g. on a colorbar from horizontal to vertical everything works without a need to adjusting these parameters.

Renamed to ticklabelrunoff (shifts in parallel to axis) and ticklabelstandoff (shifts orthogonally to axis)

test/image/mocks/date_axes_period2.json

test/plot-schema.json

src/plots/gl3d/layout/axis_attributes.js

Separate test mock will follow

And remove this feature from all non-cartesian traces. If other plots should be supported, this can be added and tested in separate PRs.

…axis.side and ticklabelposition. Standoff moves labels farther outside for outside labels and farther inside for inside labels. Runoff moves labels further along the axis range.

…abel

my-tien · 2024-06-10T10:11:12Z

Failing mapbox test appears to be unrelated to my changes. It looks good locally.

archmoj · 2024-06-10T16:22:03Z

Failing mapbox test appears to be unrelated to my changes. It looks good locally.

To fix it, please fetch upstream/master and merge it into this branch.
Thank you!

my-tien · 2024-06-12T06:22:22Z

The mapbox PR was already merged before, see here: 4ccaf73

draftlogs/7006_add.md

src/plots/cartesian/tick_label_defaults.js

gvwilson · 2024-06-26T12:57:29Z

@emilykl could you please have a look at this one?

alexcjohnson · 2024-06-26T14:00:41Z

I wonder if it would be more intuitive to specify this as something like ticklabeloffset: {x: 20, y: 10} ie a number of pixels to move the labels in the x direction (positive to the right, negative to the left) and the y direction (positive up, negative down)?

standoff is clear to me that it refers to moving orthogonal to the axis and away from the line or tick marks. But runoff I find confusing, and I can't think of another name that clearly refers to motion in that direction, which is what led me to suggest changing both of them.

archmoj · 2024-06-26T15:11:09Z

I wonder if it would be more intuitive to specify this as something like ticklabeloffset: {x: 20, y: 10} ie a number of pixels to move the labels in the x direction (positive to the right, negative to the left) and the y direction (positive up, negative down)?

standoff is clear to me that it refers to moving orthogonal to the axis and away from the line or tick marks. But runoff I find confusing, and I can't think of another name that clearly refers to motion in that direction, which is what led me to suggest changing both of them.

@alexcjohnson Thanks very much for the feedback. At the start of this PR x and y attributes are used. But I found them quite confusing to work with. Here it was suggested to use standoff.

For the second attribute (runoff) name let's see if @emilykl and @LiamConnors would have any suggestion?

…tside

…runoff

archmoj · 2024-07-02T15:10:25Z

I checked with @LiamConnors and @emilykl and they both suggested to use ticklabelshift instead of ticklabelrunoff.

draftlogs/7006_add.md

src/plots/cartesian/axes.js

archmoj · 2024-07-04T13:53:27Z

@my-tien This PR is looking very good.
@stephprobst Do you want any specific figure to be tested in this PR?

stephprobst · 2024-07-04T13:55:21Z

@archmoj : Thanks for the mention. No, no particular figure in mind. It's a very general use case.

src/plots/cartesian/layout_attributes.js

Update draftlog for renaming of `ticklabelrunoff` to `ticklabelshift` Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>

(no need to test if ax.side matches axis, because this check is done before)

draftlogs/7006_add.md

src/plots/cartesian/layout_attributes.js

archmoj · 2024-07-05T13:00:26Z

@my-tien Just few fixes needed by you here (see my recent comments) and we should be good to merge it today!

Improve draftlog text for ticklabelstandoff and ticklabelshift Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>

…d move outside and outside ticks could move inside

archmoj

💃

Add property axis.ticklabelshiftx and axis.ticklabelshifty

6c81892

Also modifies mock date_axes_period2 to test the new properties

archmoj reviewed May 27, 2024

View reviewed changes

src/plots/cartesian/axes.js Outdated Show resolved Hide resolved

my-tien and others added 4 commits May 28, 2024 09:52

makeTransTickLabelFn: default vals for ax.ticklabelshiftx/y

ad4509b

Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>

Add draftlog for PR 7006

fa50121

Add ticklabelshiftx/y property to colorbar, indicator, gl3d, carpet, …

d33e47c

…polar, smith, ternary

baseline image for updated date_axes_period2 using ticklabelshiftx/y

3dcb2aa

ticklabelshiftx/y: valType number → integer. for carpet: editType tic…

facdad3

…ks → calc carpet is a trace and therefore doesn't support editType ticks

archmoj added feature something new community community contribution status: reviewable labels May 28, 2024

archmoj reviewed May 30, 2024

View reviewed changes

archmoj reviewed May 31, 2024

View reviewed changes

test/image/mocks/date_axes_period2.json Outdated Show resolved Hide resolved

archmoj reviewed May 31, 2024

View reviewed changes

test/plot-schema.json Outdated Show resolved Hide resolved

archmoj reviewed May 31, 2024

View reviewed changes

test/plot-schema.json Outdated Show resolved Hide resolved

archmoj reviewed May 31, 2024

View reviewed changes

src/plots/gl3d/layout/axis_attributes.js Outdated Show resolved Hide resolved

my-tien added 6 commits June 3, 2024 17:07

Revert ticklabelshiftx/y change to mock date_axes_period2

ecf4787

Separate test mock will follow

Replace ticklabelshiftx/y with ticklabelrunoff and ticklabelstandoff

2b9c90c

And remove this feature from all non-cartesian traces. If other plots should be supported, this can be added and tested in separate PRs.

Adjust ticklabelrunoff/ticklabelstandoff behavior depending on axis, …

f06a81e

…axis.side and ticklabelposition. Standoff moves labels farther outside for outside labels and farther inside for inside labels. Runoff moves labels further along the axis range.

baseline test mock for ticklabelrunoff/ticklabelstandoff

57c82ee

Merge remote-tracking branch 'origin-plotly/master' into shift_axis_l…

4ccaf73

…abel

baseline image for mock zzz_ticklabelrunoff_standoff

4b62652

archmoj added the status: has TODOs label Jun 10, 2024

archmoj reviewed Jun 12, 2024

View reviewed changes

draftlogs/7006_add.md Outdated Show resolved Hide resolved

archmoj reviewed Jun 12, 2024

View reviewed changes

src/plots/cartesian/tick_label_defaults.js Outdated Show resolved Hide resolved

archmoj removed the status: has TODOs label Jun 12, 2024

gvwilson assigned emilykl Jun 26, 2024

my-tien added 3 commits June 26, 2024 17:14

correct var name. it's not ticks inside/outside, but labels inside/ou…

e8f88ba

…tside

Clarify the direction of movement for ticklabelstandoff and ticklabel…

d9210da

…runoff

update plot-schema as well

e33270b

archmoj changed the title ~~Add ticklabelstandoff and ticklabelrunoff to cartesian axes~~ Add ticklabelstandoff and ticklabelshift to cartesian axes Jul 2, 2024

Rename ticklabelrunoff → ticklabelshift

ec5df56

gvwilson assigned archmoj and unassigned gvwilson and emilykl Jul 3, 2024

archmoj reviewed Jul 4, 2024

View reviewed changes

draftlogs/7006_add.md Outdated Show resolved Hide resolved

archmoj reviewed Jul 4, 2024

View reviewed changes

src/plots/cartesian/axes.js Outdated Show resolved Hide resolved

archmoj reviewed Jul 4, 2024

View reviewed changes

src/plots/cartesian/layout_attributes.js Show resolved Hide resolved

my-tien and others added 2 commits July 5, 2024 07:01

Update draftlogs/7006_add.md

e3b0baa

Update draftlog for renaming of `ticklabelrunoff` to `ticklabelshift` Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>

Simplify code for standoff sign

23a21c1

(no need to test if ax.side matches axis, because this check is done before)

archmoj reviewed Jul 5, 2024

View reviewed changes

draftlogs/7006_add.md Outdated Show resolved Hide resolved

archmoj reviewed Jul 5, 2024

View reviewed changes

draftlogs/7006_add.md Outdated Show resolved Hide resolved

archmoj reviewed Jul 5, 2024

View reviewed changes

src/plots/cartesian/layout_attributes.js Outdated Show resolved Hide resolved

my-tien and others added 2 commits July 5, 2024 15:02

Update draftlogs/7006_add.md

6715564

Improve draftlog text for ticklabelstandoff and ticklabelshift Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>

improve ticklabelstandoff description and note that inside ticks coul…

26315dc

…d move outside and outside ticks could move inside

archmoj approved these changes Jul 5, 2024

View reviewed changes

archmoj merged commit 8b1f5bd into plotly:master Jul 5, 2024
1 check passed

FlorentinTh mentioned this pull request Aug 9, 2024

[Snyk] Upgrade plotly.js-cartesian-dist-min from 2.12.1 to 2.34.0 FlorentinTh/LE2ML-GUI#773

Open

guaiya0507 mentioned this pull request Aug 9, 2024

[Snyk] Upgrade plotly.js-dist-min from 2.33.0 to 2.34.0 guaiya0507/mytool#3

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `ticklabelstandoff` and `ticklabelshift` to cartesian axes #7006

Add `ticklabelstandoff` and `ticklabelshift` to cartesian axes #7006

my-tien commented May 27, 2024 •

edited

Loading

my-tien commented May 28, 2024 •

edited

Loading

my-tien commented May 28, 2024 •

edited

Loading

archmoj May 30, 2024

my-tien May 31, 2024 •

edited

Loading

archmoj May 31, 2024

my-tien Jun 3, 2024

my-tien commented Jun 10, 2024

archmoj commented Jun 10, 2024

my-tien commented Jun 12, 2024

gvwilson commented Jun 26, 2024

alexcjohnson commented Jun 26, 2024

archmoj commented Jun 26, 2024

archmoj commented Jul 2, 2024

archmoj commented Jul 4, 2024

stephprobst commented Jul 4, 2024

archmoj commented Jul 5, 2024

archmoj left a comment

Add ticklabelstandoff and ticklabelshift to cartesian axes #7006

Add ticklabelstandoff and ticklabelshift to cartesian axes #7006

Conversation

my-tien commented May 27, 2024 • edited Loading

my-tien commented May 28, 2024 • edited Loading

my-tien commented May 28, 2024 • edited Loading

archmoj May 30, 2024

Choose a reason for hiding this comment

my-tien May 31, 2024 • edited Loading

Choose a reason for hiding this comment

archmoj May 31, 2024

Choose a reason for hiding this comment

my-tien Jun 3, 2024

Choose a reason for hiding this comment

my-tien commented Jun 10, 2024

archmoj commented Jun 10, 2024

my-tien commented Jun 12, 2024

gvwilson commented Jun 26, 2024

alexcjohnson commented Jun 26, 2024

archmoj commented Jun 26, 2024

archmoj commented Jul 2, 2024

archmoj commented Jul 4, 2024

stephprobst commented Jul 4, 2024

archmoj commented Jul 5, 2024

archmoj left a comment

Choose a reason for hiding this comment

Add `ticklabelstandoff` and `ticklabelshift` to cartesian axes #7006

Add `ticklabelstandoff` and `ticklabelshift` to cartesian axes #7006

my-tien commented May 27, 2024 •

edited

Loading

my-tien commented May 28, 2024 •

edited

Loading

my-tien commented May 28, 2024 •

edited

Loading

my-tien May 31, 2024 •

edited

Loading