Completes our navigation story #1313
Conversation
The default implementation of `Unwrappable.unwrap` creates a `Sequence` that is almost never necessary (but it is at least fail-safe so I think we should keep it). Here we provide improved implementations for `Wrapper` and `BackStackScreen`.
rjrjr
force-pushed
the
ray/nav-polish
branch
2 times, most recently
from
a311082 to
d5f09d6
Compare
| skipFirstScreen: Boolean = false, | ||
| private val onNavigate: (Any) -> Unit = { println(it) } | ||
| ) { | ||
| private var lastKey: String? = if (skipFirstScreen) null else "" |
There was a problem hiding this comment.
@pyricau @steve-the-edwards We have only ever used our flavor of this on the main thread with the Main.Immediate dispatcher, so this simple implementation has been just fine. Should we stick with what we know, or should this field be marked @Volatile, or…?
There was a problem hiding this comment.
My gut is to leave it this way. It's such a trivial class that anyone doing something more interesting and finding flaws could so easily fork it.
Should I put something in the kdoc warning of its limitations?
There was a problem hiding this comment.
Enforce that it's single threaded.
There was a problem hiding this comment.
Or make it volatile it's simple enough and not that expensive.
There was a problem hiding this comment.
I think that's the sanity check I was hoping for, "@Volatile is just fine."
Takes advantage of `unwrap()` and `Compatible.keyFor()` to provide navigation logging by reporting the top (read: last-most inner-most) sub-rendering, which conventionally is the one that is visible and accessible to the user. Reports each time the `Compatible.keyFor` the top is unequal to the previous one, which conventionally indicates that a new view object will replace the previous one.
workflow-ui/core-common/src/main/java/com/squareup/workflow1/ui/navigation/NavigationMonitor.kt
Show resolved
Hide resolved
Optimize central
unwrapimplementations.The default implementation of
Unwrappable.unwrapcreates aSequencethat is almost never necessary (but it is at least fail-safe so I think we should keep it). Here we provide improved implementations forWrapperandBackStackScreen.Introduces NavigationMonitor
Takes advantage of
unwrap()andCompatible.keyFor()to provide navigation logging by reporting the top (read: last-most inner-most) sub-rendering, which conventionally is the one that is visible and accessible to the user. Reports each time theCompatible.keyForthe top is unequal to the previous one, which conventionally indicates that a new view object will replace the previous one.