Skip to content

Fix setting of HSL values from hexa input #54

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

Merged
merged 2 commits into from
Jul 24, 2023
Merged

Conversation

bytesnz
Copy link
Contributor

@bytesnz bytesnz commented Oct 24, 2021

I noticed there was an issue setting the saturation and lightness values when a hexa value is input into the hexa input when the color format was set to HSL.

Simple fix of changing setHexa() to use updateHSX() instead of RGBtoHSV().

Change setHexa to use updateHSX instead of RGBtoHSV as wasn't updating properly
when format was set to HSL
@bsmth bsmth self-requested a review June 29, 2023 15:43
@bsmth
Copy link
Member

bsmth commented Jul 13, 2023

Hi @bytesnz - thanks a lot for the submission, I don't see the difference with the changes. Would you mind adding some steps to reproduce so I can follow along?

Thanks a lot! :)

@bytesnz
Copy link
Contributor Author

bytesnz commented Jul 17, 2023

Took a bit to figure out/remember what the problem was.

When you change the hex value in the input box, the lightness value doesn't get updated

image

To replicate:

  1. Change the HSL values to 350 98 80
  2. Change the hex value to #4B000D

Observed:

  • Hue and saturation change, but lightness does not

Expected:
- HSL should now be 161 81 54

@bytesnz
Copy link
Contributor Author

bytesnz commented Jul 17, 2023

Code has been updated since I committed, so there is a conflict now. Will update it.

@bsmth
Copy link
Member

bsmth commented Jul 20, 2023

Thanks a lot for coming back to it and giving some context. I'll wait for your updates re: merge conflict, feel free to ping me for a look when you're ready

@bytesnz
Copy link
Contributor Author

bytesnz commented Jul 22, 2023

Merged 🙂

@bsmth bsmth self-assigned this Jul 24, 2023
@bsmth
Copy link
Member

bsmth commented Jul 24, 2023

Thanks @bytesnz - I'm going to take a look again shortly

Copy link
Member

@bsmth bsmth left a comment

Choose a reason for hiding this comment

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

Very good, tnx, I tested with:

  • #8471BB
  • #805BEF
  • #000000
  • #FF0000

And I can confirm the Lightness value is being updated properly.

Thank you @bytesnz for your fix - I'm glad it could land eventually :)

@bsmth bsmth merged commit 284370d into mdn:main Jul 24, 2023
@bytesnz
Copy link
Contributor Author

bytesnz commented Jul 24, 2023

Awesome. Thanks @bsmth 🎉

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants