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

Deep equal check on state hooks #122

Merged
merged 1 commit into from
Jan 3, 2024
Merged

Conversation

bratanon
Copy link
Contributor

@bratanon bratanon commented Dec 29, 2023

We currently check for equality of two objects using only === which will not work as they are not primitiv values, so we need to deepEqual them.

The current approach will do many unnecessary updates, and will kind of break the "map" function in home assistant if you have many entities. As the more entities you have, the more the customized _updateHass will be called.

I'm not able to zoom in the map, as it keeps bringing me back to the init zoom level, same with moving around the map. It will constantly take me to the default location.

I have tested this PR a while, and I can't see any issue with it. State and icon_colors are updated as they should.

Fixes #123

@bratanon bratanon requested a review from Mariusthvdb December 29, 2023 14:20
@Mariusthvdb
Copy link
Owner

not sure about this tbh.
the power of JS is the type checking, and by setting those to strings only, we are foregoing that aren't we.

also, Ive never experience what you have so I should probably first check the current state of affairs with an example issue, could you please create that?

only thing I ever encountered that might be caused by custom0ui is the constant updating swirling icon in an apex charts card.

custom-ui.js Show resolved Hide resolved
@bratanon
Copy link
Contributor Author

the power of JS is the type checking, and by setting those to strings only, we are foregoing that aren't we.
Not at all. This is not typescript, its plain Javascript which doesn't even have a real type checking.

Take this as an example

const obj1 = {}
const obj2 = {}

console.log(obj1 === obj2) // will be false.
console.log(obj1 == obj2) // will be false.
console.log(Object.is(obj1, obj2)) // will be false.

This also states the fact that the current implementation is broken, it doesn't simply work, as it doesn't do anything.
It will never go beond the if statement because it will either be caught by either the first statement if hass.states is truly, or the other one if its not.

also, Ive never experience what you have so I should probably first check the current state of affairs with an example issue, could you please create that?

Sure thing.

Copy link
Owner

@Mariusthvdb Mariusthvdb left a comment

Choose a reason for hiding this comment

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

thanks, would you care to prepare the new release, and upload the file in the repo too?

@bratanon bratanon merged commit 7cde213 into Mariusthvdb:master Jan 3, 2024
1 check passed
@bratanon
Copy link
Contributor Author

bratanon commented Jan 3, 2024

thanks, would you care to prepare the new release, and upload the file in the repo too?

@Mariusthvdb Im not sure what do with the other files, like the gpt file which doesn't use the same if-statement.

@Mariusthvdb
Copy link
Owner

Mariusthvdb commented Jan 4, 2024

I should ditch the gpt file, it’s not what we want..

the separate ‘icon_color’ has its own repo now.

Guess we should only keep custom-ui-(only).

@bratanon since there now is a much better custom-more-info, replacing all functionality inside custom-ui for this, we should go forward with the -only version, and refer to the new resource for (un)filtering attributes and customizing the more-info cards @elchininet and I have published during the holidays.

archiving the existing custom-ui would be the best we can do I believe

@bratanon
Copy link
Contributor Author

bratanon commented Jan 6, 2024

@Mariusthvdb I don't really follow how the setup should be with all the different repos now. I actually think it would be more sufficient if you managed this release.

@Mariusthvdb
Copy link
Owner

Mariusthvdb commented Jan 7, 2024

I can see it is a bit confusing now, but I dont agree we should only manage this release.

Custom-ui is legacy software and more and more so I am afraid. It still works, and is very powerful, but it seems to have no real future in modern HA.

Part of the functionality, the UpdateMoreInfo, was way to fragile to keep working and especially maintain. That's why we now have made the new plugin for that: custom-more-info.

That new plug is modern, and fully compatible with current Dashboard/Frontend, and fully maintainable. @elchininet did a wonderful job there (were still optimizing its options, but even in its current state, it is awesome already)

Because of that I am convinced we should take out at least that functionality of custom-ui, so the 2 dont interfere, and custom-ui has less impact. As you described yourself, custom-ui is updating constantly, and we should prevent that as much as we can.

Next step I took, was to only inject the icon_color attribute, and take out the templating functionality, because that too is updating constantly, upon each and every state change.

my repo for custom-icon-color does exactly that, and nothing more. We've even managed to make it work with entity cards now too, so its gotten better than its original :-) (though I presume we can bring that same functionality to custom-ui too, should be easy)

all in all:

custom-ui (needs to be archived as is)

original, but legacy, offers templating and icon_color, and more-info hiding attributes, with ugly animated dropdown box

custom-ui-only

original, but legacy, offers templating and icon_color. No more-info (refer to new plugin custom-more-info). This should be the plugin to (try to) maintain in this repo. If possible modernize.

custom-icon-color

original (and yes still legacy), no more templating, offers only icon_color. much lighter and can be used in modern template: entities so I can see some future for this small plugin because of that only.

deleted from this repo


custom-more-info

new, modern, offers superior customizing of the more-info panels, hides attributes dropdown completely, super nice.

custom-attributes

we have also made this plugin, and it handles only the attributes in the more-info panels. It is superseded by custom-more-info, but if you dint want/need the more-info handling of history and logbook, you can keep it as small as this plugin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

_updateHass are called with the same unchanged states
2 participants