Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: use domainMin and domainMax to set scale padding #3906

Merged
merged 4 commits into from Apr 22, 2024
Merged

Conversation

lsh
Copy link
Member

@lsh lsh commented Apr 22, 2024

Co-authored with @Jami159

Previously, padding was only set if a domain array was used, but it didn't use domainMin or domainMax. Rearranging this code means that the new calculation occurs on the already clamped domain.

Editor link

Signed-off-by: Lukas Hermann <lukas.hermann@databricks.com>
@lsh lsh requested a review from a team as a code owner April 22, 2024 19:39
Signed-off-by: Lukas Hermann <lukas.hermann@databricks.com>
@lsh lsh requested review from kanitw and domoritz April 22, 2024 20:37
Copy link
Member

@domoritz domoritz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove the example since the code quality isn't good enough for the public gallery. Instead, just put a link to the editor into the pull request. Then add a small unit test just to make sure that this doesn't break again in the future.

@lsh lsh requested a review from domoritz April 22, 2024 21:17
Co-authored-by: Al-Jami Ismail <aljami1379@gmail.com>
@lsh lsh merged commit da448d0 into main Apr 22, 2024
4 checks passed
@lsh lsh deleted the lsh/domain-min-max-pad branch April 22, 2024 22:02
@lsh lsh mentioned this pull request May 8, 2024
lsh added a commit that referenced this pull request May 10, 2024
**docs**
* remove Google Analytics. (via #3894) (Thanks @domoritz!)

**monorepo**
* switch to ESLint flat config file. (via #3920) (Thanks @lsh!)
* update node versions in ci. (via #3915) (Thanks @domoritz!)

**vega-encode**
* use domainMin and domainMax to set scale padding. (via #3906) (Thanks
@Jami159 & @lsh!)

**vega-scale**
* Add observable10 palette. (via #3843) (Thanks @mcnuttandrew!)
* Respect tickMinStep for binned scale. (via #3904) (Thanks @alexxu-db!)

**vega-scenegraph**
* Revert TooltipHideEvent to MouseOutEvent to fix events on mobile. (via
#3909) (Thanks @kadamwhite!)

**vega-typings**
* Add observable10 palette. (via #3843) (Thanks @mcnuttandrew!)

**vega-view**
* turn off all handlers in View.finalize() to fix memory leak. (via
#3896) (Thanks @cmerrick!)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants