Mariusthvdb / custom-ui

Add templates and icon_color to Home Assistant UI
163 stars 30 forks source link

More readable code #51

Closed bratanon closed 2 years ago

bratanon commented 2 years ago

This PR is a more readable version if #50 .

I think it will be more easy to apply fixes in the future with more readable code that is not obscurified/minified like the current one are.

bratanon commented 2 years ago

See #50

Mariusthvdb commented 2 years ago

thanks for the effort, I did wish we had a non minified version. As explained, I hacked into the final code, and didnt build it from the source files.

Until I get positive feedback on how to fix this for HA 2022.4 and above, I dont feel there's any profit in trying now though.\ So yes, appreciated, and I agree it would be better. But different priorities now I am afraid.

Still, if you manage to make the code better and have it behave as it did under 2022.3. Id be happy to test.

Mariusthvdb commented 2 years ago

just so you know I did test this, and I must say it does seem to work after all!

homeassistant:
  customize:

    input_boolean.test:
      package: Test
      templates:
        icon: >
          return (state === 'on') ? 'mdi:test-tube' : 'mdi:test-tube-off';
        icon_color: >
          return (state === 'on') ? 'gold' : 'green';

    input_number.test_number:
      templates:
        icon_color: >
          if (state == 0) return 'white';
          if (state == 10) return 'yellow';
          if (state == 20) return 'brown';
          if (state == 30) return 'navy';
          if (state == 40) return 'lightblue';
          if (state == 50) return 'green';
          if (state == 60) return 'orange';
          if (state == 70) return 'purple';
          if (state == 80) return 'black';
          if (state == 90) return 'gold';
          return 'red';

returns

test-custom-ui

Id be happy if people could test this, so Ill post the new code as a beta and hope for some feedback.

Cant thank you enough for now!

Would you please explain how you managed to re-write this from the minified code?

bratanon commented 2 years ago

Well, it works until something else changes. I.e. there is a state update from the websocket. It will then revert back to the old icon color, icon, and state. Atleast that was what was happending to me. If there is other test results showing this works, lets continue to make sure that there are no performance issues with this. I would be more then happy help with that aswell.

As I mentioned, when I tried this is a super small env with only 2 sensors, almost like your picture, I got it working. However, when testing it on my main setup with a lot of sensors, and when there are more then 10 websocket state updates every second, it went bad.

I have tried to actually see exactly whats changes with 2022.4 in the frontend in order to see why custom-ui is not working anymore, but its quite hard to find. The only thing I can see is an update on HAWS.

I was under the impression that they did something huge with the frontend, in ordet to support websockets, but as it turned out, there was already websockets for state handling the frontend and has been a really long time.

So the changes now seems to be that they are only sending a single state on updates, which makes sense, but I cant find that change in the FE anyware. Which also makes no sense why custom-ui is broken.

bratanon commented 2 years ago

After some more testing, I get the same results as you. It works for with your config using input_number and input_boolean.

However, I added a sensor template:

template:
  - sensor:
      - name: "Test sensor"
        state: >
          {{ states('input_boolean.test') }} #This is your input_boolean.
sensor.test_sensor:
  templates:
    state: >
      return (state === 'on') ? 'Boolean is on' : 'Boolean is off';
    icon_color: >
      return (state === 'on') ? 'blue' : 'red';

And this simple time pattern trigger automation that does nothing expect just triggers every 10 sec. (Which triggers an websocket event).

 alias: Test automation
description: ''
trigger:
  - platform: time_pattern
    seconds: /10
condition: []
action:
  - delay:
      hours: 0
      minutes: 0
      seconds: 1
      milliseconds: 0
mode: single

The input_number and input_boolean works, but the template sensor is reverted back after every automation trigger.

Mariusthvdb commented 2 years ago

I will check that, though at first sight everything still works as hoped for.

do note you are templating the state, and you really shouldn't do that, as it can cause unexpected behavior..... please keep it at templating icon and icon_color for this test.

also, although totally possible, customizing a template's icon is not needed, as we can set icon template in the sensor's config directly.

What's unclear to me, is how that trigger (for a websocket event) could do that , and my full production system doesn't see that happening.(while web socket events happen all the time...)

Btw with the minified'old' custom-ui and the new ha 2022.4 we also noticed a difference between legacy format template sensors and template: sensors see https://github.com/Mariusthvdb/custom-ui/issues/48#issuecomment-1095334318

Maybe this is related/caused by the same underlying reason.

It was remarkable at least to experience this in the first place and warrants further exploration per se.

bratanon commented 2 years ago

Without state templating, it seems to work much better as you say. Also the legacy templating works more stable the the "new" one.

I will continue to test this.

Mariusthvdb commented 2 years ago

yep. just so you know, that was already not recommended in Andrey's days..

so the big question is, why does the code work in the new 'readable' version, and needs a page reload in the minified version?

not that I want to go back, I like the readability ;-) seems that only rewriting it should not have effect on functionality.

Anyways, if people confirm this to be working, I can release it, and finally get to improving the code in such a way, we can fix the more-info issues. (hide-attributes: template works on opening of the dropdown, but, when there's no other attribute to show, shows an empty dropdown, which is really ugly)

I do have a lead from the main Frontend devs how that could be solved, but didnt get it right just yet...

Opened a discussion for that: https://github.com/Mariusthvdb/custom-ui/discussions/52

bratanon commented 2 years ago

Have made some more cleanup and it seems to work now when not using state templating.

Peek 2022-04-18 14-33

Mariusthvdb commented 2 years ago

great. that confirms my earlier findings, which is a huge relieve.....

could you add that to a final PR, so I can merge and release? just so we are all using the same code '-)

bratanon commented 2 years ago

Added it now.

However, I would like to try to find a way to handle state templating aswell as I have been using that for a long time. It might not work, and then what we have now is good to go I would say.

I dont know if there are many others that are using state templating, but if the majority is not, then I guess we can release what we have now, then later maybe add a state templaing fix if its possible.