Map-based backstack crashes when popping. #63
Assignees
Comments
rjrjr
added a commit
that referenced
this issue
Aug 26, 2022
`BackstackTransitionsTest` now uses a map, to expose problems hidden when key and content are the same thing. As a result we crash when popping. Reproduces #63.
rjrjr
added a commit
that referenced
this issue
Aug 26, 2022
`BackstackTransitionsTest` now uses a map, to expose problems hidden when key and content are the same thing. As a result we crash when popping. Reproduces #63.
rjrjr
added a commit
that referenced
this issue
Aug 26, 2022
`BackstackTransitionsTest` now uses a map, to expose problems hidden when key and content are the same thing. As a result we crash when popping. Reproduces #63.
rjrjr
added a commit
that referenced
this issue
Aug 26, 2022
`BackstackFrame` now holds on to both a `key` and a `@Composable` derived from the `content` function it was originally seen with. Previously during a pop we would try to display the outgoing `key` by passing it to the `content(T)` function passed with the updated `backstack` list. Fixes #63
|
No matter how I try to put Looking at the stack trace, no matter how I package things up, the composable lambda passed to the original I think I can make it work by changing the API to encourage the back stack elements to be self sufficient -- make them implement an interface similar to Workflow's I'm kind of sad about that because it'll make the API feel more OO and less Composesque, but I'm getting to the point where I don't believe it can be made to work any other way. Basically: fun interface WithContent {
@Composable fun Content()
}
fun <T : WithContent> Backstack(
backstack: List<T>,
modifier: Modifier = Modifier,
frameController: FrameController<T>
) |
rjrjr
added a commit
that referenced
this issue
Aug 27, 2022
All of our tests and demos were built using `String` as the key, with `content` that does nothing but render the key. This approach doesn't reflect reality very well, and masked #63, where keys for more interesting objects can get out of sync with the `content` lambda that can render them. When popping, you would wind up crashing when the up to date lambda is unable to interpret the key for the screen that is being animated away. The fix is to change the API from something that takes a list of keys and a function that can render them, to a list of model objects that themselves are able to provide `@Composable Content()`. IMHO the updated API actually feels pretty good, more like the conventional hoisted-state `@Composable Foo(model: FooModel)` idiom. (Of course I've been working on this all day, so I'm biased.) We provide a new interface: ```kotlin interface BackstackFrame<out K : Any> { val key: K @composable fun Content() } ``` And change the signature of the `Backstack()` function: ```kotlin fun <K : Any> Backstack( frames: List<BackstackFrame<K>>, modifier: Modifier = Modifier, frameController: FrameController<K> ) ``` Note that the param type, `K`, is still the type of the key, not the type of a particular flavor of `BackstackFrame`. This makes it easy for us to provide convenience functions to map lists of arbitrary model objects to `BackstackFrame` instances, so it's not much more verbose than it used to be to make it go. Before: ```kotlin Backstack(backstack) { screen -> when(screen) { Screen.ContactList -> ShowContactList(navigator) is Screen.ContactDetails -> ShowContact(screen.id, navigator) is Screen.EditContact -> ShowEditContact(screen.id, navigator) } } ``` After: ```kotlin Backstack( backstack.toBackstackModel { screen -> when(screen) { Screen.ContactList -> ShowContactList(navigator) is Screen.ContactDetails -> ShowContact(screen.id, navigator) is Screen.EditContact -> ShowEditContact(screen.id, navigator) } ) ``` Note that there are two flavors of `toBackstackModel`. The second one supports models with more interesting keys. ```kotlin data class Portrait( val id: Int, val url: String ) Backstack( backstack.toBackstackModel( getKey = { it.id } ) { PrettyPicture(it) } ) ``` Fixes #63
rjrjr
added a commit
that referenced
this issue
Aug 27, 2022
All of our tests and demos were built using `String` as the key, with `content` that does nothing but render the key. This approach doesn't reflect reality very well, and masked #63, where keys for more interesting objects can get out of sync with the `content` lambda that can render them. When popping, you would wind up crashing when the up to date lambda is unable to interpret the key for the screen that is being animated away. The fix is to change the API from something that takes a list of keys and a function that can render them, to a list of model objects that themselves are able to provide `@Composable Content()`. IMHO the updated API actually feels pretty good, more like the conventional hoisted-state `@Composable Foo(model: FooModel)` idiom. (Of course I've been working on this all day, so I'm biased.) We provide a new interface: ```kotlin interface BackstackFrame<out K : Any> { val key: K @composable fun Content() } ``` And change the signature of the `Backstack()` function: ```kotlin fun <K : Any> Backstack( frames: List<BackstackFrame<K>>, modifier: Modifier = Modifier, frameController: FrameController<K> ) ``` Note that the param type, `K`, is still the type of the key, not the type of a particular flavor of `BackstackFrame`. This makes it easy for us to provide convenience functions to map lists of arbitrary model objects to `BackstackFrame` instances, so it's not much more verbose than it used to be to make it go. Before: ```kotlin Backstack(backstack) { screen -> when(screen) { Screen.ContactList -> ShowContactList(navigator) is Screen.ContactDetails -> ShowContact(screen.id, navigator) is Screen.EditContact -> ShowEditContact(screen.id, navigator) } } ``` After: ```kotlin Backstack( backstack.toBackstackModel { screen -> when(screen) { Screen.ContactList -> ShowContactList(navigator) is Screen.ContactDetails -> ShowContact(screen.id, navigator) is Screen.EditContact -> ShowEditContact(screen.id, navigator) } ) ``` Note that there are two flavors of `toBackstackModel`. The second one supports models with more interesting keys. ```kotlin data class Portrait( val id: Int, val url: String ) Backstack( backstack.toBackstackModel( getKey = { it.id } ) { PrettyPicture(it) } ) ``` Fixes #63
rjrjr
added a commit
that referenced
this issue
Aug 27, 2022
All of our tests and demos were built using `String` as the key, with `content` that does nothing but render the key. This approach doesn't reflect reality very well, and masked #63, where keys for more interesting objects can get out of sync with the `content` lambda that can render them. When popping, you would wind up crashing when the up to date lambda is unable to interpret the key for the screen that is being animated away. The fix is to change the API from something that takes a list of keys and a function that can render them, to a list of model objects that themselves are able to provide `@Composable Content()`. IMHO the updated API actually feels pretty good, more like the conventional hoisted-state `@Composable Foo(model: FooModel)` idiom. (Of course I've been working on this all day, so I'm biased.) We provide a new interface: ```kotlin interface BackstackFrame<out K : Any> { val key: K @composable fun Content() } ``` And change the signature of the `Backstack()` function: ```kotlin fun <K : Any> Backstack( frames: List<BackstackFrame<K>>, modifier: Modifier = Modifier, frameController: FrameController<K> ) ``` Note that the param type, `K`, is still the type of the key, not the type of a particular flavor of `BackstackFrame`. This makes it easy for us to provide convenience functions to map lists of arbitrary model objects to `BackstackFrame` instances, so it's not much more verbose than it used to be to make it go. Before: ```kotlin Backstack(backstack) { screen -> when(screen) { Screen.ContactList -> ShowContactList(navigator) is Screen.ContactDetails -> ShowContact(screen.id, navigator) is Screen.EditContact -> ShowEditContact(screen.id, navigator) } } ``` After: ```kotlin Backstack( backstack.toBackstackModel { screen -> when(screen) { Screen.ContactList -> ShowContactList(navigator) is Screen.ContactDetails -> ShowContact(screen.id, navigator) is Screen.EditContact -> ShowEditContact(screen.id, navigator) } ) ``` Note that there are two flavors of `toBackstackModel`. The second one supports models with more interesting keys. ```kotlin data class Portrait( val id: Int, val url: String ) Backstack( backstack.toBackstackModel( getKey = { it.id } ) { PrettyPicture(it) } ) ``` Fixes #63
rjrjr
added a commit
that referenced
this issue
Aug 27, 2022
All of our tests and demos were built using `String` as the key, with `content` that does nothing but render the key. This approach doesn't reflect reality very well, and masked #63, where keys for more interesting objects can get out of sync with the `content` lambda that can render them. When popping, you would wind up crashing when the up to date lambda is unable to interpret the key for the screen that is being animated away. The fix is to change the API from something that takes a list of keys and a function that can render them, to a list of model objects that themselves are able to provide `@Composable Content()`. IMHO the updated API actually feels pretty good, more like the conventional hoisted-state `@Composable Foo(model: FooModel)` idiom. (Of course I've been working on this all day, so I'm biased.) We provide a new interface: ```kotlin interface BackstackFrame<out K : Any> { val key: K @composable fun Content() } ``` And change the signature of the `Backstack()` function: ```kotlin fun <K : Any> Backstack( frames: List<BackstackFrame<K>>, modifier: Modifier = Modifier, frameController: FrameController<K> ) ``` Note that the param type, `K`, is still the type of the key, not the type of a particular flavor of `BackstackFrame`. This makes it easy for us to provide convenience functions to map lists of arbitrary model objects to `BackstackFrame` instances, so it's not much more verbose than it used to be to make it go. Before: ```kotlin Backstack(backstack) { screen -> when(screen) { Screen.ContactList -> ShowContactList(navigator) is Screen.ContactDetails -> ShowContact(screen.id, navigator) is Screen.EditContact -> ShowEditContact(screen.id, navigator) } } ``` After: ```kotlin Backstack( backstack.toBackstackModel { screen -> when(screen) { Screen.ContactList -> ShowContactList(navigator) is Screen.ContactDetails -> ShowContact(screen.id, navigator) is Screen.EditContact -> ShowEditContact(screen.id, navigator) } ) ``` Note that there are two flavors of `toBackstackModel`. The second one supports models with more interesting keys. ```kotlin data class Portrait( val id: Int, val url: String ) Backstack( backstack.toBackstackModel( getKey = { it.id } ) { PrettyPicture(it) } ) ``` Fixes #63
rjrjr
added a commit
that referenced
this issue
Aug 27, 2022
All of our tests and demos were built using `String` as the key, with `content` that does nothing but render the key. This approach doesn't reflect reality very well, and masked #63, where keys for more interesting objects can get out of sync with the `content` lambda that can render them. When popping, you would wind up crashing when the up to date lambda is unable to interpret the key for the screen that is being animated away. The fix is to change the API from something that takes a list of keys and a function that can render them, to a list of model objects that themselves are able to provide `@Composable Content()`. IMHO the updated API actually feels pretty good, more like the conventional hoisted-state `@Composable Foo(model: FooModel)` idiom. (Of course I've been working on this all day, so I'm biased.) We provide a new interface: ```kotlin interface BackstackFrame<out K : Any> { val key: K @composable fun Content() } ``` And change the signature of the `Backstack()` function: ```kotlin fun <K : Any> Backstack( frames: List<BackstackFrame<K>>, modifier: Modifier = Modifier, frameController: FrameController<K> ) ``` Note that the param type, `K`, is still the type of the key, not the type of a particular flavor of `BackstackFrame`. This makes it easy for us to provide convenience functions to map lists of arbitrary model objects to `BackstackFrame` instances, so it's not much more verbose than it used to be to make it go. Before: ```kotlin Backstack(backstack) { screen -> when(screen) { Screen.ContactList -> ShowContactList(navigator) is Screen.ContactDetails -> ShowContact(screen.id, navigator) is Screen.EditContact -> ShowEditContact(screen.id, navigator) } } ``` After: ```kotlin Backstack( backstack.toBackstackModel { screen -> when(screen) { Screen.ContactList -> ShowContactList(navigator) is Screen.ContactDetails -> ShowContact(screen.id, navigator) is Screen.EditContact -> ShowEditContact(screen.id, navigator) } ) ``` Note that there are two flavors of `toBackstackModel`. The second one supports models with more interesting keys. ```kotlin data class Portrait( val id: Int, val url: String ) Backstack( backstack.toBackstackModel( getKey = { it.id } ) { PrettyPicture(it.url) } ) ``` Fixes #63
rjrjr
added a commit
that referenced
this issue
Aug 27, 2022
All of our tests and demos were built using `String` as the key, with `content` that does nothing but render the key. This approach doesn't reflect reality very well, and masked #63, where keys for more interesting objects can get out of sync with the `content` lambda that can render them. When popping, you would wind up crashing when the up to date lambda is unable to interpret the key for the screen that is being animated away. The fix is to change the API from something that takes a list of keys and a function that can render them, to a list of model objects that themselves are able to provide `@Composable Content()`. IMHO the updated API actually feels pretty good, more like the conventional hoisted-state `@Composable Foo(model: FooModel)` idiom. (Of course I've been working on this all day, so I'm biased.) We provide a new interface: ```kotlin interface BackstackFrame<out K : Any> { val key: K @composable fun Content() } ``` And change the signature of the `Backstack()` function: ```kotlin fun <K : Any> Backstack( frames: List<BackstackFrame<K>>, modifier: Modifier = Modifier, frameController: FrameController<K> ) ``` Note that the param type, `K`, is still the type of the key, not the type of a particular flavor of `BackstackFrame`. This makes it easy for us to provide convenience functions to map lists of arbitrary model objects to `BackstackFrame` instances, so it's not much more verbose than it used to be to make it go. Before: ```kotlin Backstack(backstack) { screen -> when(screen) { Screen.ContactList -> ShowContactList(navigator) is Screen.ContactDetails -> ShowContact(screen.id, navigator) is Screen.EditContact -> ShowEditContact(screen.id, navigator) } } ``` After: ```kotlin Backstack( backstack.toBackstackModel { screen -> when(screen) { Screen.ContactList -> ShowContactList(navigator) is Screen.ContactDetails -> ShowContact(screen.id, navigator) is Screen.EditContact -> ShowEditContact(screen.id, navigator) } ) ``` Note that there are two flavors of `toBackstackModel`. The second one supports models with more interesting keys. ```kotlin data class Portrait( val id: Int, val url: String ) Backstack( backstack.toBackstackModel( getKey = { it.id } ) { PrettyPicture(it.url) } ) ``` Fixes #63
rjrjr
added a commit
that referenced
this issue
Aug 27, 2022
All of our tests and demos were built using `String` as the key, with `content` that does nothing but render the key. This approach doesn't reflect reality very well, and masked #63, where keys for more interesting objects can get out of sync with the `content` lambda that can render them. When popping, you would wind up crashing when the up to date lambda is unable to interpret the key for the screen that is being animated away. The fix is to change the API from something that takes a list of keys and a function that can render them, to a list of model objects that themselves are able to provide `@Composable Content()`. IMHO the updated API actually feels pretty good, more like the conventional hoisted-state `@Composable Foo(model: FooModel)` idiom. (Of course I've been working on this all day, so I'm biased.) We provide a new interface: ```kotlin interface BackstackFrame<out K : Any> { val key: K @composable fun Content() } ``` And change the signature of the `Backstack()` function: ```kotlin fun <K : Any> Backstack( frames: List<BackstackFrame<K>>, modifier: Modifier = Modifier, frameController: FrameController<K> ) ``` Note that the param type, `K`, is still the type of the key, not the type of a particular flavor of `BackstackFrame`. This makes it easy for us to provide convenience functions to map lists of arbitrary model objects to `BackstackFrame` instances, so it's not much more verbose than it used to be to make it go. Before: ```kotlin Backstack(backstack) { screen -> when(screen) { Screen.ContactList -> ShowContactList(navigator) is Screen.ContactDetails -> ShowContact(screen.id, navigator) is Screen.EditContact -> ShowEditContact(screen.id, navigator) } } ``` After: ```kotlin Backstack( backstack.toBackstackModel { screen -> when(screen) { Screen.ContactList -> ShowContactList(navigator) is Screen.ContactDetails -> ShowContact(screen.id, navigator) is Screen.EditContact -> ShowEditContact(screen.id, navigator) } } ) ``` Note that there are two flavors of `toBackstackModel`. The second one supports models with more interesting keys. ```kotlin data class Portrait( val id: Int, val url: String ) Backstack( backstack.toBackstackModel( getKey = { it.id } ) { PrettyPicture(it.url) } ) ``` Fixes #63
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
If you modify
BackstackTransitionsTest.assertTransitionto build its backstacks out ofMap<Int, String>, like so, you crash withNoSuchElementExceptionwhen popping. We're popping to a list that no longer includes the information to paint the outgoing screen, which is a pretty realistic situation.The text was updated successfully, but these errors were encountered: