https://kotlinlang.org logo
Join SlackCommunities
Powered by
any thoughts about following? <https://github.com/...
# confetti
j
any thoughts about following? https://github.com/joreilly/Confetti/pull/688
y
No concern about ditching ViewModels, I think the Compose ecosystem already has a split between pure Compose can manage all your state, and the Android folk who want to keep those lifecycle objects. I don't think a KMP project needs to introduce ViewModels outside of Android, so finding solutions and ditching sounds appealing.
I don't know on the specific choice of Decompose vs Voyager vs Was there one more?
j
I don't know these in enough detail but seems like particular approaches to navigation shouldn't require as big a change to overall app architecture.
my 2c is that I like what KMMViewModel for example offers in terms of ease of observing UI state in SwiftUI.....but certainly other approaches that can be used there
y
I wouldn't rush this, and maybe worth having a bit of a bakeoff.
My gut says KMP projects would be better off without Cargo Culting ViewModels into iOS.
j
I suppose the other aspect of that is that MVVM is quite common in iOS projects that use SwiftUI
so having somewhat of an aligned architecture has some benefits
a
That still looks like MVVM. I think AndroidX ViewModel is just an unfortunate naming. E.g. it has nothing about MVx, but the whole purpose is retaining over configuration changes. If there would be no such thing as configuration changes, I believe there would be no need in AndroidX ViewModel. So, you can consider Decompose components as ViewModels, just not surviving configuration changes by default (handled separately). The way you implement your MVx is up to you.
In this particular case (Confetti), there is the app-scoped repository which handles configuration changes just fine. So from my point of view even on Android, ViewModels do not provide any real benefit. On the other hand, there is now proper DI - all 
configure
methods are gone, as well as 
lateinit
properties.
Also fun fact - during development I had only two crashes - one because of Koin (some deps were not properly provided), and another one because I forgot to remove some lateinit properties. The latter is now eliminated. Theoretically, Koin can also be replaced with manual DI, so the project will become fully safe. 😀
j
btw I thought we'd got rid of those 
configure
methods thanks to https://github.com/joreilly/Confetti/pull/660
ah, nvm, issue in my search! Looks like we have them in a few places still
One other thing to note here is that a lot of functionality got added in quite a short period of time as we came up to KotlinConf and AndroidMakers.....I think feeling was that we probably would need to go back and perhaps refactor/simplify certain things but haven't had chance yet to do that
Re. "the ViewModel plays two roles: 1. It retains in the back stack; 2. It retains over configuration changes." (from PR) The other key function I see here (and probably I think more important than ones mentioned) is as a container of observable UI state. Of course this doesn't necessarily have to be something called a "ViewModel".....just I guess what that is often understood to provide. Naming is always hard of course but at least I think there's somewhat of a common understanding of function it generally provides.....and I do think having some kind of common vocabulary like that (particularly between iOS and Android) does offer some value.
Particularly also with official Google guidance generally talking about use of 
ViewModel
😉
Have pulled down the branch and trying it out.....certainly seems like navigation has been simplified
y
is as a container of observable UI state
I think this is where the pure Compose folk want remember, rememberSaveable etc.
j
what I think also affects how that's done is that they're also generally responsible for transforming state from lower layers in to that observable UI state.....again, other "things" can provide that function and google guidance also talks I think about general state containers
y
This was the debate I was thinking of, although I've seen you folk on a number of these types of threads 🙂
j
you can understand how people starting off can find things quite confusing when you see discussions like that 😃
y
Agree. I'm fine going either way.
j
it's a more philosophical point perhaps but this is why I've liked Google steadily becoming more opinionated in their guidance
it may not be perfect but provides people starting off with something they can refer to and use
y
100% the guidance is great. But I also recognise that a lot of Google investments really aren't designed for KMP apps, but are designed to have amazingly good backwards compatibility with Android. Room, Hilt, Navigation, gRPC.
j
yeah, that's a good point
will be interesting as Google provides more KMP ready stuff whether that will change much
@Arkadii Ivanov so in Decompose the 
Component
provides much the same functionality as ViewModel (at least in terms of the containing/creation of observable UI state)?
The other aspect of this, as you mentioned in PR, is retention on back stack and as you said it provides that as well
@yschimke what are your thoughts on how Decompose would fit in for the Wear OS client?
I'm curious about the iOS client but it seems like something at least quite similar to what KMMViewModel provides is available
a
In Decompose, the Component thing is closer to Android Fragment. You can navigate between them, but you don't need to extend any class. On iOS, it is possible to leverage the native SwiftUI navigation UX. And also observe state changes.
j
but the Component is typically where you maintain observable ui state?
a
The component is just a facade behind whatever implementation details. It could be a simple StateFlow, or an MVI Store, or a repository. There could also be a retained instance (kinda ViewModel) inside holding the state. A component itself is never retained, because retained fragments looked extremely error prone to me.
j
In the PR for example it looks like 
SessionsSimpleComponent
does at least something similar to what ViewModel did before....in terms of transforming data from the repository etc in to UI state?
a
All ViewModels are converted almost one-to-one to components. Just leveraged better DI. Turned out that instance retaining is not needed, thanks to the app-scoped repository. Also added extra components for better navigation - ConferenceComonent - hosts the tree once a conference is selected, HomeComponent - hosts the tree of tab screens.
j
I wasn't clear on what you meant by "instance retaining" in this case.....the data from repository still needs to be transformed to UI state....is it this what you're referring to?
or do you mean for example the instances etc that Koin is managing?
a
If I understand it correctly the repository lives in the app scope and loads and holds the entire state of the application. So on a configuration change, it's ok to recreate components. They just re-read the latest state from the repository. I have tested it manually, when I scroll a list, then rotate the screen to landscape and back to portrait - there is no loading indicator, and the scroll position is restored.
j
ah, ok...that makes sense
not certain if this will apply the same way for wear os app which uses somewhat different approach for fetching/caching data
a
I'm sure we'll find a way, it's possible to retain instances over configuration changes if needed. Is it actually a thing in Wear OS?
j
I don't think so.
unless there's other configuration changes that could trigger that
but, in case, doesn't seem like that should be issue based on what you memtioned
a
Sure! Take your time and evaluate. Let me know if you would like me to try iOS or wear or something else.
j
@xxfast I guess you're already sold on Decompose? 😃
@Arkadii Ivanov I would definitely be interested in seeing how this would look on iOS if you did have chance to make some changes to support that? I do as mentioned like KMMViewModel but have other samples that use it and be nice perhaps to have something else to compare it to 🙂
will defer to @yschimke about use for Wear OS.
and the Compose for Desktop client right now is extremely basic but I'm sure could use this approach as we expand it
@mbonnin any thoughts on going further with this?
a
I agree, it would be beneficial to demonstrate different approaches. I haven't touched anything apart from Android yet. Will try iOS next then. Thanks!
m
@mbonnin any thoughts on going further with this?
Will have to dive into this more but I agree generally that we shouldn't need to handle process restoration at the (androix) ViewModel layer and that using them feels a bit out of place (because we don't really need anything of what makes them special on Android)
Re: trying to keep the project simple, it'd be cool to have only one navigation lib in the project. IIRC the iOS navigation is pretty different from the Android one right now. If we could share it, that'd be cool.
j
even if we couldn't/didn't do that (share with iOS) I think it would be great to have common approach to navigation across the Compose based clients
m
Agreed
y
I can't see why it won't work on Wear. Given we have a POC already thanks to Arkadii
Happy to try
x
Sorry i'm a little late to this conversation Re: ViewModels, yeah this is a loaded word and it's really just a holder for your state flow that Android needs. Sidenote: with decompose-router, i've decided to make this library architecture agnostic by refactoring any references to a view-model out, and it is now called 
rememberOnRoute
which i feel reflects its true purpose
Copy code
fun ListScreen() {
  val instance: ListInstance = rememberOnRoute { savedState -> ListInstance() }
}
Re: Using decompose for navigation, I think most of the multiplatform navigation libraries out there makes you to inherit some class in order to define a screen - which I think is very un-compose like (where the semantic view tree is not exposed as a inheritance hierarchy). With Decompose, you can make it more compose-like with decompose-router, and for non-compose projects I would definitely go with just Decompose
j
@Stylianos Gakis I'm assuming this would also have impact on changes you were making in https://github.com/joreilly/Confetti/pull/592
a
Oh, I think my PR should certainly fix #551 mentioned there.
s
Yeah it would basically make this PR obsolete, in case we’re not using the save/restoring state that androidx.navigation is doing. Not sure how decompose handles going through bottom nav items and preserving multiple backstacks, but it’d definitely be a replacement of what I was trying to do there. (ps sorry for not finding the time to wrap that PR up, really haven’t had the time to address the comments there until now. If we switch from androidx.navigation anyway we can close that PR and never merge it though of course)
a
A bottom bar screen goes into the back stack when a another tab is activated. The activated tab is brought to front if exists in the back stack, or a new screen is created. Nested stacks are also supported. I also noticed there is something about deeplinks. I think we can add deeplinks separately, either before Decompose and then I would cherry-pick, or just after that in top.
s
And I assume decompose also supports having the first bottom bar destination always staying in the backstack (with its internal backstack and everything), so that pressing back goes there first and then pressing back again goes out of the app, instead of leaving the app when pressing back from any of the bottom destinations right?
a
Could you please elaborate what you mean by "pressing back goes there first and then pressing back again goes out of the app"?
s
Open the app and you are met with the “Schedule” screen. Then you press “Speakers” and see the speakers screen. Doing the back gesture puts you in the “Schedule” screen again. Doing the back gesture again starts the predictive back animation which shows that the app will be closing, and as you finish the gesture you end up in your launcher screen.
The “wrong” flow would be: Open the app and you are met with the “Schedule” screen. Then you press “Speakers” and see the speakers screen. Doing the back gesture again starts the predictive back animation which shows that the app will be closing, and as you finish the gesture you end up in your launcher screen. Which would mean exiting the app from a place which isn’t the “start” destination. This makes it an unintuitive experience as you can exit the app by pressing back from multiple places.
a
I remember there was a material guideline saying to not switch tabs with the back button and just close the app. But it's gone now. Currently in my PR the app is closed when you press the back button. I can enable the back button handling for this case if needed.
s
Aha, I am fairly certain the guidelines at least now are exactly the opposite of what you say above. Maybe this suggestion has changed throughout the years, it’s very likely. The official docs about supporting multiple backstacks https://developer.android.com/guide/navigation/multi-back-stacks#manual very specifically make the code which runs when you press a bottom nav to: • restore and save the state when doing these navigation movements • pop up to the start destination before doing so (very important that it’s not inclusive, so the start destination stays in the backstack at all times) so that you alwys got the start destination in the backstack, and all other bottom nav destinations are on top of it. • With this pop up to start detination action + doing 
singleTop = true
it means that as you are pressing the bottom nav items, you’re never building up a big backstack. The backstack always looks like just 
startDestination
or on 
startDestination + any of the other bottom nav destinations
. This automatically makes it so that back button does not exit the app unless specifically on the start destination. You can look in NowInAndroid app as well which behaves as I describe here.
a
Cool, I will enable this behaviour, no problem at all.
x
Yeah I think material guidelines on this flip-floped a few times, and I find the singleTop behaviour unintuitive, as a user (at least for me personally).
a
singleTop
hierarchy for tabs looks good, because it means that the state is preserved for tabs previously opened. But yeah, popping the tabs stack by the back button might be unintuitive. With Decompose, we can have the former without the latter (the current behaviour in the PR). I'm fine with either.
Sorry for the delay, I'm a bit overwhelmed with my regular job. This is still on me!
j
Yeah, no rush at all....this is just a side thing for all of us 😃
a
@John O'Reilly Do you mind bumping min iOS version from 16 to 16.1? Asking due to https://stackoverflow.com/questions/73978107/incomplete-swipe-back-gesture-causes-navigationpath-mismanagement
Otherwise I will interop via UINavigationController on pre 16.1
j
Yeah, that should be fine I think
a
Awesome, thanks!
I have updated the iOS app, finally 😀
@John O'Reilly The automotive target has been recently disabled. Should I somehow migrate that now, or keep the code commented?
j
We disabled it so we could publish to play store (as we were having issue getting it accepted with it).....I think idea was that we could re-enable it at least for local tests/demos.....or maybe only enable in debug builds. cc @cafonsomota
I think only changes to re-enable again is to just comment back those android manifest settings
(and, as mentioned, perhaps we can have debug specific version of that manifest file that has those enabled)
or at least publish release version temporarily to internal testing channel on play store (as we are able to do that with auto enabled) if say demo/talk needed that
c
by automotive you mean auto, right @Arkadii Ivanov? 🙂 we've been rejected by the Play Store and I haven't still managed to understand why; I have some time this week and will give it a fourth try
m
Took me sooo much time to realize automotive != auto 😅
a
Yes, automotive) the naming comes from the docs - https://developer.android.com/training/cars/testing#test-automotive-os
Thanks, I will try enabling and updating the code.
j
it's pretty easy to setup "DHU" to test it https://developer.android.com/training/cars/testing
y
I just installed the auto emulator in AS giraffe. Is that better?
c
but isn't Arkadii talking about automotive?
a
That's the same?
y
Oops should have read up.
c
DHU (desktop head unit) is for android auto, which is what confetti supports; you can see it as a projection of our app in the car; automotive is an OS for the car which has it's own apps installed
there's also the emulator for the car for Android automotive though
y
Yep, the auto emulator in AS works for the one confetti supports
I haven't ever run the DHU
a
Oh ok! Then I talked about Android auto 😀
c
the naming is not the most clear, I know 😅
it was what Martin says, it takes some time to get used when we say automotive and auto x)
j
Yep, the auto emulator in AS works for the one confetti supports
Cool, didn't realise you could do that....must try it
a
@John O'Reilly The PR is ready for review, all checks have passed on my fork.
b
Sorry I didn’t participate before! I’ll just say that I liked KMMViewModel because the ViewModel pattern is very familiar to Android developers 😅. I’ve always been a fan of using “standard” libraries, by that I mean the ones from the Android team + mainstream ones. I realize this doesn’t work for us since we’re multiplatform and there’s nothing for navigation. I also understand one goal of Confetti is to try different/new things! Which is very cool! 😄 My (probably very superficial) impression from the PR is that Decompose is kind of an impactful library and that I personally would rather use something more “lightweight” (but what?), - honestly, I haven’t dug too much, and maybe that’s not the case, or it’s not an issue for us.
j
anyone else by chance getting following (trying to checkout this PR)?
Copy code
gh pr checkout 688
From <https://github.com/joreilly/Confetti>
 ! [rejected]          refs/pull/688/head -> decompose  (non-fast-forward)
failed to run git: exit status 1
y
Do you have that branch checked out already?
Maybe history rewritten?
j
yep, that was the issue....I'd forgotten I'd checked it out before
@Arkadii Ivanov small thing but looks like the various 
Component
classes in 
mobileMain
shared code are still in ViewModel classes....assume that's probably just function of how code has been transitioned....easy thing to update anyway
also, I guess if/when some of those components can be shared with likes of Compose Desktop client as well we can move in to 
commonMain
(along potentially with moving Compose code itself in to common code....but that's for another day!)
a
I think those are just file names ending "ViewModel". Yeah, I can rename files too!
I think all components could be moved to commonMain, assuming that dependencies could be satisfied
j
btw is this something we could/should use in Wear OS app as well? What do you think @yschimke?
y
Yes, I'm happy to follow up
I'd love wear to use much more of the same code apart from UI
j
Ok, sounds like we're happy to merge this? We can fine tune things as needed once it's in.
m
Sorry didn't have much time to look into this but yes definitely I'm happy to see this merged!
j
ok, just merged!
a
Thanks for the review and feedback!
j
Latest version of app (including these changes) just published to play store (internal testing)....version 1.0.187
a
With uncommented Auto things in the manifest? I remember you mentioned some issues with publications.
j
It's only an issue if we try to promote beyond internal testing channel
a
Ah, got it! You can try now btw! Who knows.
j
we tried a few times with some updates @cafonsomota made but was rejected each time 😞
y
I think you need audio playback of sessions to make a compelling case. I don't think the nav case really works :)
a
Depends on the rejection reason. Maybe they didn't like a dependency that has been removed (or could be now removed).
c
I need to see if we can stream the videos as audio (it will also be helpful for my commute time x))
y
That should definitely be possible with exoplayer.
If you add on Auto, I'll add on Wear as well
j
That would be really cool @cafonsomota!
y
Is there a conference with an easy to link feed of videos?
c
deal! today is a bit difficult but I'll see that tomorrow 🙂
I know that droidcon has this support, but I think we need to add the match with the talk manually
or write a script to match the talk with the video title x)
y
Best done in the backend I hope
c
yup 🙂
j
the sessionize data doesn't generally get updated with video links etc?
c
good question, how do we have access to it? is it public?
m
Not sure if it's in the sessionize data but I have something like this for Android Makers. I'll add to the backend tonight
@cafonsomota I'm guessing Youtube/Vimeo doesn't work, right? You'll need plain 
.mp4
?
y
Fwiw, maybe include multiple links with some metadata.
Be nice to be able to choose to link out to YouTube, or download link
m
Youtube/Vimeo is easier because conference organizers rely on that for analytics, comments, etc... Pretty sure that wouldn't work for background audio but maybe for auto that could work?
y
I'm not pushing for one format or the other. But more that we expose all available formats
It might actually be simplest to launch out to YouTube to play in most cases. They might have an auto mode?
m
Not sure how easy it is to market "watching videos as you drive" 😄
So that's the droidcon video feed: https://www.droidcon.com/category/video/feed/
It links to droidcon hosted vimeo videos
c
I think on Auto you can't even play videos; Automotive will allow it in the nearby future if the car is parked, I feel audio is a better solution - at least for me as a driver 🙂; I think it needs to be mp4 yes
27 Views
Open in Slack
PreviousNext
Powered by