search

WebThingsIO / android-app

A Web of Things client for Android
Mozilla Public License 2.0
6 stars 3 forks source link

Basic screen with topmenu #59

Closed arist0v closed 2 years ago

arist0v commented 2 years ago

Create a Basic UI with top Menu

tim-hellhake commented 2 years ago

@benfrancis the code looks ok, I think we can merge it. I made a screenshot to save you some time:

image

arist0v commented 2 years ago

change done

tim-hellhake commented 2 years ago

@benfrancis image

arist0v commented 1 year ago

Have you look if it's used on the next pr??(i based my code on how to i found so maybe it's just because i try to split my pr ;-) )

Le jeu. 12 mai 2022, 20 h 04, Tim Hellhake @.***> a écrit :

@.**** approved this pull request.

The unused setCurrentScreen method is still in there but I don't want to be too pedantic. It's up to @benfrancis https://github.com/benfrancis now 😄

In app/src/main/java/io/webthings/app/utils/MainViewModel.kt https://github.com/WebThingsIO/android-app/pull/59#discussion_r859090445 :

  • fun setCurrentScreen(screen: NavRoutes){

  • _currentScreen.value = screen

  • }

As far as I can see, nobody calls this setter. Typically you shouldn't commit any dead code. Either is not necessary or belongs to another commit. ⬇️ Suggested change

  • fun setCurrentScreen(screen: NavRoutes){

  • _currentScreen.value = screen

  • }

In app/src/main/java/io/webthings/app/utils/MainViewModel.kt https://github.com/WebThingsIO/android-app/pull/59#discussion_r869665660 :

  • fun setCurrentScreen(screen: NavRoutes){

  • _currentScreen.value = screen

  • }

@arist0v https://github.com/arist0v any thoughts on this?

— Reply to this email directly, view it on GitHub https://github.com/WebThingsIO/android-app/pull/59#pullrequestreview-953924599, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABSPAADPGD7D5PDILFI5YMTVJWL73ANCNFSM5UMP5F4Q . You are receiving this because you were mentioned.Message ID: @.***>