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

ColorPicker - fix - hue changes to red after move lightness to white #2942

Merged
merged 3 commits into from Feb 11, 2024

Conversation

Inbal-Tish
Copy link
Collaborator

@Inbal-Tish Inbal-Tish commented Feb 7, 2024

Description

The bug: When moving the lightness slider to white, a #fffff color is set back to the component creating a color of h=0,s=0,l=0 values.
Now when moving the hue slider the color remains grey as the saturation is 0, and when moving the lightness slider the color remains red as the hue is 0.
The solution: Passing the color as HSL instead of converting it to hex allows the new color to keep the values of the hue and saturation in the transition so when moving either slider the color changes from the last values and not from the white color values.

Changelog

ColorPicker - fix - hue changes to red after moving lightness to white

Additional info

@Inbal-Tish Inbal-Tish changed the title ColorPicker - fix types on subcomponents ColorPicker - fix - hue changes to red after move lightness to white Feb 7, 2024
Copy link
Contributor

@nitzanyiz nitzanyiz left a comment

Choose a reason for hiding this comment

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

Looks good. Maybe we should create a type for the string | HSLA wdyt?

@Inbal-Tish
Copy link
Collaborator Author

Looks good. Maybe we should create a type for the string | HSLA wdyt?

Feels forced...

Looks good. Maybe we should create a type for the string | HSLA wdyt?

Feels a bit forced, no?

@Inbal-Tish Inbal-Tish merged commit 634df31 into master Feb 11, 2024
1 check passed
@Inbal-Tish Inbal-Tish deleted the fix/ColorPicker_types branch February 11, 2024 11:23
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