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

Set min light brightness to 1 #848

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

wilco375
Copy link

Description

This PR updates the light brightness slider such that it goes down to 1% instead of 0%. See related issue for reasoning.

Related Issue

This PR fixes or closes issue: fixes #847

Motivation and Context

Allows setting a light to its lowest brightness without turning it off

How Has This Been Tested

New code has been compiled and added to HA as described in the README under Manual

Types of changes

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • 🚀 New feature (non-breaking change which adds functionality)
  • 🌎 Translation (addition or update a translation)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have tested the change locally.
  • I followed the steps if I add a new language .

@wilco375
Copy link
Author

This could also be implemented as an extra switch in the light card to choose between 0% or 1% as a minimum if that is more desirable.

@piitaya
Copy link
Owner

piitaya commented Nov 16, 2022

I think it must 1 but we need to make the icon clickable before to have a easy way to turn off the light. It can be confusing to have it as an option.

@wilco375
Copy link
Author

I think it must 1 but we need to make the icon clickable before to have a easy way to turn off the light. It can be confusing to have it as an option.

I'm not sure I follow what you mean; the icon is already clickable, right?

@piitaya
Copy link
Owner

piitaya commented Nov 16, 2022

With Home Assistant Tile card, you can assign different action for icon and info tap_action. We should add this to Mushroom too to allow user to easily turn off the light (by clicking the icon).

@wilco375
Copy link
Author

wilco375 commented Nov 17, 2022

With Home Assistant Tile card, you can assign different action for icon and info tap_action. We should add this to Mushroom too to allow user to easily turn off the light (by clicking the icon).

Maybe I'm misunderstanding you, but changing the tap action is already possible in Mushroom, right?
image

Or is your point that some users might have changed the tap action and use swipe to turn off the light?

@antonio1475
Copy link

antonio1475 commented Nov 17, 2022

Maybe I'm misunderstanding you, but changing the tap action is already possible in Mushroom, right?

Mushroom has tap actions for the whole card (outside of the slider)
The new Tile Card splits the tap_action between the whole card and the icon:
image
(one action for the icon in yellow, one action for the rest of the card in green)

In any case, @piitaya I don't really see the dependency to that, since mushroom already has even more (3) actions (Tap, Hold, Double Tap), so users can still configure any of those 3 to toggle (which is the default anyway). No?

Or is your point that some users might have changed the tap action and use swipe to turn off the light?

That is of course a risk, but it could be announced as a Breaking change and users can re-configure any of the 3 actions to toggle?

@wilco375
Copy link
Author

Mushroom has tap actions for the whole card (outside of the slider) The new Tile Card splits the tap_action between the whole card and the icon

Ah, thanks for the clarification

@samuolis
Copy link

@piitaya Is it hard to add support for mushroom cards to allow icon click? Maybe you have timeline when it will be available in mushroom?

@krimo80
Copy link

krimo80 commented Mar 5, 2023

When will this fix be applied to the release..? ;-)

@Snellingen
Copy link

What is blocking this fix right now? Is it that this change has have an option for the card to set min/max for the slider to avoid breaking changes?

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.

[Bug]: Swiping a light down to the lowest brightness turns it off
6 participants