-
Couldn't load subscription status.
- Fork 103
Adapt changes in NavigationEventHandler
#2436
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 8 commits
ea50af7
42ae29a
4ff87d9
001bd30
52b1477
8180899
75288bb
5386552
1a9eeaf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,10 +19,25 @@ | |
| package androidx.compose.ui.backhandler | ||
|
|
||
| import androidx.compose.runtime.Composable | ||
| import androidx.compose.runtime.DisposableEffect | ||
| import androidx.compose.runtime.LaunchedEffect | ||
| import androidx.compose.runtime.getValue | ||
| import androidx.compose.runtime.mutableStateOf | ||
| import androidx.compose.runtime.remember | ||
| import androidx.compose.runtime.rememberCoroutineScope | ||
| import androidx.compose.runtime.setValue | ||
| import androidx.compose.ui.ExperimentalComposeUiApi | ||
| import androidx.navigationevent.NavigationEvent | ||
| import androidx.navigationevent.NavigationEventInfo | ||
| import androidx.navigationevent.NavigationEventTransitionState | ||
| import androidx.navigationevent.compose.LocalNavigationEventDispatcherOwner | ||
| import androidx.navigationevent.compose.NavigationBackHandler | ||
| import androidx.navigationevent.compose.rememberNavigationEventState | ||
| import kotlinx.coroutines.CancellationException | ||
| import kotlinx.coroutines.channels.Channel | ||
| import kotlinx.coroutines.flow.Flow | ||
| import kotlinx.coroutines.flow.consumeAsFlow | ||
| import kotlinx.coroutines.launch | ||
|
|
||
| @Deprecated("Use NavigationEventHandler instead") | ||
| @ExperimentalComposeUiApi | ||
|
|
@@ -31,33 +46,69 @@ actual fun PredictiveBackHandler( | |
| enabled: Boolean, | ||
| onBack: suspend (progress: Flow<BackEventCompat>) -> Unit | ||
| ) { | ||
| LocalNavigationEventDispatcherOwner.current ?: return | ||
| /* | ||
| TODO: https://youtrack.jetbrains.com/issue/CMP-8937 | ||
| NavigationEventHandler(enabled) { progress -> | ||
| val compatProgress = progress.map { navEvent -> | ||
| val coroutineScope = rememberCoroutineScope() | ||
| val navEventState = rememberNavigationEventState(NavigationEventInfo.None) | ||
|
|
||
| var progressChannel: Channel<BackEventCompat>? by remember(onBack) { | ||
| mutableStateOf(null) | ||
| } | ||
|
|
||
| fun getActiveProgressChannel(): Channel<BackEventCompat> { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As a TIL question. What is the benefit of using Channel instead of Flow these days? Is it for performance? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is just a lighter low-level abstraction. |
||
| val currentProgressChannel = progressChannel | ||
| if (currentProgressChannel == null) { | ||
| val progress = Channel<BackEventCompat>() | ||
| progressChannel = progress | ||
| coroutineScope.launch { | ||
| onBack(progress.consumeAsFlow()) | ||
| } | ||
| return progress | ||
| } else { | ||
| return currentProgressChannel | ||
| } | ||
| } | ||
|
|
||
| val transitionState = navEventState.transitionState | ||
| if (transitionState is NavigationEventTransitionState.InProgress) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does it get to run when the transition state turns from Idle to InProgress again after a composition? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, nvm, it does. |
||
| LaunchedEffect(transitionState) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does the coroutine relaunch after every transitionState change? For example, if we see: Does it launch 5 times? |
||
| val navEvent = transitionState.latestEvent | ||
| val swipeEdge = when (navEvent.swipeEdge) { | ||
| NavigationEventSwipeEdge.Left -> BackEventCompat.EDGE_LEFT | ||
| NavigationEventSwipeEdge.Right -> BackEventCompat.EDGE_RIGHT | ||
| else -> 0 | ||
| NavigationEvent.EDGE_RIGHT -> BackEventCompat.EDGE_RIGHT | ||
| else -> BackEventCompat.EDGE_LEFT | ||
| } | ||
| BackEventCompat(navEvent.touchX, navEvent.touchY, navEvent.progress, swipeEdge) | ||
| val event = BackEventCompat( | ||
| navEvent.touchX, navEvent.touchY, navEvent.progress, swipeEdge | ||
| ) | ||
| getActiveProgressChannel().send(event) | ||
| } | ||
| } | ||
|
|
||
| NavigationBackHandler( | ||
| state = navEventState, | ||
| isBackEnabled = enabled, | ||
| onBackCancelled = { | ||
| getActiveProgressChannel().close(CancellationException("Cancelled")) | ||
| progressChannel = null | ||
| }, | ||
| onBackCompleted = { | ||
| getActiveProgressChannel().close() | ||
| progressChannel = null | ||
| } | ||
| ) | ||
| DisposableEffect(Unit) { | ||
| onDispose { | ||
| progressChannel?.close(CancellationException("Disposed")) | ||
| progressChannel = null | ||
| } | ||
| onBack(compatProgress) | ||
| } | ||
| */ | ||
| } | ||
|
|
||
| @Deprecated("Use NavigationEventHandler instead") | ||
| @ExperimentalComposeUiApi | ||
| @Composable | ||
| actual fun BackHandler(enabled: Boolean, onBack: () -> Unit) { | ||
| PredictiveBackHandler(enabled) { progress -> | ||
| try { | ||
| progress.collect { /*ignore*/ } | ||
| onBack() | ||
| } catch (e: CancellationException) { | ||
| //ignore | ||
| } | ||
| } | ||
| NavigationBackHandler( | ||
| state = rememberNavigationEventState(NavigationEventInfo.None), | ||
| isBackEnabled = enabled, | ||
| onBackCompleted = onBack | ||
| ) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,6 +44,9 @@ import androidx.lifecycle.Lifecycle | |
| import androidx.lifecycle.LifecycleOwner | ||
| import androidx.lifecycle.LifecycleRegistry | ||
| import androidx.lifecycle.compose.LocalLifecycleOwner | ||
| import androidx.navigationevent.NavigationEventDispatcher | ||
| import androidx.navigationevent.NavigationEventDispatcherOwner | ||
| import androidx.navigationevent.compose.LocalNavigationEventDispatcherOwner | ||
| import kotlin.coroutines.CoroutineContext | ||
| import kotlin.coroutines.EmptyCoroutineContext | ||
| import kotlin.coroutines.cancellation.CancellationException | ||
|
|
@@ -456,7 +459,10 @@ open class SkikoComposeUiTest @InternalTestApi constructor( | |
| } | ||
|
|
||
| @OptIn(InternalComposeUiApi::class) | ||
| internal inner class SkikoTestOwner : TestOwner, LifecycleOwner { | ||
| internal inner class SkikoTestOwner : | ||
| TestOwner, | ||
| LifecycleOwner, | ||
| NavigationEventDispatcherOwner { | ||
| override val mainClock | ||
| get() = mainClockImpl | ||
|
|
||
|
|
@@ -474,6 +480,7 @@ open class SkikoComposeUiTest @InternalTestApi constructor( | |
| } | ||
|
|
||
| override val lifecycle = LifecycleRegistry.createUnsafe(this) | ||
| override val navigationEventDispatcher = NavigationEventDispatcher() | ||
|
|
||
| fun captureToImage(semanticsNode: SemanticsNode): ImageBitmap = | ||
| [email protected](semanticsNode) | ||
|
|
@@ -535,6 +542,7 @@ open class SkikoComposeUiTest @InternalTestApi constructor( | |
| private fun ProvideCommonCompositionLocals(content: @Composable () -> Unit) { | ||
| CompositionLocalProvider( | ||
| LocalLifecycleOwner provides testOwner, | ||
| LocalNavigationEventDispatcherOwner provides testOwner, | ||
| content = content, | ||
| ) | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,7 +30,6 @@ import androidx.compose.ui.unit.asDpRect | |
| import androidx.compose.ui.unit.toOffset | ||
| import androidx.compose.ui.unit.width | ||
| import androidx.navigationevent.NavigationEvent | ||
| import androidx.navigationevent.NavigationEventDispatcher | ||
| import androidx.navigationevent.NavigationEventInput | ||
| import kotlin.math.abs | ||
| import kotlinx.cinterop.BetaInteropApi | ||
|
|
@@ -57,13 +56,6 @@ internal class UIKitNavigationEventInput( | |
| private val density: Density, | ||
| private val getTopLeftOffsetInWindow: () -> IntOffset | ||
| ) : NavigationEventInput() { | ||
| val isBackGestureActive: Boolean | ||
| get() = false | ||
| fun onDidMoveToWindow(window: UIWindow?, composeRootView: UIView) {} | ||
| fun onKeyEvent(event: KeyEvent): Boolean = false | ||
| /* | ||
| TODO: https://youtrack.jetbrains.com/issue/CMP-8937 | ||
|
|
||
| companion object { | ||
| private const val BACK_GESTURE_SCREEN_SIZE = 0.3 | ||
| private const val BACK_GESTURE_VELOCITY = 100 | ||
|
|
@@ -85,24 +77,9 @@ TODO: https://youtrack.jetbrains.com/issue/CMP-8937 | |
| edges = UIRectEdgeRight | ||
| } | ||
|
|
||
| override fun onAdded(dispatcher: NavigationEventDispatcher) { | ||
| super.onAdded(dispatcher) | ||
| updateRecognizersEnabledState(dispatcher.hasEnabledCallbacks()) | ||
| } | ||
|
|
||
| override fun onHasEnabledCallbacksChanged(hasEnabledCallbacks: Boolean) { | ||
| super.onHasEnabledCallbacksChanged(hasEnabledCallbacks) | ||
| updateRecognizersEnabledState(hasEnabledCallbacks) | ||
| } | ||
|
|
||
| override fun onRemoved() { | ||
| super.onRemoved() | ||
| updateRecognizersEnabledState(false) | ||
| } | ||
|
|
||
| private fun updateRecognizersEnabledState(isEnabled: Boolean) { | ||
| leftEdgePanGestureRecognizer.enabled = isEnabled | ||
| rightEdgePanGestureRecognizer.enabled = isEnabled | ||
| override fun onHasEnabledHandlersChanged(hasEnabledHandlers: Boolean) { | ||
| leftEdgePanGestureRecognizer.enabled = hasEnabledHandlers | ||
| rightEdgePanGestureRecognizer.enabled = hasEnabledHandlers | ||
| } | ||
|
|
||
| private val activeGestureStates = listOf( | ||
|
|
@@ -140,7 +117,7 @@ TODO: https://youtrack.jetbrains.com/issue/CMP-8937 | |
|
|
||
| fun onKeyEvent(event: KeyEvent): Boolean { | ||
| if (event.type == KeyEventType.KeyDown && event.key == Key.Escape) { | ||
| dispatchOnCompleted() | ||
| dispatchOnBackCompleted() | ||
| return true | ||
| } else { | ||
| return false | ||
|
|
@@ -157,7 +134,7 @@ TODO: https://youtrack.jetbrains.com/issue/CMP-8937 | |
| val touch = recognizer.locationOfTouch(0u, view).asDpOffset() | ||
| val eventOffset = | ||
| touch.toOffset(density) - getTopLeftOffsetInWindow().toOffset() | ||
| dispatchOnStarted( | ||
| dispatchOnBackStarted( | ||
| NavigationEvent( | ||
| touchX = eventOffset.x, | ||
| touchY = eventOffset.y, | ||
|
|
@@ -183,7 +160,7 @@ TODO: https://youtrack.jetbrains.com/issue/CMP-8937 | |
| } else { | ||
| (bounds.width - touch.x) / bounds.width | ||
| } | ||
| dispatchOnProgressed( | ||
| dispatchOnBackProgressed( | ||
| NavigationEvent( | ||
| touchX = eventOffset.x, | ||
| touchY = eventOffset.y, | ||
|
|
@@ -210,30 +187,29 @@ TODO: https://youtrack.jetbrains.com/issue/CMP-8937 | |
| if (edge == UIRectEdgeLeft) [email protected] else [email protected] | ||
| when { | ||
| //if movement is fast in the right direction | ||
| velX > BACK_GESTURE_VELOCITY -> dispatchOnCompleted() | ||
| velX > BACK_GESTURE_VELOCITY -> dispatchOnBackCompleted() | ||
| //if movement is backward | ||
| velX < -10 -> dispatchOnCancelled() | ||
| velX < -10 -> dispatchOnBackCancelled() | ||
| //if there is no movement, or the movement is slow, | ||
| //but the touch is already more than BACK_GESTURE_SCREEN_SIZE | ||
| abs(x) >= size.width * BACK_GESTURE_SCREEN_SIZE -> dispatchOnCompleted() | ||
| else -> dispatchOnCancelled() | ||
| abs(x) >= size.width * BACK_GESTURE_SCREEN_SIZE -> dispatchOnBackCompleted() | ||
| else -> dispatchOnBackCancelled() | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| UIGestureRecognizerStateCancelled -> { | ||
| dispatchOnCancelled() | ||
| dispatchOnBackCancelled() | ||
| } | ||
|
|
||
| UIGestureRecognizerStateFailed -> { | ||
| dispatchOnCompleted() | ||
| dispatchOnBackCompleted() | ||
| } | ||
| } | ||
| } | ||
| } | ||
| */ | ||
| } | ||
|
|
||
| /** | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that onBack is still called after cancellation. Not sure if it's our fault. For example:
And after it prints "cancelled", it then prints another "start" and back event.