voyager
voyager copied to clipboard
adrielcafe
Change back handler behavior
Update back handler behavior to work according to docs
Fixes #154 Fixes #255 As well #72
Current behavior
| Method | How it works |
|---|---|
onBackPressed = null |
✅ stack size = 0: default platform behavior :x: stack size > 0: default platform behavior Should pop backstack instead. BackHandler is not being handled properly, app is being closed. |
onBackPressed = { false } |
:x: stack size = 0: default platform behavior Back press being supplied means that the developer will take care of that. According to docs "return false won't pop the current screen" is not actually true✅ stack size > 0: does nothing |
onBackPressed = { true } |
✅ stack size = 0: default platform behavior ✅ stack size > 0: pop |
New behavior
| Method | How it works |
|---|---|
onBackPressed = null |
✅ stack size = 0: default platform behavior ✅ stack size > 0: pop |
onBackPressed = { false } |
✅ stack size = 0: does nothing✅ stack size > 0: does nothing |
onBackPressed = { true } |
✅ stack size = 0: default platform behavior ✅ stack size > 0: pop |
Tested using sample:multiplatform
I don't think we should force BackHandler always. There are use cases where you make voyager backHandler nullable and use Compose BackHandler by hand.
Why are you blocking a fix to a clear bug. You literally have to spam the back button to get it to work and that is in NO WAY a good user experience. This library is not coded by monkeys (devs overall did a great job) so I am 100% sure the intended behavior is not to have to do this to use the back button so why block a fix? Sure look at better ways but this should be merged in as a stopgap to prevent people from dealing with this honestly garbage back handling.
I get that code is not perfect and everyone makes mistakes. This is natural and normal. What is not normal is when you have a issue as problematic as this, and then are handed a fix on a silver platter, you just stall it for now close to 1.5 months
I don't think we should force BackHandler always. There are use cases where you make voyager backHandler nullable and use Compose BackHandler by hand.
This is not a problem created by this PR, this is how it works by default. You cannot hold OP liable for issues that are already in the design of the library. All OP did was modify default behavior. You can re code the current behavior by forcing false and overriding onBackPressed so this should be a non issue.
@DevNatan can you have a change here? Not having any behavior when the BackPressed callback is null? This is avoid Voyager emit a BackHandler when some use case requires to rewrite the BackHandler with custom approach and set the backPressed to null, for example, this is the actual currently recommendation for accessing navigator on the Back Pressed callback (see https://github.com/adrielcafe/voyager/issues/333).