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

Add option to make tap on entity state complication toggle the corresponding device. #4073

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

clju
Copy link

@clju clju commented Dec 26, 2023

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

screen

Link to pull request in Documentation repository

Documentation: home-assistant/companion.home-assistant#1015

Any other notes

Copy link

@home-assistant home-assistant bot left a comment

Choose a reason for hiding this comment

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

Hi @clju

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

@@ -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>
Copy link
Member

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?

Copy link
Author

@clju clju Dec 26, 2023

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

  1. 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?

  1. 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.

  2. 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?

Copy link
Member

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.

Copy link
Author

@clju clju Dec 28, 2023

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.
@jpelgrom
Copy link
Member

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.

@jpelgrom jpelgrom marked this pull request as draft January 16, 2024 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants