The last collectible is shown for a moment while opening a new collectible #18750 #18793
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
Jenkins BuildsClick to see older builds (48)
|
|
Hey @mmilad75 I can see in this PR, we show a blank screen for a while. We can reduce the exposure to that screen (and actually probably solve it) by triggering the navigation event at the right moment. Currently, when we press a collectible, we:
The blank screen can be reduced if we change the order of the events to:
Since this PR is too related I think it's a place to address it. |
@ulisesmac Done. Please have another look |
J-Son89
approved these changes
Feb 13, 2024
ulisesmac
approved these changes
Feb 13, 2024
Comment on lines
124
to
127
| (if collectible | ||
| {:fx [[:dispatch [:wallet/store-last-collectible-details collectible]]]} | ||
| {:fx [[:dispatch [:wallet/store-last-collectible-details collectible]] | ||
| [:dispatch [:navigate-to :wallet-collectible]]]} | ||
| (log/error "failed to get collectible details" |
There was a problem hiding this comment.
Hey @mmilad75
thanks for applying the suggestion, but I think we could delay the navigation even more, inside the event that is storing:
wdyt?
There was a problem hiding this comment.
Sure 👍 Let me apply it
smohamedjavid
approved these changes
Feb 14, 2024
50% of end-end tests have passedFailed tests (23)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Expected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Passed tests (24)Click to expandClass TestActivityMultipleDevicePR:
Class TestActivityMultipleDevicePRTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestDeepLinksOneDevice:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityOneDeviceMerged:
|
|
hi @mmilad75 thank you for PR. Take a look please at found issue. For me, it's reproducible only on IOS Unfortunately, there are no exact steps to reproduce it. It happens randomly to me. I hope the logs might help understand what's going on. ISSUE 1: Collectible is opened twice on iOSSteps:
Actual result:The collectible is opened twice. twice.mp4Expected result:The collectible is opened once. Device:
Logs: |
It seems we are navigating twice 👀 make sure we only have one call. |
vkjr
reviewed
Feb 16, 2024
| (rn/use-effect | ||
| (fn [] | ||
| #(rf/dispatch [:wallet/clear-last-collectible-details])) | ||
| []) |
There was a problem hiding this comment.
I think this empty vector can be omitted
There was a problem hiding this comment.
I want to run the code only in componentWillUnmount
vkjr
approved these changes
Feb 16, 2024
|
@VolodLytvynenko Hey man. Please have another look. The issue is fixed |
mmilad75
force-pushed
the
milad/18750-clear-last-collectible-on-leave
branch
from
ca72ef9
to
eb36e09
Compare
0% of end-end tests have passedFailed tests (47)Click to expandClass TestActivityMultipleDevicePRTwo:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestDeepLinksOneDevice:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityMultipleDevicePR:
Expected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
|
|
Hi @mmilad75. Thank you for PR. No issues from my side. PR is ready to be merged |
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 #18750
This PR clears the data saved on
:last-collectible-detailswhen the collectible screen is closed.Screen.Recording.2024-02-12.at.19.55.17.mov