-
Notifications
You must be signed in to change notification settings - Fork 979
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
[#19544] Show preselected networks in QR code #20070
Conversation
Jenkins BuildsClick to see older builds (24)
|
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.
🚀 Nice work!
(:test-preferred-chain-ids %) | ||
(:prod-preferred-chain-ids %))) |
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.
I almost forgot this is a set (which helps to perform the filter check below 😅 )
73% of end-end tests have passed
Failed tests (12)Click to expandClass TestWalletMultipleDevice:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestWalletOneDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Expected to fail tests (2)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Passed tests (38)Click to expandClass TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMerged:
Class TestDeepLinksOneDevice:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestWalletOneDevice:
Class TestActivityMultipleDevicePR:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityMultipleDevicePRTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
|
e6bdeef
to
1f1a7f5
Compare
(h/setup-subs {:dimensions/window-width 500 | ||
:mediaserver/port 200 | ||
:wallet/accounts [{:address "0x707f635951193ddafbb40971a0fcaab8a6415160" | ||
:name "Wallet One" | ||
:emoji "😆" | ||
:color :blue}] | ||
:wallet/preferred-chain-names-for-address #{:eth :opt :arb1}}))) |
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.
Added one more sub to make tests pass
;; NOTE: Fails with error below possibly due to Infura outage: | ||
;; FAIL ./status_im.contexts.shell.share.wallet.component_spec.js | ||
;; ● share wallet addresses › should display the multichain account | ||
;; | ||
;; No protocol method IDeref.-deref defined for type undefined | ||
(h/test-skip "should display the multichain account" | ||
(h/test "should display the multichain account" |
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.
This error isn't due to Infura, the error is because we are trying to perform a deref
(@
) on a nil
value.
As you may know, we use rf/sub
to subscribe, it's a wrapper that performs a deref
on a call to (rf/subscribe [...])
, so when we have no subscription we are calling (deref nil)
.
Now this test is passing :)
cc: @J-Son89 @siddarthkay
f29ecf4
to
d8bf6c3
Compare
Hi @ulisesmac ! Thanks for your PR. Do you know should this also work vice verse? 👇 video_2024-05-20_18-45-55.mp4 |
Hey @mariia-skrypnyk , Thanks for testing! If we edit the preferences in the share screen, the app shouldn't save changes: |
Thanks @ulisesmac ! So am I right that this rule also works not only for Receive but for Share screen? |
The expected behavior is:
Please ping me on Discord if this isn't clear enough, also I think the note in figma helps unless we are talking about different things 🤔 |
@ulisesmac thanks, everything is clear! Your implementation looks good on both platforms. |
50% of end-end tests have passed
Failed tests (6)Click to expandClass TestWalletMultipleDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestWalletOneDevice:
Passed tests (6)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
|
Failed e2e are not related and you can merge! |
d8bf6c3
to
bbd8ae9
Compare
fixes #19544
Summary
While displaying a QR code we didn't show the preconfigured preferred networks, this PR fixes that:
Screencast.from.2024-05-16.09-44-09.webm
And we are still able to edit the networks in the Share screen whithout saving the changes:
Screencast.from.2024-05-16.09-48-58.webm
Testing notes
The saved preferred networks in normal mode are different than the ones saved in testnet mode. So a user may prefer
:eth
in normal mode and:arb1
in testnet. This is being properly handled.Platforms
status: ready