Zhuinden / simple-stack

[ACTIVE] Simple Stack, a backstack library / navigation framework for simpler navigation and state management (for fragments, views, or whatevers).
Apache License 2.0
1.37k stars 75 forks source link

TargetSdkVersion 34 replaces onBackPressed() with OnBackInvokedCallback #259

Closed Zhuinden closed 1 year ago

Zhuinden commented 2 years ago

With the "advent" of OnBackPressedDispatcher, we already had to do this:

class MainActivity : AppCompatActivity(), SimpleStateChanger.NavigationHandler {
    private val backPressedCallback = object: OnBackPressedCallback(true) {
        override fun handleOnBackPressed() {
            if (!Navigator.onBackPressed(this@MainActivity)) {
                this.remove() // this is the only safe way to invoke onBackPressed while cancelling the execution of this callback
                onBackPressed() // this is the only safe way to invoke onBackPressed while cancelling the execution of this callback
                this@MainActivity.onBackPressedDispatcher.addCallback(this) // this is the only safe way to invoke onBackPressed while cancelling the execution of this callback
            }
        }
    }

    override fun onCreate(savedInstanceState: Bundle?) {
        // ...
        onBackPressedDispatcher.addCallback(backPressedCallback) // this is required for `onBackPressedDispatcher` to work correctly
        // ...
    }

    @Suppress("RedundantModalityModifier")
    override final fun onBackPressed() { // you cannot use `onBackPressed()` if you use `OnBackPressedDispatcher`
        super.onBackPressed() // `OnBackPressedDispatcher` by Google effectively breaks all usages of `onBackPressed()` because they do not respect the original contract of `onBackPressed()`
    }
}

However, now, onBackPressed() is deprecated in Android Tiramisu, replaced with onBackPressedCallback and onBackInvokedCallback, which seem to have the same design problems as OnBackPressedDispatcher: namely, that once you registered to receive a callback and you have received said callback, you can't cancel the request.

Clearly, the people at Google writing this code have never heard of event bubbling, which has worked in most UI frameworks in the past 15+ years. (they did, they just want to animate BACK if it's not handled by the app, so this change actually makes sense...)

However, this means we might need to bite the bullet and replace the current event-bubbling approach of ScopedServices.HandlesBack with tinkering with exposing if a service on top is currently handling back or not.

This is quite a heavy change and will require Android T to come out.

matejdro commented 1 year ago

It seems like ScopedServices.HandlesBack has now been superseded by the OnBackPressedCallback (any service can just use Google's callback instead of simple stack's one). Maybe it's time to deprecate that functionality and suggest to use the official callback solution instead?

That gets rid of the scoped services issue. For the rest, extra boolean canGoBack() method could be added to the Navigator. Then OnBackInvokedCallback can be implemented by installing a CompletionListener and enabling/disabling callback based on the result of the canGoBack method.

Zhuinden commented 1 year ago

I have a sincere dislike for the "new" notifications UI in Github. Back in the day, I never had to bulk-process 5 pages of notifications before I get important ones...

Anyways, the official callback alone wouldn't work, as a scoped service lives within the non-config scope. To intercept back in a scoped service, Simple-Stack will need to intercept it and implement the same dispatch behavior in ScopeNodes by default so that if either ScopeNode has an active back handler that wants to handle back, it would get it in "VM layer".

I had been trying to think of a way to introduce a new "back mode" to opt-in into the ahead-of-time model, so that there is a migration path and transition process + the original Back processing made perfect sense until suddenly Google changed the way it works with targetSdkVersion 34.

But I admit, this is getting more and more important to handle. Considering we use Simple-Stack in our apps, it will be done eventually.

matejdro commented 1 year ago

Huh, is there a use case where a non-config-scope service would need to intercept back? It seems like these two shouldn't mix.

Zhuinden commented 1 year ago

I actually did it with only "breaking" ScopedServices.HandlesBack and it's opt-in.

Eheheheh