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.

Sign up for GitHub

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

Jump to bottom

Fix LegendThreshold #1831

Merged
merged 3 commits into from
May 20, 2024
Merged

Fix LegendThreshold #1831

merged 3 commits into from
May 20, 2024

Conversation

ZachBarbre
Copy link
Contributor

🐛 Bug Fix

  • Changed how the labelTransform get an upper and lower value to get the color out of the threshold scale.
    This should guarantee a larger or smaller number than the upper and lower bounds is passed into the scale function to retrieve the correct color.
  • Remove the logical OR from the scale() input. It should now never be undefined and this was the cause of a bug if the the lower bound was 1 making value falsy.
  • Because I can't help but make eslint happy, added a type to the domain.

Issue #1828

hshoff
hshoff approved these changes May 20, 2024
View reviewed changes
Copy link
Member

@hshoff hshoff left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @ZachBarbre!

@hshoff hshoff merged commit 0762cf3 into airbnb:master May 20, 2024
2 checks passed
@github-actions GitHub Actions
Copy link

🎉 This PR is included in version v3.10.3 of the packages modified 🎉

@hshoff
Copy link
Member

hshoff commented May 20, 2024 •
edited
Loading

Confirmed fixed with @visx/legend@3.10.3

before: https://codesandbox.io/p/sandbox/determined-sanne-4j7x4y?file=%2Fsrc%2FApp.js%3A11%2C20
after: https://codesandbox.io/p/sandbox/determined-sanne-forked-k8zkm8?file=%2Fpackage.json%3A12%2C5-12%2C29

before after
image image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hshoff hshoff hshoff approved these changes

Assignees
No one assigned
Labels
🐛 bug patch-release @visx/legend
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ZachBarbre @hshoff