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

[BasicUI] Implement new sitemap element Colortemperaturepicker #2851

Merged
merged 1 commit into from
Nov 9, 2024

Conversation

lolodomo
Copy link
Contributor

@lolodomo lolodomo commented Nov 1, 2024

@lolodomo lolodomo added enhancement New feature or request basic ui Basic UI labels Nov 1, 2024
@lolodomo lolodomo requested a review from kaikreuzer as a code owner November 1, 2024 13:59
@lolodomo lolodomo marked this pull request as draft November 1, 2024 13:59
@lolodomo lolodomo force-pushed the basicui_colortemperaturepicker branch 2 times, most recently from 2450372 to ff1b4c7 Compare November 1, 2024 14:29
@lolodomo lolodomo changed the title [BasicUI] Implement new sitemap element colortemperaturepicker [BasicUI] Implement new sitemap element Colortemperaturepicker Nov 1, 2024
@lolodomo lolodomo force-pushed the basicui_colortemperaturepicker branch 5 times, most recently from 913160a to 203c218 Compare November 2, 2024 14:43
@lolodomo
Copy link
Contributor Author

lolodomo commented Nov 2, 2024

Here is the rendering of the new widget:

image

When clicking on the button, the following pop-up is displayed:

image

@mueller-ma
Copy link
Member

I'd prefer to have a preview of the state as actual color and as Kelvin, as I (and probably many other people) don't know how 4500 K look like. Suggestion:

382481287-49ab7a5d-ed81-4611-9901-c056b156c15e

@lolodomo
Copy link
Contributor Author

lolodomo commented Nov 2, 2024

I'd prefer to have a preview of the state as actual color and as Kelvin, as I (and probably many other people) don't know how 4500 K look like. Suggestion:

Very good idea, I will do that.
I will also keep the value.

Edit: it could even replace the button with the icon. The button would contain the current color.

@lolodomo
Copy link
Contributor Author

lolodomo commented Nov 3, 2024

Conversion from Kelvin to RGB is not done in frontend. Backend is providing 21 RGB values to frontend covering the min/max range (to create the linear gradient).
When the page is first opened, I can set the proper color as preview but when item state is updated, I can only use one of the 21 colours, the most approaching.
Of course, I can also increase this number of values.

The idea of this "preview" color could also be applied to the Colorpicker widget.

@andrewfg
Copy link

andrewfg commented Nov 3, 2024

The idea of this "preview" color could also be applied to the Colorpicker widget.

Good idea!

@andrewfg
Copy link

andrewfg commented Nov 3, 2024

when item state is updated, I can only use one of the 21 colours, the most approaching.

I guess there are two sub- cases..

  1. when the state is updated from the picker itself
  2. when the state is updated by notification from OH core

@lolodomo
Copy link
Contributor Author

lolodomo commented Nov 3, 2024

When the color is updated from the picker itself, a command is sent to the item. The widget is updated when it receives a feedback state from the item.

@lolodomo lolodomo force-pushed the basicui_colortemperaturepicker branch 8 times, most recently from 8c29be0 to 4093b0d Compare November 9, 2024 12:42
@lolodomo lolodomo marked this pull request as ready for review November 9, 2024 13:11
@lolodomo
Copy link
Contributor Author

lolodomo commented Nov 9, 2024

This is now ready for review.

Here is the rendering of the widget:

image

image

And the new pop-up:

image

Related to openhab/openhab-core#3891

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
@lolodomo lolodomo force-pushed the basicui_colortemperaturepicker branch from 4093b0d to 7de9493 Compare November 9, 2024 13:25
@kaikreuzer kaikreuzer merged commit 9e1a25d into openhab:main Nov 9, 2024
5 checks passed
@kaikreuzer kaikreuzer added this to the 4.3 milestone Nov 9, 2024
@lolodomo lolodomo deleted the basicui_colortemperaturepicker branch November 9, 2024 16:40
@mueller-ma
Copy link
Member

@lolodomo The "ok" button in the popup suggests that the value is updated when the button is pressed. But it seems that it's updated as soon as the slider is changed. Maybe rename the button to "close"?

@lolodomo
Copy link
Contributor Author

lolodomo commented Dec 2, 2024

Yes, same behaviour as for the color picker. I agree that "Close" would be more appropriate in both pop-ups.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
basic ui Basic UI enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants