-
Notifications
You must be signed in to change notification settings - Fork 984
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
feat: Start bridge from asset drawer #19860
Conversation
2bfbd34
to
8a001a4
Compare
Jenkins BuildsClick to see older builds (24)
|
@@ -241,12 +241,16 @@ | |||
(rf/reg-event-fx :wallet/get-keypairs get-keypairs) | |||
|
|||
(rf/reg-event-fx :wallet/bridge-select-token | |||
(fn [{:keys [db]} [{:keys [token stack-id]}]] | |||
(fn [{:keys [db]} [{:keys [token stack-id from-drawer?]}]] |
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'd suggest to rename from-drawer?
-> open-modal?
because the event don't need to know any information about surrounding context, wdyt?
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 agree, thanks!
:on-press #(js/alert "to be implemented")}) | ||
:on-press (fn [] | ||
(rf/dispatch [:hide-bottom-sheet]) | ||
(rf/dispatch [:wallet/bridge-select-token |
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.
You should be using the bridging wizard instead and should add a new flow for bridge
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 agree @mmilad75, this is a ideal use case for wizard navigation.
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.
@mmilad75 fixed it. Could you please have a look at the code changes.
And here is a video:
Screen_Recording_20240502_094056_Status.mp4
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. Thanks man 🙏
8a001a4
to
b10c251
Compare
90% of end-end tests have passed
Failed tests (3)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestWalletMultipleDevice:
Expected to fail tests (2)Click to expandClass TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Passed tests (47)Click to expandClass TestActivityMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestDeepLinksOneDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestWalletOneDevice:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityMultipleDevicePRTwo:
|
Hi @OmarBasem thanks for the PR! ISSUE 1: Error when starting the bridge flowSteps:
video_2024-05-03_11-39-11.mp4Please do not forget to smoke-test PR changes on real devices when / if possible 🙏 Also, a wrong issue seems to be linked to this PR in the description (it's already closed #19807). Please edit the description accordingly. |
43aa085
to
0a9260d
Compare
@OmarBasem please lmk when the conflict and all the review comments are resolved, I'll recheck the PR, thanks! |
f5cea34
to
863449a
Compare
10% of end-end tests have passed
Failed tests (46)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMerged:
Class TestActivityMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityMultipleDevicePRTwo:
Class TestWalletMultipleDevice:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestDeepLinksOneDevice:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestWalletOneDevice:
Expected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Passed tests (5)Click to expandClass TestCommunityOneDeviceMerged:
|
80% of end-end tests have passed
Failed tests (8)Click to expandClass TestDeepLinksOneDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestWalletMultipleDevice:
Class TestWalletOneDevice:
Expected to fail tests (1)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Passed tests (37)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMerged:
Class TestActivityMultipleDevicePRTwo:
Class TestActivityMultipleDevicePR:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestWalletOneDevice:
|
hi @OmarBasem, thanks for the PR, e2e failures are not related |
Thanks @OmarBasem and @yevh-berdnyk! |
863449a
to
3c54307
Compare
3c54307
to
e31ab0a
Compare
fixes: #19806
This PR implements starting the bridge flow from the assets drawer.
Note: the bridge flow has some bugs and is still under development
Demo:
Screen_Recording_20240501_152120_Status.mp4