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

💄 additional active icon states #4510

Merged
merged 9 commits into from
Jan 23, 2020
Merged

Conversation

iantrich
Copy link
Member

@iantrich iantrich commented Jan 18, 2020

I think highlighting devices that require potential attention is the main goal and after some thought propose the following changes:

Domain - Active State
Existing

light - on
switch - on
binary_sensor - on
fan - on
sun - above_horizon

Additional

input_boolean - on
cover - open
media_player - playing/paused
device_tracker/person - home
plant - problem
lock - unlocked
climate - heating/cooling

fixes #3897

@iantrich iantrich requested a review from bramkragten January 18, 2020 02:10
@iantrich iantrich self-assigned this Jan 18, 2020
Copy link
Member

@ludeeus ludeeus left a comment

Choose a reason for hiding this comment

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

I feel like paused for media players should be removed, the media player does not ha an active state when it's not doing anything.

As for the lock, I get why you had it like this, but it should be inverted to conform with other active states.

src/components/entity/state-badge.ts Outdated Show resolved Hide resolved
src/components/entity/state-badge.ts Outdated Show resolved Hide resolved
src/panels/lovelace/cards/hui-entity-button-card.ts Outdated Show resolved Hide resolved
src/panels/lovelace/cards/hui-entity-button-card.ts Outdated Show resolved Hide resolved
Copy link
Member

@balloob balloob left a comment

Choose a reason for hiding this comment

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

My comment from linked PR - #3897 (comment):

Each time this comes up, I say the same thing: don't suggest that we need to color this or that, instead, come up with a strategy that we can use for each place (entity row, entity button etc) on how we should color each type of entity/device class and each of their states.

@PeteBa
Copy link
Contributor

PeteBa commented Jan 19, 2020

Looks like there is two competing strategies here. The first being highlight when active (e.g. "locked") the other being highlight when requires attention (e.g. "unlocked"). Feels like that choice needs to be made first.

Personal preference is the former as it can be explicitly coded. The difficulty with "requires attention" is that it varies with context e.g. an ulocked front door requires attention but so does a locked fire-exit door.

@iantrich
Copy link
Member Author

@balloob I believe I did, my strategy is highlighting items requiring potential attention. Or are you saying I should explicitly state for each and every entity/state combination? I think that's overkill and this can evolve overtime.

I don't think there should be a difference between state badge and entity button.

@balloob
Copy link
Member

balloob commented Jan 19, 2020

I think that if something requires attention, like a plant in "problem" state, we should color it red or even add a red badge to the icon with an ! in it.

@balloob
Copy link
Member

balloob commented Jan 19, 2020

So I'm glad that we're talking about this issue btw, and let's find a solution 👍 and sorry for not reading the description thoroughly when leaving my review.

So I don't know if "potentially requires attention" is a good strategy. Is a person being home requiring attention ? Wouldn't either "active" or "open" be a better strategy?

@arsaboo
Copy link
Contributor

arsaboo commented Jan 19, 2020

Will it be possible (with this PR) to allow little more user control? For example, we may not know what requires attention. Also, attention may not be a simple binary state, i.e., there are different levels of attention. Something to think about (if not for this PR, maybe for the future).

@balloob
Copy link
Member

balloob commented Jan 19, 2020

What is the driver of this change? Right now glance and picture elements use the same component as the entities row card. Glance and picture elements could benefit of such changes, entity row not (it would become too busy).

@arsaboo that is out of scope of this pr

@iantrich
Copy link
Member Author

@balloob person was admittedly something I struggled to fit within this reasoning. Let me take sometime to better describe each case and we'll go from there. I really want to get something resolved on this as well 😉

@iantrich
Copy link
Member Author

Also,
I believe all those elements benefit from state-badge changes. I'll double check

@balloob
Copy link
Member

balloob commented Jan 19, 2020

The problem is that if everything wants to grab attention, nothing will.

@iantrich
Copy link
Member Author

So my idea is that we should only highlight/change an icon color when it warrants attention which can be a very tricky thing to balance as @balloob noted. My initial changes were the ones I saw most value from, but if I'm doing this, I might as well do it right and consider ALL domains, so I'll try my best to explain each domain below (sourced from domain_icon.ts, so may not be complete) and hopefully we can finish with something that makes a lot of people happy:

  • alarm_control_panel: armed
    When an alarm is armed, your home is typically restricted in some way. e.g. notifications on motion which I believe warrants more attention from a user. As you'll see below, this somewhat contends with other ideas on things like doors/locks, but the fact that an armed home is usually used to restrict a home, I think it warrants greater attention than disarmed.
  • alert: on
    I don't think there will be much discussion for this one.
  • alexa: (I'm not sure what this represents, actually. The connection?)
  • automation: (I'm undecided if this one warrants any change or not)
  • binary_sensor: on
    I don't think there will be much discussion for this one.
  • calendar: on
    I believe this is only on during an event, and if so would make sense to be when it is highlighted
  • camera: streaming
    Not really sure about this one yet, I'm conflicted
  • climate: heating/cooling
    Your furnace actively pushing air and thereby using more energy warrants more attention
  • configurator: (Should not change)
  • conversation: (Should not change)
  • counter: (Should not change)
  • cover: open
    I don't think there will be much discussion for this one.
  • device_tracker/person: home
    I've went back and forth on this one a bit, but I believe that home is of more significance than away. Change my mind 😄
  • fan: on
    I don't think there will be much discussion for this one.
  • google_assistant: (I'm not sure what this represents, actually. The connection?)
  • group: (This can take on numerous states, dependent on what type of entities it consists of. I don't have a clear answer for this.)
  • history_graph: (A relic of states UI, correct?)
  • homeassistant: (I'm not sure what this represents)
  • homekit: (I'm not sure what this represents, actually. The connection?)
  • image_processing: (I'm not familiar enough with this integration)
  • input_boolean: on
    I don't think there will be much discussion for this one.
  • input_datetime: (Should not change)
  • input_number: (Should not change)
  • input_select: (Should not change)
  • input_text: (Should not change)
  • light: on
    I don't think there will be much discussion for this one.
  • lock: unlocked
    Along the same lines as a garage door or door sensor, an unlocked door makes for a less secure home
  • mailbox: (I don't know enough about this integration)
  • media_player: not off or idle
    This would then be where a media player is being used in some form and therefore is more notable
  • notify: (I don't know what this would be. Notification groups are not in states?)
  • persistent_notification: on
    I don't think there will be much discussion for this one.
  • plant: problem
    I don't think there will be much discussion for this one.
  • proximity: (Should not change)
  • remote: (Should not change)
  • scene: on
    I don't think there will be much discussion for this one.
  • script: running
    I don't think there will be much discussion for this one.
  • sensor: (Should not change)
  • simple_alarm: I'm not familiar with this one
  • sun: above_horizon
    I don't think there will be much discussion for this one.
  • switch: on
    I don't think there will be much discussion for this one.
  • timer: active/paused
    Both much different than idle and likely to be addressed
  • updater: (I'm not familiar with this one)
  • vacuum: cleaning
  • water_heater: (Should not change)
    I'm not sure about this one, but appears as just the operation mode is reported as state
  • weather: (Should not change)
  • weblink: (Should not change)
  • zone: (Should not change)
  • zwave: dead
    I don't know enough about zwave to answer this one for sure

@balloob
Copy link
Member

balloob commented Jan 20, 2020

I guess I am not convinced that "needing attention" is a good strategy. I still prefer using energy.

Not all domains will have states. Some are only because they are shown in the logbook. Like Alexa/Google.

Scenes don't change state and can't be colored. Same with zones.

Climate. This one if anything should be linked to the action, not the mode ? And if it's set to mode, all modes should be colored except if it's off. Should follow same coloring as the thermostat card. Consistency is important. We can't have two colors to indicate that a device is heating.

I don't believe that a person or device tracker being home should be colored.

@iantrich
Copy link
Member Author

I agree that using energy is a good piece of criteria, but is also too narrow for all of home automation in my opinion.

Climate: I would be on board with aligning with thermo card for hvac_action

@SeanPM5
Copy link
Contributor

SeanPM5 commented Jan 20, 2020

I lean towards preferring the "requires attention" idea and think ian's list above is mostly good.

Perhaps another way of thinking about this: if you're about to go to sleep/leave home/go on vacation and take a quick look at Home Assistant before doing so, what information is most important to know at a glance? Whether there's any doors left opened or unlocked, whether your alarm is armed, if you forgot to turn off some device, etc.

I don't think device tracker / person should be colored as that one feels too subjective. You could argue that "home" state is more significant as automations depend on that, but many parents would probably think their child being "away" requires more attention.

@balloob
Copy link
Member

balloob commented Jan 20, 2020

I think that if we want to build an interface that is glance worthy in case you're going to sleep/leave home etc, we should probably not conflate that with our control UI?

(we have already discussed a couple of times to get sentence summaries of rooms/everything and is still on some list)

@balloob
Copy link
Member

balloob commented Jan 20, 2020

Mockup:

image

I think that too much yellow will render it useless. Maybe not color any non-controllable entities ?

@SeanPM5
Copy link
Contributor

SeanPM5 commented Jan 20, 2020

I agree that if most icons become yellow it would kinda defeat the point and just tip things too far in the opposite direction.

But at the same time, the current icon coloring strategy feels somewhat arbitrary? For example:

icons

These all have the same exact "on" state, they're all in the same card, they all have toggles, yet some get coloring and others don't. They all look the same, yet behave differently. Isn't this kinda confusing for new users? I don't know how to easily explain this beyond "that's just the way things are."

One idea I had before was to drop coloring on entity rows altogether. The reasoning being that in most cases there's a colorful blue toggle to quickly determine state, and coloring the icon feels like duplicated functionality. iOS and Android don't color icons when there's a toggle switch. Plus for non-controllable entities, the state is always shown right next to it as text. But I suspect this would be kinda controversial so I was too afraid to ever suggest it publicly 😄

IMO though, this has always been less about the Entities card and more about other cards like Glance and Button. The glance card can be configured with show_state: false and really needs coloring in that case. Here is the same entities from above but in glance card form:

glance

Everything is "on" here just like above, yet if you showed anyone this picture, they would say that two things are on and three are off.

This is even more of an issue on the button card because that doesn't even have a show_state option at all. If you put input_boolean.guest_mode inside of a button card you'd have no way of knowing whether it was on or off. This makes the button card useless in many cases.

@iantrich
Copy link
Member Author

The glance and button cards really do highlight the need. I agree with entity rows, to an extent. I don't think we should remove it, but perhaps have an option for the card color_state_changes or something.

FWIW, the remote is a bit useless; you can't even turn a remote off to my knowledge

@bramkragten
Copy link
Member

We should at the very least align the different domains with coloring, that binary sensors do color and other domains don't doesn't make sense. We should make sure that a lock/cover binary sensor is the same as the lock and cover domain.

I do agree with Sean that it is mostly the glance/picture elements/button cards that need this, the entities row is not really needed, but I think it is weird to not do it in the entity row for consistency? As you would still want to do it for light to show what color it is?

I gave it some thought and requiring attention might be a slippery slope for a ruleset. It might be different from entity to entity and user to user. Anything that has a clear on/off state could be colored like a switch and binary sensor already are.

But I don't think much of Ian's table will change if we would base it on on/off.

For domains that do not have a clear on/off like climate, for example, we might be able to use other colors to indicate the state. For climate, we use color in the thermostat card to indicate the state, so we could expand those to the rest of the cards.

Eventually, I think we will get settings for this, also for sensor values to be in specific ranges to give it a specific color, etc. But we should have a sane default that works for 90% of the users and use cases.

@iantrich
Copy link
Member Author

iantrich commented Jan 20, 2020

I don't think we should touch sensor 😄
I would much prefer we do something like this for something that has so much variation

type: glance
entities:
  - entity: sensor.download_speed
    icon_color: red
    states:
      - value: 10
        operator: '>'
        icon_color: yellow
      - value: 20
        operator: '>'
        icon_color: green

How we accomplish that is another discussion though 😛 @balloob and I had previously discussed a state-merge-config card that would allow you to merge a configuration based on the current state of an entity. I think it would be an AWESOME card, but wrapper cards can be quite advanced and there's little chance of a nice UI editor for something like that. Anyways, that leads back to a user being able to define the icon color via the configuration which already I've had push back on.

I think ideally each card just have a state merge function that allowed for such changes, but again, I'm side-tracking myself 😄

@bramkragten
Copy link
Member

Yeah I said eventually, like not in this PR ;-)

@balloob
Copy link
Member

balloob commented Jan 21, 2020

Alright, what about:

  • We implement Ian's proposal more or less.
  • <state-badge> gets a new boolean property if it should be state-colored. (This is the component that renders the colored icons today.)
  • Entity rows set this default to false.
  • Glance/picture elements set this to True.
  • Maybe but rather not: add an option to entities card to enable state-colored

@Mariusthvdb
Copy link
Contributor

Alright, what about:

  • Maybe but rather not: add an option to entities card to enable state-colored

First of all let me thank you for discussing this, as a fervent customizer of state colors, I truly welcome the extra options for Lovelace this would offer. So do continue!

Please allow my 2cts on this: I feel using/adding state-colered: would be a great addition indeed, and I truly hope you will add it.

Moreover, might I suggest 1 extra option expanding on that:

  • if state-colered: true, also add the option for setting the color using state-color-on: and state-color-off:
    That would allow the user to set the accent- and off-color easily. (of course not setting it would follow the current theme coloring scheme by default)

Doing so would keep the global coloring schemes based on theme, And enable incidental coloring per card individually, using outside-theme coloring, which would be a main driver for using the state-color: option in the first place.

Please also remember users might want to change the off color also, not only the on color...
thanks.

@SeanPM5
Copy link
Contributor

SeanPM5 commented Jan 21, 2020

I think balloob's proposal sounds good.

  • Makes glance/button/picture elements cards a lot more useful
  • Brings consistent behavior to entity row
  • Avoids the potential 'sea of yellow icons' problem
  • Is very easy to explain. This card doesn't have coloring, these other ones do.
  • Seems relatively easy to change behavior later on if needed?

I suspect some will gripe about the entity row changes in the short term simply because they've been used to things working a certain way. Though I think most people will get over it quickly and realize coloring isn't necessary there.

But might want to make a compromise and still always color lights though in entity rows? Not sure.

@balloob
Copy link
Member

balloob commented Jan 21, 2020

@Mariusthvdb color customizations can happen with a theme-per-row setting. But let's keep that out of this PR.

@iantrich iantrich changed the title 💄 additional active icon states WIP 💄 additional active icon states Jan 22, 2020
@iantrich iantrich changed the title WIP 💄 additional active icon states 💄 additional active icon states Jan 22, 2020
@iantrich iantrich requested a review from balloob January 22, 2020 02:45
extract common css
properly set boolean attributes
separate error states/color
fix unavailable color
@balloob
Copy link
Member

balloob commented Jan 22, 2020

I think that it looks fine from my end. @bramkragten and @SeanPM5, any opinions on coloring?

@SeanPM5
Copy link
Contributor

SeanPM5 commented Jan 23, 2020

Coloring choices look good to me 👍

@iantrich
Copy link
Member Author

FYI, colored lights still change in the entities card

@balloob
Copy link
Member

balloob commented Jan 23, 2020

Makes sense colored bulbs are still colored. Should we make non-colored lights turn yellow? 😛

@iantrich
Copy link
Member Author

Makes sense colored bulbs are still colored. Should we make non-colored lights turn yellow?

the switch should suffice and a user can override if they wish with state_color option

@balloob balloob merged commit 753804f into home-assistant:dev Jan 23, 2020
@iantrich iantrich deleted the active-icon branch January 24, 2020 01:55
@lock lock bot locked and limited conversation to collaborators Jan 25, 2020
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.

Change icon color based on state
9 participants