Skip to content

Use new light HS API for the color picker#982

Merged
emlove merged 1 commit intomasterfrom
light-api-hs-color
Mar 21, 2018
Merged

Use new light HS API for the color picker#982
emlove merged 1 commit intomasterfrom
light-api-hs-color

Conversation

@emlove
Copy link
Copy Markdown
Contributor

@emlove emlove commented Mar 11, 2018

Description

Frontend PR for home-assistant/core#11288

This PR updates the frontend to use the new hue/sat API for the light color picker.

@NovapaX
Copy link
Copy Markdown
Contributor

NovapaX commented Mar 15, 2018

Awesome, what a cleanup 👍
I have mixed feelings about removing the 'value' from the color picker element. It's currently not used in de our context of the lights. But removing it reduces the reusability of the element (for example to pick interface colors). But whatever, it's easy to put back in :)

Thinking further:
The color picker was designed with reusability in mind, so it would take and return both hsv and rgb color objects. (so it could also be used to pick colors for things where we would need a rgb value, theming/customizing for example) I know we don't use that currently. But the rgb code is not really in the way, and execution is limited by throttle.

@emlove
Copy link
Copy Markdown
Contributor Author

emlove commented Mar 15, 2018

Well right now at least it didn't have any way to actually select a value, it was just hard-coded to 100%. We'd need a new UI element to properly select a value if we wanted to use it for that. That would definitely make it more reusable though.

Also certainly not opposed to adding RGB back to this component if we want to use it, but I'd rather not ship unused code since anything included here contributes to the size of the frontend build clients need to download.

@NovapaX
Copy link
Copy Markdown
Contributor

NovapaX commented Mar 15, 2018

Agree on your points.
We'll just add it back in if needed. (long live version control)

@emlove
Copy link
Copy Markdown
Contributor Author

emlove commented Mar 20, 2018

Didn't realize we ran into merge conflicts here. I'll get this branch up-to-date tonight.

@balloob balloob force-pushed the light-api-hs-color branch from 9ace3b2 to 3218936 Compare March 21, 2018 20:49
@emlove emlove merged commit 0df4aa6 into master Mar 21, 2018
@emlove emlove deleted the light-api-hs-color branch March 21, 2018 21:35
@github-actions github-actions bot locked and limited conversation to collaborators Jul 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants