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

Convert wear home activity to compose #1844

Merged
merged 11 commits into from
Nov 3, 2021

Conversation

dshokouhi
Copy link
Member

@dshokouhi dshokouhi commented Oct 27, 2021

Summary

Fixes: #1803 and #1838

Converts the home activity to use Compose. Added a few more entity domains with support for more. This requires a minimum SDK bump to API 25 as that is required for compose on Wear OS. There is also a loading screen while we wait for the list of entities to load

Screenshots

image

image

Link to pull request in Documentation repository

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

Any other notes

@leroyboerefijn
Copy link
Contributor

This is really awesome! I will give it a test spin later today.

My first comment based on your screenshot is that I would prefer to make both the chips and the margins a bit smaller (like in the current release). How do you feel about that?

@dshokouhi
Copy link
Member Author

Good suggestion, will look into that later today :)

@leroyboerefijn
Copy link
Contributor

leroyboerefijn commented Oct 28, 2021

Again, great work! It works really well, and I like this API a lot :) Also, the default scroll animations are great!

A few small design remarks from my side:

  • The top margin is a bit too small for me, 'input booleans' almost falls off in the corners
  • I think you can decrease the side margins and the margins between chips as well. Maybe make it look a bit more like the left lists example on this page: https://developer.android.com/training/wearables/compose#lists
  • Why is the logout button a button and not a chip as well? For me the icon and text are differently aligned than in all other chips (I know I'm nitpicking a bit :P)
  • I have a few entity names that take up three lines, for those the third line only shows partially. Is it possible to limit it at two lines?
  • Could you add a Scaffold? https://developer.android.com/training/wearables/compose#scaffold
  • I feel that it would be nice to have the headers centered, but it is a matter of personal taste of course
  • The Rotary input doesn't work to scroll the list. I'm not sure how to do this in compose though. For recyclerview we had to request focus I believe

@dshokouhi
Copy link
Member Author

I like this API a lot :) Also, the default scroll animations are great!

Couldnt agree more, the flow is just so much easier than having to deal with XML and all that comes with it lol

  • The top margin is a bit too small for me, 'input booleans' almost falls off in the corners

yea I wasnt really sure how I felt about it, a lot of the heading design was based off how much text this took on screen. I suppose we can try to make the text smaller so it can be center aligned. Only concern is what happens if the translated language takes up more space? Calling it booleans doesnt make sense either :/

  • I think you can decrease the side margins and the margins between chips as well. Maybe make it look a bit more like the left lists example on this page:

yea as of now I went with a default design from an article I read that covered ScalingLazyList :) I can change the margins a bit to match the prod version more. I am not much of a design guy but can def take a stab at making it better lol

  • Why is the logout button a button and not a chip as well? For me the icon and text are differently aligned than in all other chips (I know I'm nitpicking a bit :P)

umm because I was playing around with the API 😂 will convert it to a chip so it matches the rest LOL my bad

  • I have a few entity names that take up three lines, for those the third line only shows partially. Is it possible to limit it at two lines?

Good one, we can probably just trim the name to a certain amount of characters so we don't go over it.

  • Could you add a Scaffold?

I can look into :)

  • I feel that it would be nice to have the headers centered, but it is a matter of personal taste of course

yea I was trying to do with that TextAlign but it did not work as intended 😬will look to see if we can center it better

  • The Rotary input doesn't work to scroll the list. I'm not sure how to do this in compose though. For recyclerview we had to request focus I believe

i dont know if its ready for compose yet, I saw they have FocusRequester but that only works on input fields. Still haven't figure out how to add that back yet but its possible to be a missing feature ATM?

@leroyboerefijn
Copy link
Contributor

Couldnt agree more, the flow is just so much easier than having to deal with XML and all that comes with it lol

Haha yes definitely!

Only concern is what happens if the translated language takes up more space? Calling it booleans doesnt make sense either :/

Hmm yes that is a good one... I'm not sure how to handle different translations like that... For now we can just focus on English, right? Let's see what happens once we get translators

I am not much of a design guy but can def take a stab at making it better lol

Haha no worries, I'm sure it will turn out fine!

umm because I was playing around with the API 😂 will convert it to a chip so it matches the rest LOL my bad

Hahaha yeah I can imagine ;)

i dont know if its ready for compose yet, I saw they have FocusRequester but that only works on input fields. Still haven't figure out how to add that back yet but its possible to be a missing feature ATM?

Maybe it is indeed not possible yet then. In any case, if it's too much trouble we can always worry about it later ;)

@dshokouhi
Copy link
Member Author

  • Material theme and scaffold added with scroll position indicator
  • logout converted to chip
  • all chips now take up the whole space like original design
  • headers are now center aligned
  • friendly name is trimmed to 30 characters as that was the cut off I noticed on my watch

@leroyboerefijn
Copy link
Contributor

  • friendly name is trimmed to 30 characters as that was the cut off I noticed on my watch

I'm seeing this just now, you can set max lines and type of overflow for a chip. I think that would be better than a maximum nr of characters. Sorry for not seeing this earlier. https://developer.android.com/training/wearables/compose#chips

@dshokouhi
Copy link
Member Author

  • friendly name is trimmed to 30 characters as that was the cut off I noticed on my watch

I'm seeing this just now, you can set max lines and type of overflow for a chip. I think that would be better than a maximum nr of characters. Sorry for not seeing this earlier. https://developer.android.com/training/wearables/compose#chips

Thank you! no need to apologize when you show a better path :)

@leroyboerefijn
Copy link
Contributor

Great update! It's looking really nice :)

@apo-mak
Copy link
Contributor

apo-mak commented Oct 29, 2021

@dshokouhi
Thank you very much for the "home view" overhaul, i was trying to find time to sit down and re create all of it with manual lists and xml's 😂 to fix this #1803 and now is wayyyy better 😄 .

Quick question .. now that we do not have an " EntityButtonViewHolder.kt" and we show icons in a different way do we still need the icon drawable ? ( ex. ic_light.xml ) https://github.com/dshokouhi/home-assistant-android/blob/wear_home_compose/wear/src/main/res/drawable/

p.s. can you also add this and the other merged request in the Wear OS Project
https://github.com/home-assistant/android/projects/2
:bowtie:

@dshokouhi
Copy link
Member Author

Quick question .. now that we do not have an " EntityButtonViewHolder.kt" and we show icons in a different way do we still need the icon drawable ? ( ex. ic_light.xml )

ah good catch, more files to remove

@JBassett JBassett linked an issue Oct 29, 2021 that may be closed by this pull request
@dshokouhi
Copy link
Member Author

There is now a loading screen while we wait for the viewmodel to update

image

@JBassett JBassett merged commit b6e0837 into home-assistant:master Nov 3, 2021
@hkusulja
Copy link

hkusulja commented Nov 3, 2021

Thank you so much for contribution for my github feature list here.
I am awaiting Sunday so I can get beta from Google Play Store and try it.
Question is, is it possible to have multiple tiles, and where / how to defined what is on tile list and what is not?
For example, I have lot of lights and switches, and just some of them want that are available on first / main tile for me. And some others on another tile, anyhow, not all should be visible on tiles. Is this managed somewhere in HA web or mobile app or somewhere alse?

@leroyboerefijn
Copy link
Contributor

@hkusulja Tile support will be added in a separate PR and will have to wait a tiny bit longer. Right now it will only support one tile with 7 shortcuts. It will be merged once I finish the settings in the wear os app, to select which 7.

The same settings will be added to the android app as well, somewhere in the future. Multiple tile support is not considered yet, but could always be a feature request of course

@hkusulja
Copy link

hkusulja commented Nov 3, 2021

Ok, no problem about waiting, this team is already doing a great job.
It would be easier to choose / manage tile settings on the phone app, instead on the watch directly, but ok.
One fully working tile is fine as a priority and later for multiple tiles, i will check and create github feature request.

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.

Wear OS - switches control Wear OS - Lights list is empty
7 participants