You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "Quatters (via GitHub)" <gi...@apache.org> on 2025/03/18 12:27:25 UTC
[PR] [WIP] fix(chart control): Change default of "Y Axis Title Margin" [superset]
Quatters opened a new pull request, #32720:
URL: https://github.com/apache/superset/pull/32720
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
Changes the default value of the "Y Axis Title Margin" from 15 to 30. It was reported that previous default often caused overlapping with axis value text.
Fixes #32528
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
| Before | After |
|--------|-------|
| <img width="300" src="https://github.com/user-attachments/assets/dcdff19e-20e5-4dcf-8593-28c8999fd4b6"> | <img width="300" src="https://github.com/user-attachments/assets/552e6742-6695-4bd4-ae4d-8fad21f07abf"> |
### TESTING INSTRUCTIONS
1. Create Timeseries chart, e.g. Area Chart
2. Go to Customize tab
3. Check that "Y Axis Title Margin" is 30 by default
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [x] Has associated issue: https://github.com/apache/superset/issues/32528
- [ ] Required feature flags:
- [x] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(chart control): Change default of "Y Axis Title Margin" [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on PR #32720:
URL: https://github.com/apache/superset/pull/32720#issuecomment-2734782117
@Vitor-Avila should only affect new charts
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(chart control): Change default of "Y Axis Title Margin" [superset]
Posted by "Quatters (via GitHub)" <gi...@apache.org>.
Quatters commented on PR #32720:
URL: https://github.com/apache/superset/pull/32720#issuecomment-2740128744
@mistercrunch thanks for the hint! I fixed tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(chart control): Change default of "Y Axis Title Margin" [superset]
Posted by "Vitor-Avila (via GitHub)" <gi...@apache.org>.
Vitor-Avila merged PR #32720:
URL: https://github.com/apache/superset/pull/32720
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(chart control): Change default of "Y Axis Title Margin" [superset]
Posted by "Vitor-Avila (via GitHub)" <gi...@apache.org>.
Vitor-Avila commented on PR #32720:
URL: https://github.com/apache/superset/pull/32720#issuecomment-2734841450
great, thanks for confirming @mistercrunch and @Quatters 🙌 I thought so too, just wanted to be extra safe. I re-triggered the failed jobs
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(chart control): Change default of "Y Axis Title Margin" [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on PR #32720:
URL: https://github.com/apache/superset/pull/32720#issuecomment-2740645465
Not big on these tests targeting pixels, but given it's the canvas rendering engine I'm not sure if there's a better way. Guessing we could switch the svg rendering engine of echarts and improve those tests to target svg component more explicitely. Not a top-priority but would be nice. I fought those tests in the past around some auto-scrolling action in cypress that would make the clicks not work.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(chart control): Change default of "Y Axis Title Margin" [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on PR #32720:
URL: https://github.com/apache/superset/pull/32720#issuecomment-2734492158
Thanks for the PR! I restarted two cypress job in CI that appeared to be flaking.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(chart control): Change default of "Y Axis Title Margin" [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on PR #32720:
URL: https://github.com/apache/superset/pull/32720#issuecomment-2737128397
My guess is there's some sort of "pixel-perfect" click in these tests that the new margin default offsets. Looking on what's failing `drillby.test.ts` and `drilltodetail.test.ts` these tests right-click into a chart to trigger events. I hate to say this, but you probably want to look for right-click events in those tests and shift them to the right a little (maybe by 15/2 pixels?).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(chart control): Change default of "Y Axis Title Margin" [superset]
Posted by "Vitor-Avila (via GitHub)" <gi...@apache.org>.
Vitor-Avila commented on PR #32720:
URL: https://github.com/apache/superset/pull/32720#issuecomment-2734742083
hey @Quatters,
If had a chart created with the **Y Axis Title Margin** config set to the default value (have never touched it) would this code-change affect those too? Or would it only affect new charts?
I think this change makes sense, just trying to confirm if we need to have any migration for existing charts that might not have this property set or etc.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(chart control): Change default of "Y Axis Title Margin" [superset]
Posted by "Quatters (via GitHub)" <gi...@apache.org>.
Quatters commented on PR #32720:
URL: https://github.com/apache/superset/pull/32720#issuecomment-2736739889
Any ideas why these failing? Does not seem it's related to changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(chart control): Change default of "Y Axis Title Margin" [superset]
Posted by "Quatters (via GitHub)" <gi...@apache.org>.
Quatters commented on PR #32720:
URL: https://github.com/apache/superset/pull/32720#issuecomment-2734837650
@mistercrunch looks like jobs are still flaking.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(chart control): Change default of "Y Axis Title Margin" [superset]
Posted by "Quatters (via GitHub)" <gi...@apache.org>.
Quatters commented on PR #32720:
URL: https://github.com/apache/superset/pull/32720#issuecomment-2734811831
@Vitor-Avila hi!
Existing charts wouldn't be affected.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(chart control): Change default of "Y Axis Title Margin" [superset]
Posted by "Vitor-Avila (via GitHub)" <gi...@apache.org>.
Vitor-Avila commented on PR #32720:
URL: https://github.com/apache/superset/pull/32720#issuecomment-2736870167
it could just be a flaky test -- I triggered them again
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(chart control): Change default of "Y Axis Title Margin" [superset]
Posted by "Vitor-Avila (via GitHub)" <gi...@apache.org>.
Vitor-Avila commented on PR #32720:
URL: https://github.com/apache/superset/pull/32720#issuecomment-2740418293
Awesome -- that did the trick! Thanks @Quatters
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org