-
Notifications
You must be signed in to change notification settings - Fork 81
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
Support multiplatform BackHandler
/PredictiveBackHandler
.
#1771
base: jb-main
Are you sure you want to change the base?
Conversation
...ial3/material3/src/uikitMain/kotlin/androidx/compose/material3/internal/BackHandler.uikit.kt
Outdated
Show resolved
Hide resolved
compose/mpp/demo/src/uikitMain/kotlin/androidx/compose/mpp/demo/UIKitPredictiveBackExample.kt
Outdated
Show resolved
Hide resolved
compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/backhandler/UIKitBackEvent.kt
Outdated
Show resolved
Hide resolved
...ose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/backhandler/UIKitPredictiveBackHandler.kt
Outdated
Show resolved
Hide resolved
.../src/commonMain/kotlin/androidx/navigation/compose/internal/PlatformPredictiveBackHandler.kt
Outdated
Show resolved
Hide resolved
...topMain/kotlin/androidx/navigation/compose/internal/PlatformPredictiveBackHandler.desktop.kt
Outdated
Show resolved
Hide resolved
...macosMain/kotlin/androidx/navigation/compose/internal/PlatformPredictiveBackHandler.macos.kt
Outdated
Show resolved
Hide resolved
...uikitMain/kotlin/androidx/navigation/compose/internal/PlatformPredictiveBackHandler.uikit.kt
Outdated
Show resolved
Hide resolved
...src/webMain/kotlin/androidx/navigation/compose/internal/PlatformPredictiveBackHandler.web.kt
Outdated
Show resolved
Hide resolved
e36fb22
to
8ad3ee6
Compare
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.
I don't think that adding it to skikoMain is the right move at the moment. We DO want to make common API for this, but not in compose-ui module
compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/backhandler/BackEventCompat.kt
Outdated
Show resolved
Hide resolved
54d51a3
to
99c6ebc
Compare
BackHandler
/PredictiveBackHandler
.
BackHandler
/PredictiveBackHandler
.BackHandler
/PredictiveBackHandler
.
...ose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/NavigationDrawer.kt
Outdated
Show resolved
Hide resolved
...ose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/NavigationDrawer.kt
Outdated
Show resolved
Hide resolved
compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/SearchBar.kt
Outdated
Show resolved
Hide resolved
...material3/material3/src/commonMain/kotlin/androidx/compose/material3/internal/BackHandler.kt
Outdated
Show resolved
Hide resolved
...ui-backhandler/src/androidMain/kotlin/androidx/compose/ui/backhandler/BackHandler.android.kt
Show resolved
Hide resolved
...e/ui/ui-backhandler/src/commonMain/kotlin/androidx/compose/ui/backhandler/BackEventCompat.kt
Outdated
Show resolved
Hide resolved
compose/ui/ui-backhandler/src/jbMain/kotlin/androidx/compose/ui/backhandler/BackHandler.jb.kt
Outdated
Show resolved
Hide resolved
} | ||
|
||
fun setView(rootView: UIView) { | ||
rootView.addGestureRecognizer(leftEdgePanGestureRecognizer) |
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.
Gesture recognizers must be removed from view when they not needed. Currently, I can see 2 events:
ComposeHostingViewController.dispose()
- when window is null in
onDidMoveToWindow(window: UIWindow?)
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.
We discussed it with @elijah-semyonov and decided it is not needed here.
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.
As far as I can see, it creates 2 gesture recognisers per each ComposeHostingViewController and add them to some external view - these recognisers will stay there forever. So I think we need to delete them anyway.
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.
will stay there forever
no. when ComposeHostingViewController is gone, it will be destroyed because a system view keeps a weak reference
velX < -10 -> listener.onCancelled() | ||
//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 -> listener.onCompleted() |
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.
On iOS it's a bit more complicated. It looks more like: min(200.dp, bounds.width / 2)
, or similar..
I think we need to play (in another MR) here a bit to make behavior close to what we have in UINavigationController
...ose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/backhandler/UIKitBackGestureDispatcher.kt
Outdated
Show resolved
Hide resolved
5653ec4
to
1cd1470
Compare
1cd1470
to
bcdb1ba
Compare
… them via a compose static local.
…led. It is not possible to run a suspend function when the progress flow is canceled because the onBack suspend lambda is canceled as well. See: androidx.activity.compose.OnBackInstance.cancel
bcdb1ba
to
15aa84a
Compare
Support
BackHandler
/PredictiveBackHandler
.Fixes https://youtrack.jetbrains.com/issue/CMP-4419
Screen.Recording.2025-01-10.at.21.19.40.mov
Testing
Release Notes
Highlights - Multiple Platforms
BackHandler
andPredictiveBackHandler
. And use them in material3 widgets and androidx-navigation library.