fix: hardware induced back actions #20183
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
OmarBasem
changed the title
OmarBasem
requested review from
J-Son89,
vkjr,
ulisesmac,
briansztamfater and
BalogunofAfrica
Jenkins BuildsClick to see older builds (57)
|
OmarBasem
changed the title
BalogunofAfrica
approved these changes
May 27, 2024
|
should we add some other teams as a reviewer to this pr because whatever approach we take for this we should probably standardise across the app? 🤔 |
OmarBasem
force-pushed
the
fix/back-unmount
branch
from
c4263b0 to
fe25882
Compare
|
Hey @status-im/mobile-devs, I would appreciate someone from outside wallet having a look on this PR since that bug could be present in other parts of the app also. In this PR I fixed the |
Parveshdhull
approved these changes
May 28, 2024
|
Hi @OmarBasem, Thank you very much great work. PR lgtm 🎉
I don't think the community has any on-close callback. The current solution in PR looks ok to me, but if you check https://github.com/status-im/status-mobile/pull/13379/files we used approach of registering/removing I don't remember technical details about why we did that but I do remember there were a few edge cases related to chat screen when timing was important. Like quickly reopening the chat screen and maybe un-mount getting dispatched at wrong time or something and ending up with blank chat etc. So current solution works, but probably more robust solution is to registering/removing Probably @flexsurfer might have more info. |
|
yes, this might lead to issues when closing and opening screen |
|
Thanks for your input @Parveshdhull!
What about using swipe gesture on iOS? cc @flexsurfer |
Yes, ios is problematic. You can find more details in this PR reviews #16035. Initialy I also opted for |
|
Maybe we can use both solutions 🤔. that way atleast android will be robust. |
@Parveshdhull I think this is not ideal. Personally as an iOS user I always use gestures for navigating. I rarely use top bar buttons.
I don't think this is ideal either, it introduces complexities to the code. I think the way it is okay for now (I also think that exiting and re-entering the screen quickly is likely to happen more on chat screen than other screens). I have opened an issue to investigate that problem further and find a more robust solution that works on both platforms #20226 👍 |
37% of end-end tests have passedFailed tests (29)Click to expandClass TestActivityMultipleDevicePR:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityMultipleDevicePRTwo:
Class TestCommunityMultipleDeviceMerged:
Class TestDeepLinksOneDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Class TestWalletMultipleDevice:
Expected to fail tests (4)Click to expandClass TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestWalletOneDevice:
Passed tests (19)Click to expandClass TestWalletOneDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
|
ulisesmac
reviewed
May 29, 2024
OmarBasem
force-pushed
the
fix/back-unmount
branch
from
fe25882 to
4a65605
Compare
|
Maybe we should remove the |
OmarBasem
force-pushed
the
fix/back-unmount
branch
from
3eaf935 to
70e029a
Compare
Finally!! Thank you @VolodLytvynenko for your testing and patience also! :) |
yevh-berdnyk
pushed a commit
that referenced
this pull request
Jul 14, 2024
* fix: hardware induced back actions (#20183)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes: #20182
Summary
This PR fixes hardware induced back actions in wallet. Currently we had the
on-closefunction to be called when pressing on theXbutton. This can cause bugs when navigation back using hardware button or swipe gesture. Theon-closefunction need to be calledon-unmountinstead.Demo
Example fix: currently in develop when disabling a network on send-amount screen then navigation back with hardware button, then going to send screen again, the network will stay disabled. The below video shows fix for that:
Screen_Recording_20240527_101000_Status.mp4