[logbook] implement shouldUpdate#4832
Conversation
| if (changedProps.has("hass")) { | ||
| const oldHass = changedProps.get("hass") as HomeAssistant | undefined; | ||
| if (oldHass && oldHass.language !== this.hass.language) { | ||
| this._rtl = computeRTL(this.hass); |
There was a problem hiding this comment.
We shouldn't have side effects in our shouldUpdate. Setting this._rtl should stay in updated.
There was a problem hiding this comment.
we don't evne use this._rtl it seems? We can drop it.
There was a problem hiding this comment.
It is used, reflected attribute
There was a problem hiding this comment.
shouldUpdate should return true when the language is changed or when entries is changed.
This change will affect state icons btw, they will no longer be updated when a state changed. But I don't think that is a big problem as the icon would not match the logbook entry anyway? 🤔
There was a problem hiding this comment.
Yes, rtl is used for date title. My idea was calculate this._rtl before the update, so I think performUpdate is a better place than updated.
There was a problem hiding this comment.
No need to do it in performUpdate as your shouldUpdate is false when _rtl is changed, it will not cause another render.
315bc95 to
db441c1
Compare
|
|
||
| protected firstUpdated(changedProps: PropertyValues) { | ||
| super.firstUpdated(changedProps); | ||
| protected performUpdate() { |
db441c1 to
5a1384f
Compare
|
What I want to achieve here is to make the first |
5a1384f to
a0d4472
Compare
|
Move to |
Co-Authored-By: Paulus Schoutsen <paulus@home-assistant.io>
Proposed change
I noticed many unnecessary calls of loogbook
render. This is because of updatedhassproperty which actually usually contains no changes. This is especially bad when there are many entries loaded and causes UI freezes.Implement
shouldUpdateto update logbook only when entries are updated or when rtl is changed.Type of change
Example configuration
Additional information
Checklist
If user exposed functionality or configuration variables are added/changed: