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

Input_number: Added display_state option #886

Closed
wants to merge 2 commits into from

Conversation

cdce8p
Copy link
Member

@cdce8p cdce8p commented Feb 12, 2018

Description:
Added parameter to make changes made by /pull/808 optional.
The default for display_state is true, so it doesn't change anything until it's set to false.

Pull request in home-assistant (if applicable): home-assistant/core#12351
Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#4654

@cdce8p cdce8p changed the title Input-number: Added option to hide state Input_number: Added display_state option Feb 12, 2018
@c727
Copy link
Contributor

c727 commented Feb 13, 2018

The key is named display_state and is used to hide unit of measurement? Makes no sense.
Also why should someone hide uom when it's optional all in all?

@balloob
Copy link
Member

balloob commented Feb 13, 2018

I don't like configuration options for UI. Now all of a sudden the backend is aware of the frontend and is storing config options in the state machine. Both are a no-no.

If you don't like the value, you can use custom UI. We should not merge this.

@balloob balloob closed this Feb 13, 2018
@cdce8p
Copy link
Member Author

cdce8p commented Feb 13, 2018

@c727 It does make more sense for the silder. In my case: I use sliders to set a timer and the time is displayed in an template sensor entity right above the silders. I personally like it better than the input_datetime sensor. The first pic is the default view.
pic1
This one is with display_state set to false.
pc2
I already changed it locally through a custom_component and custom UI, but I though that others may want to hide the number next to the silder as well.

@balloob I know this isn't ideal, but isn't the mode parameter somewhat similar. IMO their are cases where it's quite useful to configure extra parameter for the frontend view (friendly_name, icon, to name a few). Even if the focus should be to keep the config as easy as possible, which I completly agree with, I belive that those who want more options shouldn't need to write their own custom UI and components for it.

@balloob
Copy link
Member

balloob commented Feb 14, 2018

Every config option is something that has to be maintained forever. It is also stored in the state machine forever. There are a few exceptions like friendly name and icon and the occasional mode (although limited).

We should not aim in building a UI that can satisfy everyone's needs, as that is impossible. Instead, Home Assistant should include a UI that satisfies most needs and offer building blocks for people to build their own (custom UI, custom panel, websocket API, custom components).

@OttoWinter
Copy link
Member

A bit of my humble opinion on this:

I'm 100% your (@balloob) opinion that the frontend and backend should be separate, and that the frontend should do all UI relevant stuff. Both for keeping the codebase easy to maintain (/keeping it maintainable at all) and for avoiding "hacks" like this that eventually might get abused.

But I mean in the past exceptions like control: hidden for groups have been added, which are clearly only for the front-end. Why were they added and why are people still trying to add front-end features in the back-end? I think this is because the back-end is currently the only place where these options like group#control can be placed. The front-end simply doesn't have a persistent, "cross-browser" configuration holding option.

In my humble opinion, Home IoT services have to be customizable, since everyone has their very own setup and wants their own thing that best suits their home. Home Assistant is amazing for customization when it comes to the back-end with powerful automations, templates, etc.

However, I don't see that easy customizability in the front-end that much. Sure, you can create groups, move entities around a bit, give them icons and names. And sure: you can also built custom UI that suits your needs with some basic HTML/JS skills, but that's an awful lot of work - it's also what has kept me from doing it. You're completely right in saying that satisfying everyone UI needs is impossible - it is. But allowing for some more customization is, in my opinion, reasonable.

The customize: component solves some of these problems, like custom icons and names, and has great support for editing within your browser. I don't see why customize: or some new component (let's say ui) can't be used for front-end UI customization storage. Except: it might (and would) get bloated over time. But then again, with such amazing editing support directly from within your browser, that wouldn't be that big of a problem, in my humble opinion. And if you prefer not to customize anything, it wouldn't affect your UX at all.

Every config option is something that has to be maintained forever.

Sure, that's very true. I would argue that the back-end suffers from this (it's almost impossible to change some oftenly used core functions, from what I've seen in previous PRs by others). Arguably, the front-end hasn't been affected by this that much. But a bit more customization would, again, be nice in my view. On the other hand, you could also say that we shouldn't repeat the mistakes in the back-end in the front-end and that's a very fair point.

I believe Home Assistant as a product is also to some extent about the front-end. And while trying to keep the front- and back-end unaware of each other to raise maintainability, we shouldn't neglect the other (wording too harsh, but I hope you get my point).

But then again, on the other side, Home Assistant has probably only been able to come this far by staying true to its ideals and making well thought out decisions when it comes to the core architecture. So the current path also can't be too wrong 👏.

@balloob
Copy link
Member

balloob commented Feb 16, 2018

First off, the driving use case for this feature is bad. The author did not like our time input controls so decided to roll it's own by putting some UI elements together. However, it didn't like the look of his new time control and so decides to add config options for that. Not only is it an edge case, it is also working around a problem instead of solving it.

Things like control: hidden were added and are a mistake. They work around a bug that the frontend thinks a group is controllable while it is not (ie a group of binary sensors). However, any mistake made in the past is never a reason to do it again. This is something we also note in our checklist for contributions.

We have to live with the consequences. Even when the bug in the frontend gets fixed, that option will be around. Not good.

I think that if someone wants to create an ultimate configurable UI on top of Home Assistant, they can do so. Like HADashboard. No way does that have to happen under the official banner and maintained by us.

Home IoT services have to be customizable, since everyone has their very own setup and wants their own thing that best suits their home.

Go build it and see how far you get! 😉 You have to realize that Home Assistant has come this far because we kept it simple. There is a reason why it's not a dumpster fire of config options that limit any ability to innovate.

@OttoWinter
Copy link
Member

You're 100% right.

Go build it and see how far you get!

I actually tried this a few years back with Ruby on Rails and failed miserably 😅

First off, the driving use case for this feature is bad.

I agree, I wasn't really talking about this particular thing but in general. I get your point though.

@github-actions github-actions bot locked and limited conversation to collaborators Jul 8, 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.

None yet

5 participants