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

Editing wear favorites from the phone #1908

Merged
merged 20 commits into from Nov 23, 2021

Conversation

dshokouhi
Copy link
Member

@dshokouhi dshokouhi commented Nov 11, 2021

Summary

There is a lot going on here but I wanted to get the working code submitted early in case it can help anyone else looking to do something similar. Also a good time to think about the layout of phone settings overall.

  • Get list of entities
  • Get favorites from wearable
  • Load entities and favorites along with selected favorites via checkbox
  • Update favorites on wearable
  • Recreate app bar in compose
  • Re-organize code

Screenshots

image

image

Link to pull request in Documentation repository

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

Any other notes

Got a lot to do and we need to think about the design. Such as when a user has more than one wearable. How to access each of the settings etc...

@dshokouhi dshokouhi marked this pull request as ready for review November 12, 2021 17:59
@dshokouhi dshokouhi changed the title WIP: Begin work on editing favorites from the phone Editing favorites from the phone Nov 12, 2021
@dshokouhi dshokouhi changed the title Editing favorites from the phone Editing wear favorites from the phone Nov 12, 2021
Copy link
Collaborator

@JBassett JBassett left a comment

Choose a reason for hiding this comment

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

This needs to be rebased/merged so you can do you dependency injection with Hilt now.

@leroyboerefijn
Copy link
Contributor

Great work! Awesome that we're starting to be able to change the wear app settings from the phone :)
Below a few feature requests. But please let me know if you think it's better to do this in a separate PR.

  • Could you display friendly names in the settings? Or would you say that is for a follow up PR?
  • Now when I make changes in the settings on the phone, it doesn't automatically update. It also doesn't update when I go home and re-open the app. Would it make sense to send a broadcast in PhoneSettingsListener, which is then picked up somewhere to update the list in the MainViewModel?

@dshokouhi
Copy link
Member Author

Great work! Awesome that we're starting to be able to change the wear app settings from the phone :)

Thank you I appreciate that! This one was a bit of a challenge to get all the pieces working together but luckily a lot of the foundation was already there from your previous PR for sending the HA instance over :)

  • Could you display friendly names in the settings? Or would you say that is for a follow up PR?

I could do that but then it might make sense to show a category saying these are Input Booleans or what not. I opted for entity ID so users know which entity it is. Honestly I figured we could redo the Rows to show the icons etc... in a more friendly way 😁

Whatever we do here though should probably be reusable so maybe making the Row a better compose element makes the most sense. We have a few places to add these to like QS tiles and Shortcuts for example. But neither of those were updated to compose yet.

  • Now when I make changes in the settings on the phone, it doesn't automatically update. It also doesn't update when I go home and re-open the app.

So updating Wear when the app is open is something I couldn't get to work when I submitted this. However when the app was re-opened it always pulled in the latest favorites which were updated from the phone.

Would it make sense to send a broadcast in PhoneSettingsListener, which is then picked up somewhere to update the list in the MainViewModel?

I think a broadcast might be helpful here I'll have to look into that. 🤔, honestly didn't think about it since I felt the most important parts were already done 😂

@leroyboerefijn
Copy link
Contributor

I could do that but then it might make sense to show a category saying these are Input Booleans or what not. I opted for entity ID so users know which entity it is. Honestly I figured we could redo the Rows to show the icons etc... in a more friendly way 😁

Whatever we do here though should probably be reusable so maybe making the Row a better compose element makes the most sense. We have a few places to add these to like QS tiles and Shortcuts for example. But neither of those were updated to compose yet.

That makes a lot of sense indeed. I suppose we can reuse that row composable as you say. Maybe we can add a main label with the friendly name (if it has one) and a secondary label with the entity ID? Then the next question is do you want the icon on the left and the checkbox on the right? Or the other way around? Or both on one side? :P

So updating Wear when the app is open is something I couldn't get to work when I submitted this. However when the app was re-opened it always pulled in the latest favorites which were updated from the phone.

Interesting! Somehow that doesn't work for me. I really need to fully close the app with the back gesture, instead of just the home button...

I think a broadcast might be helpful here I'll have to look into that. 🤔, honestly didn't think about it since I felt the most important parts were already done 😂

Oh that is definitely true! This is more a finishing touch on the user experience :)

@dshokouhi
Copy link
Member Author

dshokouhi commented Nov 16, 2021

Interesting! Somehow that doesn't work for me. I really need to fully close the app with the back gesture, instead of just the home button...

ok that makes sense then because currently favorites only update in onCreate so I think a broadcast receiver to update the view model will be necessary then to trigger the update. I always went to the home screen by using the gesture during my testing :)

Maybe we can add a main label with the friendly name (if it has one) and a secondary label with the entity ID? Then the next question is do you want the icon on the left and the checkbox on the right? Or the other way around? Or both on one side? :P

Yea lots of considerations here, we could also try to make our own chip to help match the wear UI 😂

@leroyboerefijn
Copy link
Contributor

Yea lots of considerations here, we could also try to make our own chip to help match the wear UI 😂

That would be a fun one haha 😁 But would indeed be nice to have a common design element

@dshokouhi
Copy link
Member Author

Ok fixed the issue regarding favorites duplicating. I also added some code to update the favorite entities upon resuming the app. It was a bit difficult to try to get a receiver setup and its actually not correct. What we should do is save the entries to the DB and use a Flow of those values so its always updating just like we do today for entity updates.

@dshokouhi
Copy link
Member Author

Hope I addressed them all appropriately :)

@dshokouhi dshokouhi mentioned this pull request Nov 22, 2021
5 tasks
@JBassett JBassett merged commit a88704a into home-assistant:master Nov 23, 2021
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

4 participants