fix(wallet): native share should point to opensea link #20222
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
J-Son89
requested review from
ulisesmac,
OmarBasem,
mmilad75,
briansztamfater and
BalogunofAfrica
Jenkins BuildsClick to see older builds (45)
|
OmarBasem
approved these changes
May 29, 2024
| :chain-id chain-id | ||
| :token-id token-id | ||
| :contract-address contract-address | ||
| }]) |
| :token-id token-id | ||
| :contract-address | ||
| contract-address | ||
| }])}])}]))) |
vkjr
reviewed
Jun 5, 2024
| {:options | ||
| {:title title | ||
| :subject title | ||
| :message uri}}]]})) | ||
|
|
||
| (rf/reg-event-fx :wallet/share-collectible |
There was a problem hiding this comment.
I think that in-sheet? as an argument adds unnecessary knowledge about the outer context to this event. Instead we can pass dispatch-timeout and make decision of selecting dispatch vs dispatch-later based on it. And :hide-bottom-sheet could be called outside, where knowledge about UI details exist. This will make :wallet/share-collectible more flexible.
Wdyt?
There was a problem hiding this comment.
yes, that's a good point! will adjust to that!
There was a problem hiding this comment.
hmm on reviewing this, I think one thing to consider is that in-sheet? will hide the bottom sheet.
But anyway I see there is only one use of this function and it's in a bottom sheet, so I'll just remove that parameter altogether :)
vkjr
reviewed
Jun 5, 2024
| address (get-in id [:contract-id :address])] | ||
| (let [chain-id (get-in id [:contract-id :chain-id]) | ||
| token-id (:token-id id) | ||
| contract-address (get-in id [:contract-id :address])] | ||
| (rf/dispatch [:show-bottom-sheet | ||
| {:content (fn [] | ||
| [options-drawer/view |
There was a problem hiding this comment.
In few places parameters chain-id, token-id and contract-address are all extracted from one id param before passing to options-drawer/view. Instead we can pass just id and make the extraction inside options-drawer/view to reduce text repetition. This will also reduce surface for changes if structure of id will change in a future.
There was a problem hiding this comment.
thanks for the great suggestions! Will update :)
vkjr
approved these changes
Jun 5, 2024
ulisesmac
approved these changes
Jun 5, 2024
mmilad75
approved these changes
Jun 5, 2024
J-Son89
force-pushed
the
jc/opensea-share
branch
2 times, most recently
from
9678ffb to
933bec3
Compare
84% of end-end tests have passedNot executed tests (1)Failed tests (5)Click to expandClass TestWalletMultipleDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMergedTwo:
Expected to fail tests (3)Click to expandClass TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestWalletOneDevice:
Passed tests (43)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestActivityMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestDeepLinksOneDevice:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityMultipleDevicePRTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestWalletOneDevice:
|
|
Hey @J-Son89! Thank you for the PR. Please take a look at the issue ISSUE 1 After selecting app for sharing collectible link - it is opened within Status app (Android)Android only. Reproduced on Samsung Galaxy A52, Android 12 Steps:
Actual result: app is opened within Status app screencast.2024-06-10.13-23-15.mp4Expected result: app should be opened in separate tab. Below is an example of sharing community link. sharing.comm.link.mp4 |
|
thanks @pavloburykh - was not aware of this. Will address! 🙏 |
J-Son89
force-pushed
the
jc/opensea-share
branch
from
8422de0 to
a7ac971
Compare
thank you @J-Son89! Please, ping me up once PR is ready for re-test. |
J-Son89
force-pushed
the
jc/opensea-share
branch
from
a7ac971 to
89e0798
Compare
|
@pavloburykh issue 1 should be addressed now. Apologies for the delay. |
|
@J-Son89 thank you for the fixes. Please take a look at the new issue. ISSUE 2 IOS crashes when sending collectible via longtap menuIOS ONLY Reproducing on iPhoneX, IOS 16.7.7 Steps:
Actual result: app crashes telegram-cloud-document-2-5445185058564493526.mp4 |
Can't reproduce it on iPhone 11 Pro max, IOS 17.5.1. Collectibles are shared without crash for me |
|
Can't reproduce ISSUE 2 either (iPhone SE, iOS 17.5.1) |
BalogunofAfrica
approved these changes
Jun 20, 2024
J-Son89
force-pushed
the
jc/opensea-share
branch
3 times, most recently
from
df9101c to
b2b89e2
Compare
J-Son89
force-pushed
the
jc/opensea-share
branch
from
b2b89e2 to
d6992e6
Compare
|
@J-Son89 thank you! The issue is fixed for me. Ready for merge. |
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: #20223
This pr updates the native share for the collectibles in wallet so that the url it points to is for the Open Sea link
e.g https://opensea.io/assets/ethereum/0xa342f5d851e866e18ff98f351f2c6637f4478db5/93539746106169265185984122850742147987924040810729814653747710597909889193997
This has four points of entry. Home Page, Account Page, Collectible overview page, lightbox in collectible overview page.
Home Page:
Account Page:
In lightbox:
In overview page:
