Skip to content
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

OnBackPress not working properly #255

Open
dhng22 opened this issue Nov 18, 2023 · 10 comments · May be fixed by #273
Open

OnBackPress not working properly #255

dhng22 opened this issue Nov 18, 2023 · 10 comments · May be fixed by #273

Comments

@dhng22
Copy link

dhng22 commented Nov 18, 2023

This issue was stated in #154, i'm also facing the same issue, where override onBackPress when there's only one Screen inside Navigator will has no effect

@DevNatan DevNatan linked a pull request Dec 10, 2023 that will close this issue
@DevSrSouza
Copy link
Collaborator

DevSrSouza commented Feb 27, 2024

I have tested on the Voyager samples, and the back press closes the app when the onBackPressed = { true }, can you share more examples of your issue?

@DevSrSouza
Copy link
Collaborator

Sorry, I previously did understand your use case, now I do.
The idea is that when there is only on Screen on the Stack, you will never be notified on the onBackPressed.

This design is a side effect caused because we want, where there is only one Screen on the stack, fallback to the System back press callback (for example, closing the app)

@dhng22
Copy link
Author

dhng22 commented Feb 29, 2024

@DevSrSouza Any reason for that inconsistency? All i want is to close the app's drawer when user go back, but my app only have one activity. Application with single activity is common now with Compose. So Voyager's BackHandler feature is literally useless with any application that has a single activity. I dont think this is a good idea since this "side effect" is not consistent. And instead of

the most cases, you want to user to close the app

I can decide whenever system should have the control over BackHandler. Isn't that much more easier than the current design which I can't do anything. Design should be consistent between activities, not disabled when there's one activity in the stack.

@DevSrSouza
Copy link
Collaborator

Can you try something like this:

class MyScreenWithDrawer : Screen {

   @Composable
   fun Content() {
        ...
        val coroutineScope = rememberCoroutineScope()
        val scaffoldState = rememberScaffoldState()
        BackHandler(enabled = scaffoldState.drawerState.isOpen) {
           coroutineScope.launch { scaffoldState.drawerState.close() }
        }
}

@dhng22
Copy link
Author

dhng22 commented Feb 29, 2024

Yes I'm writing code in expect/actual, that works normally. But it's just uncomfortable

@DevSrSouza
Copy link
Collaborator

To be clear, Voyager implementation works like this:

BackHandler is only applied when there is more the one Screen in the Stack, if there is only one, then, it is disabled on the system back handler will be executed.

If I recall correctly, the Scaffold API does not hold a BackHandler on it implementation to Close the Drawer, you have to implement by hand.

I can decide whenever system should have the control over BackHandler. Isn't that much more easier than the current design which I can't do anything. Design should be consistent between activities, not disabled when there's one activity in the stack.

Yes, if you want to have your own Custom BackHandler, you can pass the onBackPressed = null and implement your self how will Pop Navigation and how you will handle the Scaffold state, but your use case, I would recommend using a new BackHandler.

Yes I'm writing code in expect/actual, that works normally. But it's just uncomfortable

Yes, I can see how it can be, but I how the Android implementation works for Compose, with Voyager or without Voyager, this will be the same I think.

@DevSrSouza
Copy link
Collaborator

I tested here and I was right, without the BackHandler and without Voyager it just close the app.

Screen.Recording.2024-02-28.at.23.52.40.mov

So, the Voyager onBackPressed API is only for navigation purpose.

But still... Even that your use case is not actually a Voyager problem, is how things works in the Scaffold API, there actually is a use case that has a problem with Voyager onBackPressed, is that, when there is only one screen, the onBackPressed is never called. This would be useful if you want to prompt the user if they are sure they want to close the app for a example, but at same time, it can be implemented by Screen, by using the BackHandler API.

@dhng22
Copy link
Author

dhng22 commented Feb 29, 2024

I tested here and I was right, without the BackHandler and without Voyager it just close the app.

Well sure thing that it will close the app. I'm building my app from scratch that even my Drawer is neither from the Material API nor the Scaffold API, it's customized, I'm not using the API you mentioned. So i need the control over the BackHandler to know when to close my Drawer.

So, the Voyager onBackPressed API is only for navigation purpose.

Right, It's not a big deal to me, I'm just discussing about the consistency of the BackHandler feature, since it's supported by Voyager but useless to me. Have to write expect/actual really lost the meaning of a multiplatform library and an API that it supports

@DevSrSouza
Copy link
Collaborator

How I mention before, the Voyager onBackPressed API is only meant for Navigation used. Is different from the BackHandler API.

This is actually something that I have being look into, Being truly honest with your, is not the first time I have complete disabled Voyager backPressed for doing a more specific use case, maybe provide the BackHandler API as Multiplatform only for Android used can help other uses cases, but I'm not sure this is the right move, because we are providing APIs for Navigation, this can out of the Scope of the library.

@DevSrSouza
Copy link
Collaborator

This is why I have blocked #273 , currently onBackPressed API does not cover alot of cases like yours, a new API has to be think here. The currently implementation for the scope of the library, is almost perfect and simple, out of the scope of the library, not much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants