Conversation
There was a problem hiding this comment.
Pull request overview
Updates the cover icon mapping to include more device classes and more granular icons for transitional states (opening/closing), aiming to mirror Home Assistant frontend behavior.
Changes:
- Expand
coverIconto handle additionaldevice_classvalues (e.g., gate, damper, curtain, shade). - Add distinct icons for
opening/closingstates across multiple cover device classes. - Replace the prior fallback (
domainIcon("cover", state.state)) with an explicit fallback mapping for cover states.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return "mdi:garage-open"; | ||
| } | ||
| case "gate": | ||
| switch (state) { |
There was a problem hiding this comment.
This switch is switching on the entire HassEntity object (state), but the case labels are strings. It should switch on state.state to correctly match "opening"/"closing"/"closed".
| return open ? "mdi:circle" : "mdi:circle-slice-8"; | ||
| case "shutter": | ||
| return open ? "mdi:window-shutter-open" : "mdi:window-shutter"; | ||
| switch (state) { |
There was a problem hiding this comment.
This switch is switching on the entire HassEntity object (state), but the case labels are strings. It should switch on state.state so the shutter icons can match the cover state.
| return "mdi:window-shutter-open"; | ||
| } | ||
| case "curtain": | ||
| switch (state) { |
There was a problem hiding this comment.
This switch is switching on the entire HassEntity object (state) rather than the state string. Switch on state.state so the curtain opening/closing/closed icons work.
| case "blind": | ||
| return open ? "mdi:blinds-open" : "mdi:blinds"; | ||
| case "shade": | ||
| switch (state) { |
There was a problem hiding this comment.
This switch is switching on the full HassEntity (state) instead of the state value. Switch on state.state so blind/shade icons match opening/closing/closed.
| } | ||
| case "window": | ||
| return open ? "mdi:window-open" : "mdi:window-closed"; | ||
| switch (state) { |
There was a problem hiding this comment.
This switch is switching on the full HassEntity (state) instead of state.state. As written, none of the string cases will match.
| } | ||
| default: | ||
| return domainIcon("cover", state.state); | ||
| switch (state) { |
There was a problem hiding this comment.
This default branch switch is switching on the HassEntity object (state) rather than the entity's state string. Switch on state.state so opening/closing/closed are handled correctly.
| switch (state.attributes.device_class) { | ||
| case "garage": | ||
| return open ? "mdi:garage-open" : "mdi:garage"; | ||
| switch (state) { |
There was a problem hiding this comment.
This switch is switching on the entire HassEntity object (state), but the case labels are strings like "opening"/"closed". This will never match (and should be a TS type error). Switch on state.state instead.
|
I've been away far too long, thanks for this. I totally get it if you're not longer involved in this, I haven't touched HA in 4 years, but I like the suggestions and will move forward with them even if you are unable to address the review comments. Thanks again. |
now mirroring https://github.com/home-assistant/frontend/blob/dev/src/common/entity/cover_icon.ts