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

Allow saving the light current color as a favorite #17992

Merged
merged 2 commits into from
Sep 26, 2023
Merged

Allow saving the light current color as a favorite #17992

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
36aa711
Allow saving the light current color as a favorite
karwosts Sep 22, 2023
1bafb76
refactoring
karwosts Sep 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions src/dialogs/more-info/components/lights/dialog-light-color-favorite.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ class DialogLightColorFavorite extends LitElement {
this._entry = dialogParams.entry;
this._dialogParams = dialogParams;
this._updateModes(dialogParams.defaultMode);
if (dialogParams.add) {
this._loadCurrentColor();
}
await this.updateComplete;
}

Expand Down Expand Up @@ -90,6 +93,31 @@ class DialogLightColorFavorite extends LitElement {
: this._modes[0]);
}

private _loadCurrentColor() {
const attributes = this.stateObj!.attributes;
const color_mode = attributes.color_mode;

if (attributes.color_mode === LightColorMode.XY) {
// XY color not supported for favorites. Try to grab the hs or rgb instead.
if (attributes.hs_color) {
this._color = { hs_color: attributes.hs_color };
} else if (attributes.rgb_color) {
this._color = { rgb_color: attributes.rgb_color };
}
} else if (
color_mode === LightColorMode.COLOR_TEMP &&
attributes.color_temp_kelvin
) {
this._color = {
Copy link
Member

@bramkragten bramkragten Sep 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

We should also set the correct LightPickerMode in this._mode

Copy link
Member

Choose a reason for hiding this comment

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

Mmmm I guess _updateModes does that, I think that logic should be refactored a bit so it makes more sense (combined)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure I get what you're asking for. You want to fold updateModes and loadCurrentColor into a single function?

I don't know if I see much refactoring opportunity there, there doesn't seem like too much overlap between them.

Copy link
Member

Choose a reason for hiding this comment

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

The part that sets the _mode, I found it confusing that the _color and _mode are set at different places, as they are related, and both determined by the LightColorMode.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I tried an alternate implementation. I'm not sure if I feel this is much better or less confusing, but it does set _color and _mode more consistently.

Copy link
Member

Choose a reason for hiding this comment

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

not really I think 😅 now we are calculating the current color always and only use it when add == true

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well I do only want the current color when creating a new preset, because otherwise the current color of the bulb is not what we want when editing an existing preset.

Let me know what's the path forward.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, no I get that :-) I'll propose something tomorrow, it is really nitpicking anyway....

color_temp_kelvin: attributes.color_temp_kelvin,
};
} else if (attributes[color_mode + "_color"]) {
this._color = {
[color_mode + "_color"]: attributes[color_mode + "_color"],
} as LightColor;
}
}

private _colorChanged(ev: CustomEvent) {
this._color = ev.detail;
}
Expand Down
1 change: 1 addition & 0 deletions src/dialogs/more-info/components/lights/ha-more-info-light-favorite-colors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ export class HaMoreInfoLightFavoriteColors extends LitElement {
private _add = async () => {
const color = await showLightColorFavoriteDialog(this, {
entry: this.entry!,
add: true,
title: this.hass.localize(
"ui.dialogs.more_info_control.light.favorite_color.add_title"
),
Expand Down
1 change: 1 addition & 0 deletions src/dialogs/more-info/components/lights/show-dialog-light-color-favorite.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export interface LightColorFavoriteDialogParams {
entry: ExtEntityRegistryEntry;
title: string;
defaultMode?: LightPickerMode;
add?: boolean;
submit?: (color?: LightColor) => void;
cancel?: () => void;
}
Expand Down
Loading