https://kotlinlang.org logo
#compose
Title
# compose
b

brabo-hi

01/17/2022, 9:15 PM
Hi all, when using navigation in compose what is the difference and best practice when passing data as argument, between
Copy code
navController.currentBackStackEntry?.savedStateHandle?.set("person", person)
and
Copy code
navController.currentBackStackEntry?.arguments?.putParcelable("person", person)
i

Ian Lake

01/17/2022, 9:16 PM
Neither is at all correct in any way at all ever, but the later is much worse than the former
You should not be passing a
Person
as an argument at all as per the documentation:
To pass around custom complex data, store the data elsewhere such as a ViewModel or database and only pass an identifier while navigating; then retrieve the data in the new location after navigation has concluded.
As also mentioned on that page:
Caution: Passing complex data structures over arguments is considered an anti-pattern. Each destination should be responsible for loading UI data based on the minimum necessary information, such as item IDs. This simplifies process recreation and avoids potential data inconsistencies.
Something we talked about in detail last year:

https://www.youtube.com/watch?v=0z_dwBGQQWQ&t=930s

A
Person
class certainly seems like something that you shouldn't be passing between destinations, but something that a common
PersonRepository
that both destinations use as their source of truth would make a lot more sense
Navigation does support custom types as arguments, but those are for sending composition specific state between screens and not for sending domain objects
👍 1
c

Colton Idle

01/17/2022, 9:37 PM
Navigation does support custom types as arguments, but those are for sending composition specific state between screens and not for sending domain objects (edited)
ooh. what composition specific state would that be? Would this lead to shared element transitions by chance?
i

Ian Lake

01/17/2022, 9:40 PM
No, I'm talking about state that only exists at the UI layer such as the
SearchParameters
class used as an example in those exact docs
Creating a
SearchParametersRepository
wouldn't make any sense (and has never been a recommendation) as that state doesn't exist outside of the context it is created and consumed in
Arguments are part of the identity of a destination -
plants
vs
plant/{id}
point to two different screens. Shared element information isn't part of a screen's identity (just like how animations aren't part of the screen's identity) so I wouldn't expect arguments and shared elements to have any overlap
b

brabo-hi

01/17/2022, 9:59 PM
thanks @Ian Lake for sharing these details, makes more sens now
z

Zoltan Demant

01/21/2022, 2:48 PM
@Michal Klimczak I would argue that its probably still a good idea to query for a product in both destinations, only passing the required identifier around to make that query possible (similar to what Ian mentioned above). Back in the day I used to pass around my domain models everywhere, and it worked great; but passing around identifiers and querying for the models is SO much better & easier to deal with. Having one source of truth means that all screens by default show the latest version of a given product. Furthermore, since the same repository is used in screens A & B; you can create a smart cache that makes sure a given product is only queried once, so no loss there either (and it can clear the cache entry when there are no more active queries). And saving state is now as simple as saving the identifier. Just my humble opinion 🙏🏽 For a cache solution, Id suggest looking into how they handle it in caffeine.
🙏 1
m

Michal Klimczak

01/21/2022, 3:02 PM
In Ian's example he uses Room which is convenient to prove the point. However lots of apps are only thin clients of a backend and don't require sophisticated persistence. I'm all for single source of truth, the scoping feels wrong, though - my point is that introducing cache just to pass data between two screens which are closely related is clearly an overkill, it's an example of local state (albeit shared), not global state.
i

Ian Lake

01/21/2022, 10:02 PM
"screens which show modals, dialogs, bottomsheets" don't need to implement modals, dialogs, and bottom sheets as completely separate destination. In fact, it is very rare that any of those should be independent destinations at all, but instead should just be an implementation detail on how the functionality of a single destination is implemented on a specific form factor (you are adjusting your implementations based on whether you are on a phone, tablet, etc. right? 🙂 ). You wouldn't be passing any information at all then
b

brabo-hi

01/21/2022, 10:06 PM
i

Ian Lake

01/21/2022, 10:08 PM
Just like how all
Dialog
usages across Compose don't need to be a dialog destination, not all bottom sheets need to be a bottom sheet destination
And yet, sometimes you do have a
Dialog
that needs its own independent set of viewmodels, state, etc. that is separate from whoever called it. And sometimes you need a bottom sheet that is completely independent from whoever called it. If that isn't the case, having it as a separate destination isn't the right choice in the first place
c

Colton Idle

01/21/2022, 10:13 PM
should just be an implementation detail on how the functionality of a single destination is implemented on a specific form factor
Couldn't you also do that even when using a modal as a destination? I'm not arguing against your statement. What I've found is that its way easier to do stuff in a single screen that contains all of the modals it needs. But was just curioius if there was something inherently stopping me from treating certain things (like a button press) different on a "phone" vs "tablet" (i.e. nav event to a modal vs just showing some extra info in an additional pane on the tablet)
i

Ian Lake

01/21/2022, 10:15 PM
I think we call that an 'if statement', using something like WindowSizeClass as the deciding factor: https://developer.android.com/jetpack/compose/layouts/adaptive#explicit-layout-changes
c

Colton Idle

01/21/2022, 10:27 PM
Yeah. I was just trying to ask if that's "wrong" because it sounded like from what you said (and maybe i just misinterpreted, if so sorry) that you wouldn't do that because the modal shouldn't be a destination at all. I guess I'm just working on bottom sheet picker components (i.e. date and time pickers) and so I'm really conflicted if I should make them modal destinations or just include them in each screen.
i

Ian Lake

01/21/2022, 10:36 PM
Just like how for a dialog destination we specifically say:
This is suitable only when this dialog represents a separate screen in your app that needs its own lifecycle and saved state, independent of any other destination in your navigation graph. For use cases such as
AlertDialog
, you should use those APIs directly in the
composable
destination that wants to show that dialog.
I would imagine that date and time pickers should also not be a separate destination entirely, although I realize that the bottom sheet APIs in Compose are quite a bit more...complicated to use than
Dialog
🙂
m

Michal Klimczak

01/22/2022, 6:52 AM
This is indeed something that we did outside of navigation graph for some time, but then switched to navigation because some of these bottom sheets needed to show on screens which also showed bottom navigation so they needed to be on top of it z-index wise. Maybe that could have been solved differently... And for destinations which are full screen modals I kind of never thought that doing them outside of navigation would be OK, but maybe it's just a limitation that I imposed on myself unconsciously. Just not sure about injection of their viewmodel in that scenario, but will check that.
c

Colton Idle

01/22/2022, 7:15 AM
Ian Lake, thanks for the convo. You definitely changed my perspective. I think our biggest push for moving some of those things to be "global" destinations was the fact that they can/should happen from anywhere (our time picker for example can be started from any screen) and so we didn't want to repeat that logic on every screen + the fact that we wanted the back button functionality out of the box/have an entry on the backstack. We'll reevaluate, but I have a feeling that the answer will be "it depends. lets commit to do x and then reevaluate if it doesn't scale well"
☝️ 1