plotly.js
plotly.js copied to clipboard
Add `ticklabelstandoff` and `ticklabelrunoff` to cartesian axes
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.
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>
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 :)
Failing mapbox test appears to be unrelated to my changes. It looks good locally.
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!
The mapbox PR was already merged before, see here: 4ccaf7381592e719ff31c3947e0765fed984aa70
Looking good to me. I ask @alexcjohnson and @LiamConnors for their reviews as well as approving the attribute names and descriptions. Thanks.
The mapbox PR was already merged before, see here: 4ccaf73
It's no longer the mapbox test which is failing. The image test I rerun from start and it passed.
But please have a look at this test to see we have to pull @flaky tag to it after these changes?
https://github.com/plotly/plotly.js/blob/caf32e57cfdbd370076b926f84c114093f62f0b9/test/jasmine/tests/polar_test.js#L450
The mapbox PR was already merged before, see here: 4ccaf73
It's no longer the
mapboxtest which is failing. The image test I rerun from start and it passed. But please have a look at this test to see we have to pull@flakytag to it after these changes?https://github.com/plotly/plotly.js/blob/caf32e57cfdbd370076b926f84c114093f62f0b9/test/jasmine/tests/polar_test.js#L450
Thanks @archmoj when I run the test locally, it doesn't fail, so I think it is indeed flaky.
it('should be able to restyle radial axis title', function(done) {
Thanks for testing out.
Please add @flaky tag to the start of test description like this:
it('@flaky should be able to restyle radial axis title', function(done) {
I think this could also help resolve #1673. :tada: which is one the oldest open issues now.
Please add Resolves #1673 to the PR description.
Thank you!
Please revert 6ea832f then fetch upstream/master and merge it into this branch so that the CI tests pass.
cc: #7030.
I like the attribute names. But @LiamConnors what do you think of new attribute names here? cc: @emilykl
@emilykl could you please have a look at this one?
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.
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)?
standoffis clear to me that it refers to moving orthogonal to the axis and away from the line or tick marks. ButrunoffI 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?
I checked with @LiamConnors and @emilykl and they both suggested to use ticklabelshift instead of ticklabelrunoff.
@my-tien This PR is looking very good. @stephprobst Do you want any specific figure to be tested in this PR?
@archmoj : Thanks for the mention. No, no particular figure in mind. It's a very general use case.
@my-tien Just few fixes needed by you here (see my recent comments) and we should be good to merge it today!