Ensure consistent light state icon brightness #14740

spacegaier · 2022-12-13T10:35:01Z

Breaking change

Proposed change

The coloring was off. Some did not use the RGB colors at all, others ignored the brightness.

Now only the tile card is off, where honestly I have no clear idea what it should do in this case design-wise, so I left that one alone.

Before

After

Also note that in the "light off" state the tile card is also inconsistent, which also looks like a bug for the end user.

@piitaya @matthiasdebaat Can you please comment on the tile card points? I think consistency is really key here (more important than any actual used colors IMHO).

There is also a similar, although more narrow PR I just saw: #14724

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New feature (thank you!)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Example configuration

Additional information

This PR fixes or closes issue: fixes #
This PR is related to issue or discussion:
Link to documentation pull request:

Checklist

The code change is tested and works locally.
There is no commented out code in this PR.
Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated for www.home-assistant.io

spacegaier · 2022-12-13T10:35:59Z

src/panels/lovelace/cards/hui-entity-card.ts

+  private _computeBrightness(stateObj: HassEntity | LightEntity): string {
+    if (stateObj.attributes.brightness && stateActive(stateObj)) {
+      const brightness = stateObj.attributes.brightness;
+      return `brightness(${(brightness + 245) / 5}%)`;
+    }
+    return "";
+  }
+


I don't really like this code duplication, but was already present for the _computeColor as well, so did not change that in this PR.

Yep we can change that in another PR 🙂

piitaya · 2022-12-13T10:43:18Z

There is some changes for the tile card :

no brightness because not only the icon is colored but also the shape and the slider (if set)
some transformations on the RGB value to avoid this case : ICON State Color for colored light tracks color when light is white #14696

There is also some weird think about rgb color on entity card : RGB value are displayed even if the state_color is undefined.

We haven't discuss this point. What do you think?

spacegaier · 2022-12-13T10:52:23Z

no brightness because not only the icon is colored but also the shape and the slider (if set)

Which means one cannot really use the tile card next to other existing UI elements since that looks weird/inconsistent. So a user either will have to build a dashboard with only tile cards or only non-tile cards.

* some transformations on the RGB value to avoid this case : [ICON State Color for colored light tracks color when light is white #14696](https://github.com/home-assistant/frontend/issues/14696)

Yes that was always an issue, which is one reason why I use a dark theme 😆 so that never comes into play (plus a lot less power consumption from the wall tablets).

There is also some weird think about rgb color on entity card : RGB value are displayed even if the state_color is undefined.

I need to check that, but that sounds buggy to me.

spacegaier · 2022-12-13T10:53:59Z

Reg. the last point: Did you mean entities and not entity card? Because for the latter everything behaves correctly on my end.

piitaya · 2022-12-13T10:57:04Z

We have rules like this in state-badge (used by entities), entity and button :

state_color || (domain === "light" && state_color !== false)

So :

state_color = undefined => light colored, others domains not colored
state_color = true => all domains colored
state_color = false => all domains not colored

3 states boolean 😅

piitaya · 2022-12-13T10:58:56Z

We have also differences between tile card and entities card : the off state is grey on tile card and it use the default color for others card.

spacegaier · 2022-12-13T11:01:09Z

We have rules like this in state-badge (used by entities), entity and button :
state_color || (domain === "light" && state_color !== false)
So :
* state_color = undefined => light colored, others domains not colored

* state_color = true => all domains colored

* state_color = false => all domains not colored
3 states boolean 😅

Yes, I just saw that coding as well, but I think that has been that way for a long time, so no sense in changing that now. Probably not ideal, but the user can have full control here and with this PR we show the same consistent colors.

spacegaier · 2022-12-13T11:04:12Z

We have also differences between tile card and entities card : the off state is grey on tile card and it use the default color for others card.

Yes I mentioned that in my PR description. I personally don't like that, since I strive for consistency to clearly communicate the state to the user.

One could argue that gray is clearer here for lights, since the default blue could on first glance mean a blue LED (although in most cases you will see also a state indicator either in text form, or a toggle button, round slider, ...). However, blue was always used as the default off state (and still is with the current theming changes). So the tile card goes against the status quo here (with the exclamation mark icon to differentiate for unknown/unavailable), so again inconsistent 😬 .

piitaya · 2022-12-13T13:31:56Z

src/panels/lovelace/cards/hui-entity-card.ts

+  private _computeBrightness(stateObj: HassEntity | LightEntity): string {
+    if (stateObj.attributes.brightness && stateActive(stateObj)) {
+      const brightness = stateObj.attributes.brightness;
+      return `brightness(${(brightness + 245) / 5}%)`;
+    }
+    return "";
+  }
+


Yep we can change that in another PR 🙂

Ensure consistent light state icon brightness

f66c7f1

spacegaier added this to the 2022.12 milestone Dec 13, 2022

home-assistant bot added the cla-signed label Dec 13, 2022

spacegaier commented Dec 13, 2022

View reviewed changes

piitaya previously approved these changes Dec 13, 2022

View reviewed changes

Merge branch 'dev' into consistent-light-icon

0cb52c7

piitaya dismissed their stale review via 0cb52c7 December 13, 2022 13:31

piitaya previously approved these changes Dec 13, 2022

View reviewed changes

Merge branch 'dev' into consistent-light-icon

e73ebdd

piitaya dismissed their stale review via e73ebdd December 13, 2022 15:51

piitaya added 2 commits December 13, 2022 16:53

Update hui-entity-card.ts

8ae7048

Update hui-entity-card.ts

4af5c66

bramkragten approved these changes Dec 13, 2022

View reviewed changes

bramkragten merged commit 9c27bb3 into home-assistant:dev Dec 13, 2022

spacegaier deleted the consistent-light-icon branch December 13, 2022 17:05

github-actions bot locked and limited conversation to collaborators Dec 13, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ensure consistent light state icon brightness #14740

Ensure consistent light state icon brightness #14740

spacegaier commented Dec 13, 2022 •

edited

spacegaier Dec 13, 2022

piitaya Dec 13, 2022

piitaya commented Dec 13, 2022 •

edited

spacegaier commented Dec 13, 2022

spacegaier commented Dec 13, 2022

piitaya commented Dec 13, 2022 •

edited

piitaya commented Dec 13, 2022

spacegaier commented Dec 13, 2022

spacegaier commented Dec 13, 2022 •

edited

piitaya Dec 13, 2022

Ensure consistent light state icon brightness #14740

Ensure consistent light state icon brightness #14740

Conversation

spacegaier commented Dec 13, 2022 • edited

Breaking change

Proposed change

Type of change

Example configuration

Additional information

Checklist

spacegaier Dec 13, 2022

Choose a reason for hiding this comment

piitaya Dec 13, 2022

Choose a reason for hiding this comment

piitaya commented Dec 13, 2022 • edited

spacegaier commented Dec 13, 2022

spacegaier commented Dec 13, 2022

piitaya commented Dec 13, 2022 • edited

piitaya commented Dec 13, 2022

spacegaier commented Dec 13, 2022

spacegaier commented Dec 13, 2022 • edited

piitaya Dec 13, 2022

Choose a reason for hiding this comment

spacegaier commented Dec 13, 2022 •

edited

piitaya commented Dec 13, 2022 •

edited

piitaya commented Dec 13, 2022 •

edited

spacegaier commented Dec 13, 2022 •

edited