-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Refresh frontend on start event #6042
Conversation
8100c02
to
dec3e2c
Compare
Although this probably fixes the issues with the early startup of the frontend, I don't think this is the way to go. If startup took a long time we now just refresh the page when a user was maybe in the middle of editing something. I rather fix the issues I mentioned in your PR, the first thing we will need for that is a flag so the frontend knows HA is not done starting yet. |
How should we tell the the frontend that HA isn't starting yet? I'm poking around in the dark a bit with the frontend so it may be far less frustrating for everyone if I'm not the one making the changes on this side of the house. |
I'm completely fine with making the changes on this side :-) We could add a started boolean to |
I added |
// when startup is completed | ||
conn.subscribeEvents(() => { | ||
location.reload(); | ||
}, "homeassistant_start"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is too aggressive. Most pages will actually do just fine. It's just generated Lovelace UIs opened while starting up that have issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We also shouldn't add non-data logic in the state
mix-ins as they are used for other projects too (ie cast). We have logic like this in home-assistant.ts
for when we detect a version upgrade (And a reload is needed)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. Probably makes sense to close this PR and go the route @bramkragten suggested above in #6042 (comment)
I'll leave this open and marked as a draft for now though so there is a place to comment.
This work has been superseded by a better solution in #6068 |
Proposed change
Refresh the frontend when Home Assistant fires the start event.
This PR supports home-assistant/core#36093 (review)
Type of change
Example configuration
Additional information
Checklist
If user exposed functionality or configuration variables are added/changed: