-
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
[#19921] feat: remove key pair action #20002
Conversation
Jenkins BuildsClick to see older builds (31)
|
2003e5b
to
3f7b94c
Compare
2d301da
to
4e02be3
Compare
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 stuff ✅
src/status_im/contexts/settings/wallet/keypairs_and_accounts/actions/view.cljs
Show resolved
Hide resolved
@mohsen-ghafouri One thing I noticed is that the removed-keypair toast should be in light mode 🌤️ |
4e02be3
to
6d92fad
Compare
@seanstrom good catch, I noticed they removed theme from toast's props. resolved. |
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.
LGTM! ✅
src/status_im/contexts/settings/wallet/keypairs_and_accounts/remove/view.cljs
Outdated
Show resolved
Hide resolved
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.
Looks good overall. Nice work 🚀 🙌 @mohsen-ghafouri
Just a component change in the confirmation sheet.
[quo/page-top | ||
{:container-style style/header-container | ||
:title (i18n/label :t/remove-key-pair-and-derived-accounts) | ||
:title-size :heading-2 | ||
:description :context-tag | ||
:context-tag {:type :icon | ||
:size 24 | ||
:context name | ||
:icon :i/seed-phrase}}] |
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.
@mohsen-ghafouri - We need to use the drawer-top
component here. That has the heading-2
text size.
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 think drawer-top
doesn't support icon, i will update it and use it here
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.
@smohamedjavid thanks for pointing it out, I replaced
60% of end-end tests have passed
Failed tests (20)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestWalletMultipleDevice:
Class TestWalletOneDevice:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMerged:
Expected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Passed tests (31)Click to expandClass TestDeepLinksOneDevice:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityMultipleDevicePRTwo:
Class TestActivityMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Class TestWalletOneDevice:
Class TestCommunityMultipleDeviceMerged:
|
f62e9e2
to
9235ceb
Compare
9235ceb
to
a560944
Compare
87% of end-end tests have passed
Failed tests (4)Click to expandClass TestWalletMultipleDevice:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestWalletOneDevice:
Expected to fail tests (3)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Passed tests (45)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestDeepLinksOneDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityMultipleDevicePR:
Class TestWalletOneDevice:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityMultipleDevicePRTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
|
@status-im/mobile-qa could you please check the test results? I skip manual QA as this action is still behind the feature flag |
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.
🚀
@mohsen-ghafouri sorry for delay, ready to go |
a560944
to
8b6861e
Compare
8b6861e
to
7a87cb4
Compare
fixes #19921
Summary
Implement the remove key-pair feature for the wallet settings.
Areas that maybe impacted
Steps to test
Result
Simulator.Screen.Recording.-.iPhone.13.-.2024-05-24.at.15.11.40.mp4
status: ready