adrielcafe / voyager

🛸 A pragmatic navigation library for Jetpack Compose
https://voyager.adriel.cafe
MIT License
2.31k stars 109 forks source link

onBackPressed override doesnt work #154

Open kodeplateform opened 1 year ago

kodeplateform commented 1 year ago

Hello , I'm using the voyager library with kodein with jetpack multiplateform and i need to handle the back gesture like i was doing it in jetpack compose with

BackHandler {}

I tried with :

Navigator(
  screen = HomeScreen(),
  onBackPressed = {
    true
  }
)

or

Navigator(
  screen = HomeScreen(),
  onBackPressed = {
    false
  }
)

or

Navigator(
  screen = HomeScreen(),
  onBackPressed = null
)

To disable backgesture on a first time , but in all these case the back gesture still works and the app is put on background

kodeplateform commented 1 year ago

I just tried with the multiplatform sample , and same behavior. I think the case where only one screen left on stack and onBackPressed = null is justnot handled . On that situation the user shouldnt be able to leave the app

derywat commented 9 months ago

I traced the problem down to the last line of the code below (voyager-navigator/src/commonMain/kotlin/cafe/adriel/voyager/navigator/Navigator.kt):

public class Navigator @InternalVoyagerApi constructor(
    screens: List<Screen>,
    @InternalVoyagerApi public val key: String,
    private val stateHolder: SaveableStateHolder,
    public val disposeBehavior: NavigatorDisposeBehavior,
    public val parent: Navigator? = null
) : Stack<Screen> by screens.toMutableStateStack(minSize = 1) {

Setting minSize = 1 causes navigator.canPop to return false with only one screen on stack (voyager-navigator/src/commonMain/kotlin/cafe/adriel/voyager/navigator/internal/NavigatorBackHandler.kt)

@Composable
internal fun NavigatorBackHandler(
    navigator: Navigator,
    onBackPressed: OnBackPressed
) {
    if (onBackPressed != null) {
        BackHandler(
            enabled = navigator.canPop || navigator.parent?.canPop ?: false,
            onBack = {
                if (onBackPressed(navigator.lastItem)) {
                    if (navigator.pop().not()) {
                        navigator.parent?.pop()
                    }
                }
            }
        )
    }
}

Maybe this will help someone getting it fixed.

DevNatan commented 8 months ago

@kodeplateform @derywat I can confirm, onBackPress behavior is incorrect actually and is not working according to docs. I'll implement a fix soon. Thanks!

dhng22 commented 7 months ago

@DevNatan I faced the same behaviour, any update?

DevSrSouza commented 3 months ago

By default, @kodeplateform , if you set the onBackPressed to null, you can use BackHandler. Of course, the BackHandler API is Android only, you will need to add Actual/Expect like we do on Voyager. This way you have full control on how you want the BackHandler logic to be.

We are looking into use cases and will try to fix this issue, the @DevNatan PR would fix but changes the behavior on other places that we can't do that for a library of this case with many users.

For now:


Navigator(
    ....,
    onBackPressed = null,
) {
    // this is the default Voyager implementation, you can change it how you want
    BackHandler(
        enabled = navigator.canPop || navigator.parent?.canPop ?: false,
        onBack = {
            if (onBackPressed(navigator.lastItem)) {
                if (navigator.pop().not()) {
                    navigator.parent?.pop()
                }
            }
        }
    )

   ... CurrentScreen() or Transition API if you use
}
DevSrSouza commented 3 months ago

The reason this is happening, is because when there is only one item in the Stack, the canPop returns false, then, the oficial BackHandler of Voyager is disabled. The reason is that, the most cases, you want to user to close the app, so, this is a expected behavior.

My suggestion above should work as expected. BackHandler(enabled = navigator.lastItem == YourScreenThatShouldNotCloseTheApp) { Do Something here }

stevdza-san commented 2 months ago

By default, @kodeplateform , if you set the onBackPressed to null, you can use BackHandler. Of course, the BackHandler API is Android only, you will need to add Actual/Expect like we do on Voyager. This way you have full control on how you want the BackHandler logic to be.

We are looking into use cases and will try to fix this issue, the @DevNatan PR would fix but changes the behavior on other places that we can't do that for a library of this case with many users.

For now:

Navigator(
    ....,
    onBackPressed = null,
) {
    // this is the default Voyager implementation, you can change it how you want
    BackHandler(
        enabled = navigator.canPop || navigator.parent?.canPop ?: false,
        onBack = {
            if (onBackPressed(navigator.lastItem)) {
                if (navigator.pop().not()) {
                    navigator.parent?.pop()
                }
            }
        }
    )

   ... CurrentScreen() or Transition API if you use
}

It would be nice if we could have a BackHandler for both Android, iOS inside the voyager library. Accessing back handler inside the specific screen is better then implementing it inside the root/parent composable.