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

Remove turn_off from brightness slider #1715

Merged
merged 1 commit into from
Oct 1, 2018

Conversation

amelchio
Copy link
Contributor

This makes it easier to set low brightness values by making the slider work only as a dimmer. Power must then be controlled by the switch.

The minimum is set to 1 because some bulbs will turn off with zero brightness.

Copy link
Member

@cgarwood cgarwood left a comment

Choose a reason for hiding this comment

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

Looks good to me

@balloob balloob merged commit 8be5561 into home-assistant:master Oct 1, 2018
@ghost ghost removed the in progress label Oct 1, 2018
@Mariusthvdb
Copy link
Contributor

Mariusthvdb commented Oct 2, 2018

please don't. Using the slider to switch Off is very very useful, and if not desired can be switched off easily already by using customize: off_when_min: false.

see: https://community.home-assistant.io/t/turning-a-dimmer-down-to-lowest-via-ui-slider-should-keep-the-light-on-not-switch-off/8891/17?u=mariusthvdb

schermafbeelding 2018-10-02 om 15 46 27

Brightness set to 1 and 2 without any issues, killing this functionality would require more user interaction and take away this much used functionality...
please don't.

@balloob
Copy link
Member

balloob commented Oct 2, 2018

@Mariusthvdb the screenshot you're showing is not Home Assistant. It's custom UI or custom Lovelace, it has nothing to do with this repo.

@Mariusthvdb
Copy link
Contributor

well then, this is Lovelace, with the regular HA slider.

schermafbeelding 2018-10-02 om 22 55 13

The point isn't the sceenshot. It is that in this repo functionality is cut, depriving users who do find it useful of that functionality. Btw, both Hue and Tradfri hardware dimmers can slide all the way to Off, so as it stands, Hue implementation is coherent with real life.

If must be, please make it the exception and add an option to off_when_min: true to HA natively. Like that best of both worlds would be possible.

@balloob
Copy link
Member

balloob commented Oct 3, 2018

We're not going to add an option for that to control behavior in the more info dialog. If you want to turn it off, use the toggle at the top.

@Mariusthvdb
Copy link
Contributor

Mariusthvdb commented Oct 3, 2018

sorry to hear, too bad really.

You're limiting truly useful generic functionality. While people could just as easily have been pointed to a solution for their wishes in the current setup and available tools.

It's not about adding an option, its about not taking thing out..causing a breaking change with the custom-ui.
thanks for considering and taking time to answer.

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.

5 participants