-
Notifications
You must be signed in to change notification settings - Fork 987
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[#21270] - Collectibles stuck in loading for watch only accounts #21286
Conversation
Jenkins BuildsClick to see older builds (8)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although this PR is fixing the loading?
logic, I'm not too happy with the solution.
The loading?
property previously added seems not reactive (i.e. if it's true
when the component is mounted and then switched to false
, it'll be ignored). Although it works because we are re-mounting the listing, seems not clean.
IMO, the property could have been named :always-loading?
to clearly show its purpose.
Now with the changes, the loading?
property is being checked during the calculation of the component, but we are still remounting the listing to switch from "collectibles loading" -> "loaded collectibles".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing this @ulisesmac 🙌
IMO, the property could have been named :always-loading? to clearly show its purpose.
I have no strong opinion on this. The component is supposed to have a prop status
with values accepted as default
or loading
. However, the component handles the loading state due to the image fetching. Hence, the loading?
prop is introduced to be controllable from the parent without many changes in the component (I don't want to break the component, as it's essential to handle the image loading time). I'm fine with any name 👍 .
16b7726
to
7ee38b0
Compare
71% of end-end tests have passed
Failed tests (2)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestWalletMultipleDevice:
Passed tests (5)Click to expandClass TestWalletOneDevice:
Class TestCommunityOneDeviceMerged:
Class TestWalletMultipleDevice:
Class TestCommunityMultipleDeviceMerged:
|
43% of end-end tests have passed
Failed tests (4)Click to expandClass TestWalletMultipleDevice:
Class TestCommunityOneDeviceMerged:
Passed tests (3)Click to expandClass TestWalletOneDevice:
Class TestCommunityMultipleDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
|
50% of end-end tests have passed
Failed tests (2)Click to expandClass TestWalletMultipleDevice:
Passed tests (2)Click to expandClass TestCommunityOneDeviceMerged:
|
Hi @ulisesmac ! Thanks for your fix. |
90aaf52
to
ad74b1c
Compare
fixes #21270
Summary
This PR fixes the exception thrown on Android when a loading collectible is pressed.
Although this PR solves the exception, the root issue (collectibles are stuck in a loading state) happen sometimes.
It was reported in:
And we tried to solve it in:
But it's still reproducible (waited for more than a minute, and actually after 15mins they never loaded):
Screencast.from.2024-09-18.19-17-40.mp4
What is this PR doing then?
This PR:
✅ Fixes the loading collectible state on Android (it only worked on iOS)
✅ When the user presses a loading collectible, it won't throw any error/exception.
Review notes
I'll reopen the root issue.
Platforms
Steps to test
[Android]
0x00000000219ab540356cbb839cbe05303d7705fa
[iOS]
status: ready