-
-
Notifications
You must be signed in to change notification settings - Fork 650
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
Add option to make tap on entity state complication toggle the corresponding device. #4073
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
common/src/main/java/io/homeassistant/companion/android/database/AppDatabase.kt
Outdated
Show resolved
Hide resolved
@@ -795,6 +795,7 @@ | |||
<string name="show_share_logs">Show and share logs</string> | |||
<string name="show_entity_title">Show entity name</string> | |||
<string name="show_unit_title">Show entity unit</string> | |||
<string name="forward_taps_title">Forward taps to entity</string> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reading this, I was very confused about what the option will actually do. While it may be because I'm not a native English speaker, that will mean some other people probably also won't understand it.
Why not mimic what the entity state widget on the phone does (which is much more obvious in my opinion) and make it tap action: toggle or tap action: refresh?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello,
Thanks for the feedback! I did think quite a bit about the wording and was left unsatisfied too :D
- the word "toggle" seemed a bit too restrictive to me: indeed, depending on the entity domain, it can execute the following services:
- toggle
- lock / unlock
- arm / disarm alarm
- press (for a
button
/input_button
) - turn on (for a
scene
)
Even if we take "toggle" as a loose term used to convey flipping between two states, it does not include the last two cases where the action is unary.
That being said, maybe given the precedent set by the widget (that I was unaware of), and the fact that it might be easier for users to understand (even though technically less accurate), we should go for it?
-
it might seem to the user that they are choosing between two exclusive options: refresh OR toggle. In actuality, both would refresh the complication - with one additionally triggering the entity prior to the refresh.
-
material3 / compose for wear do not appear to have a way to create drop-down dialogs easily.
Proposal: how about keeping this toggle, and changing the text to "Toggle entity when tapped" to make it more user-friendly and consistent with the widget?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Naming things is often harder than building them :D
the word "toggle" seemed a bit too restrictive to me
While I can understand your points, your two "That being said" points feel more important to me than being technically correct.
- consistency with the widget/other parts of the app for what is the same feature, instead of giving different names for the same feature
- when you don't know Home Assistant inside out, it is easier to understand what will happen when you "toggle a scene/toggle a button" vs "forward tap to a scene/forward tap a button", even if it is technically not toggling
it might seem to the user that they are choosing between two exclusive options: refresh OR toggle
I'd still expect feedback when I tap something which in addition to doing the thing, would be refreshing to show the updated state, so it's implied?
keeping this toggle, and changing the text to "Toggle entity when tapped"
The issue with keeping this as a toggle is that it is unclear what happens when you uncheck it - there is no clear opposite. The native pattern for dropdowns on a watch would be 2 chips/buttons that are radio buttons.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a clear opposite to "toggle entity when tapped": "don't toggle entity when tapped" :) IMO that's quite intuitive given the usual semantics of toggles.
I defer to your opinion in this matter - but my 2 cents is that "toggle" is not ideal. On top of the cases I listed above, consider the case of a sensor entity - how are users expected to understand "toggle" in this context? It sounds like it would be enabling / disabling the sensor, doesn't it? In any case, this is outside the scope of this PR since it would require refactoring the other uses of the "toggle" word in similar context (eg. the widget). Just wanted to give my opinion :)
Will work on the radio buttons later. Thanks for the discussion!
Edit: RadioButtons were added (properly) in wear compose 3 weeks ago, apparently missing the 1.3 beta cut. Maybe I can duct tape something together using ToggleButton with (the old) RadioButton inside, but that does not seem great for the reasons highlighted in that commit: "This differs from the existing ToggleButton in that RadioButton is selectable (and operates within a selection group) whereas ToggleButton is toggleable (and is independent).".
…ponding device. Currently, tapping a complication only refreshes the entity state. This PR adds a "toggle entity when tapped" option to the complication configuration page. When this option is enabled, tapping on the complication triggers a call to the #onPressed method of the corresponding device entity, which in turns executes a RPC to the corresponding service. This lets users quickly toggle home assistant devices with a single click on their watch face.
Going to mark this as draft while waiting for radio buttons (either the 'official' one or manually mimicking the behavior). Assuming there are no big issues you can update the dependency once it reaches alpha02/03 - we've done so before to get access to components that were needed on Wear. |
Summary
Currently, tapping a complication only refreshes the entity state. This PR adds a "toggle entity when tapped" option to the complication configuration page. When this option is enabled, tapping on the complication triggers a call to the #onPressed method of the corresponding device entity, which in turns executes a RPC to the corresponding service. This lets users quickly toggle home assistant devices with a single click on their watch face.
Screenshots
Link to pull request in Documentation repository
Documentation: home-assistant/companion.home-assistant#1015
Any other notes