Skip to content

Compute the icon based on the logbook state and not the current state#12725

Merged
balloob merged 8 commits intohome-assistant:devfrom
bdraco:fix_logbook_current_state
May 20, 2022
Merged

Compute the icon based on the logbook state and not the current state#12725
balloob merged 8 commits intohome-assistant:devfrom
bdraco:fix_logbook_current_state

Conversation

@bdraco
Copy link
Copy Markdown
Member

@bdraco bdraco commented May 18, 2022

Proposed change

  • If there was no custom icon, we used the current state to
    calculate the icon. We need to use the state from the log

I thought this was a regression caused by #12713 but I
went back to Monday's build and it was the same.

Before

Screen Shot 2022-05-18 at 15 47 54

Screen Shot 2022-05-18 at 15 41 53

After
Screen Shot 2022-05-18 at 17 37 55
Screen Shot 2022-05-18 at 17 37 45

Screen Shot 2022-05-18 at 16 34 04

Screen Shot 2022-05-18 at 17 09 06

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:

- If there was no custom icon, we used the current state to
  calculate the icon. We need to use the state from the log
@bdraco
Copy link
Copy Markdown
Member Author

bdraco commented May 18, 2022

At some point fetching the icon started returning svg data ??

@bdraco
Copy link
Copy Markdown
Member Author

bdraco commented May 18, 2022

So in #10223 we now return an SVG but overrideIcon expects something else

@bdraco
Copy link
Copy Markdown
Member Author

bdraco commented May 18, 2022

Still need to fix custom icons

@bdraco bdraco marked this pull request as ready for review May 18, 2022 22:49
!historicStateObj &&
!item.icon &&
domain &&
isComponentLoaded(this.hass, domain)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why do we care if it is loaded?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Co-authored-by: Bram Kragten <mail@bramkragten.nl>
@bdraco
Copy link
Copy Markdown
Member Author

bdraco commented May 19, 2022

Need to fix custom images
Screen Shot 2022-05-19 at 12 29 30

Actually thats correct, the logbook api doesn't provide the historic image and it provided an icon instead in this case so it should show the icon because the current image would be incorrect.
Screen Shot 2022-05-19 at 15 18 50

@bdraco bdraco marked this pull request as draft May 19, 2022 17:30
@bdraco bdraco marked this pull request as ready for review May 19, 2022 20:16
@balloob balloob merged commit a0a7ce0 into home-assistant:dev May 20, 2022
@github-actions github-actions bot locked and limited conversation to collaborators May 21, 2022
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.

4 participants