[#8940] Redesign Back, swipe-to-the-left and hardware device Back but… #9215
Conversation
Pull Request Checklist
|
Jenkins BuildsClick to see older builds (19)
|
flexsurfer
force-pushed
the
bug/back-hardware
branch
from
e799f54 to
e81c58c
Compare
flexsurfer
requested review from
vitvly and
dmitryn
and removed request for
a team
| @@ -0,0 +1,29 @@ | |||
| (ns status-im.ui.screens.routing.back-actions) | |||
|
|
|||
| (def back-actions {:chat :default | |||
There was a problem hiding this comment.
shouldn't you use :intro-wizard/navigate-back for some screens here?
There was a problem hiding this comment.
no for now, this PR is only to prevent broken states of the app, later we could decide what to do with cancel actions
There was a problem hiding this comment.
but what does it mean for this action? it prevents hardware button and 3 finger slide back but if you press the cancel button it works?
There was a problem hiding this comment.
Hardware back button is not prevented here, it still works.
There was a problem hiding this comment.
I think there's no way to disable hw back button unless resorting to RN's BackHandler.
98% of end-end tests have passedFailed tests (2)Click to expand
Passed tests (96)Click to expand |
|
@flexsurfer what should we test in this PR? |
|
so now there is a list of screens where you can use hardware back button on android or swipe on ios, for most onboarding screens and keycard screens hardware button or swipe will be unavailable |
|
@siphiuel @dmitryn, please review |
| @@ -0,0 +1,29 @@ | |||
| (ns status-im.ui.screens.routing.back-actions) | |||
|
|
|||
| (def back-actions {:chat :default | |||
There was a problem hiding this comment.
Hardware back button is not prevented here, it still works.
| (fn [action state] | ||
| (let [event (get back-actions/back-actions @view-id)] | ||
| (when (and (= (.-type action) (.-BACK navigation/navigation-actions)) event (not= :default event)) | ||
| (re-frame/dispatch [event])) |
There was a problem hiding this comment.
This dispatch is never invoked currently. So this means that create.core/intro-step-back is never invoked on Android hardware back or iOS swipe, and recover.core/cancel-pressed is never invoked for actions included above + clicking on the Back arrow. So wizard state is not updated. This causes e.g. these issues:
- If one presses Android's hardware back on Onboarding's
Select accountscreen, and then generates keys again, they are redirected toSelect key storagescreen bypassingSelect accountscreen - On Basic recovery's
Select storagescreen clicking on Cancel is not going to show any confirmation dialog.
There was a problem hiding this comment.
Maybe there are some more, but maybe we can get rid of wizard state being dependent on a specific step
There was a problem hiding this comment.
i didn't whitelist onboarding screens, so they will work fine because you can't use hardware buttons on them
There was a problem hiding this comment.
don't want to mess with wizard state in this PR, but we could try to get rid of it for sure
| ;; This atom stores event vector | ||
| ;; to be dispatched when a react-navigation's BACK | ||
| ;; actions is invoked | ||
| (def wizard-back-event (atom nil)) |
There was a problem hiding this comment.
Kudos for removing this atom in favor of back-actions, it was whacky.
| (let [event (get back-actions/back-actions @view-id)] | ||
| (when (and (= (.-type action) (.-BACK navigation/navigation-actions)) event (not= :default event)) | ||
| (re-frame/dispatch [event])) | ||
| (default-get-state-for-action action state)))) |
There was a problem hiding this comment.
For now we're always popping the item from stack here, disregarding whatever might happen in event handler, but when Cancel processing is in place (and thus a possibility to block navigation action), how should this work? Currently screens.navigation/processing-back-event? exists, will there be some alternative?
There was a problem hiding this comment.
we could try to find an alternative after v1, for now Cancel screens just ignore hardware button or swipe
flexsurfer
force-pushed
the
bug/back-hardware
branch
from
e81c58c to
fe5312d
Compare
|
@siphiuel thanks, added fix for Android https://github.com/status-im/status-react/pull/9215/files#diff-f65399a61bf325c3844d2b6b43bf1829R53 |
flexsurfer
force-pushed
the
bug/back-hardware
branch
from
9a1c783 to
c3a8ba8
Compare
…tons to align the behavior Signed-off-by: Andrey Shovkoplyas <[email protected]>
flexsurfer
force-pushed
the
bug/back-hardware
branch
from
c3a8ba8 to
cbe17cb
Compare
partially #8940
the first step of navigation improvement, for v1 we want to avoid broken app states, so i added a list of views there it's possible to navigate back safely