Wallet/edit derivation path screen #17712 #17741
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
Jenkins BuildsClick to see older builds (32)
|
Rende11
changed the title
Rende11
force-pushed
the
wallet/edit-derivation-path-screen
branch
from
7cad577
to
bb4b1aa
Compare
J-Son89
requested review from
tumanov-alex,
OmarBasem,
J-Son89,
briansztamfater,
vkjr,
smohamedjavid,
ulisesmac and
mmilad75
and removed request for
OmarBasem
J-Son89
reviewed
Oct 27, 2023
J-Son89
reviewed
Oct 27, 2023
J-Son89
reviewed
Oct 27, 2023
src/status_im2/contexts/wallet/create_account/edit_derivation_path/component_spec.cljs
Outdated
Show resolved
Hide resolved
J-Son89
reviewed
Oct 27, 2023
src/status_im2/contexts/wallet/create_account/edit_derivation_path/style.cljs
Outdated
Show resolved
Hide resolved
J-Son89
reviewed
Oct 27, 2023
src/status_im2/contexts/wallet/create_account/edit_derivation_path/style.cljs
Show resolved
Hide resolved
Rende11
force-pushed
the
wallet/edit-derivation-path-screen
branch
from
bb4b1aa
to
2ae4dc2
Compare
ulisesmac
reviewed
Oct 28, 2023
src/status_im2/contexts/wallet/create_account/edit_derivation_path/component_spec.cljs
Outdated
Show resolved
Hide resolved
src/status_im2/contexts/wallet/create_account/edit_derivation_path/view.cljs
Outdated
Show resolved
Hide resolved
src/status_im2/contexts/wallet/create_account/edit_derivation_path/view.cljs
Outdated
Show resolved
Hide resolved
Rende11
force-pushed
the
wallet/edit-derivation-path-screen
branch
from
7f4593c
to
678fe7a
Compare
80% of end-end tests have passedFailed tests (6)Click to expandClass TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMergedTwo:
Expected to fail tests (3)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestGroupChatMultipleDeviceMergedNewUI:
Passed tests (36)Click to expandClass TestActivityCenterContactRequestMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityMultipleDevicePRTwo:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityMultipleDevicePR:
|
|
Hi @Rende11 , thanks for your contribution! Could you please point me to where exactly in the app I can test this screen? |
Rende11
force-pushed
the
wallet/edit-derivation-path-screen
branch
from
678fe7a
to
10837cd
Compare
|
@qoqobolo - the screen is located in the new wallet ui -> add new account -> click edit button on derivation path to note: this is just the ui structure added here and functionality etc is not so important. also some quo components were missing so placeholders were added and will be added once they're in place That preview screen is a previous quo component that just happens to have a similar name as the screen added here 👌 |
Simulator.Screen.Recording.-.iPhone.11.Pro.-.2023-10-31.at.15.23.49.mp4(long press on wallet, long press on "Illustration here") P.S. - Broken color pickers don't relate to these changes |
67% of end-end tests have passedFailed tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Passed tests (2)Click to expandClass TestCommunityOneDeviceMerged:
|
|
@Rende11 thanks again for your work, LGTM. |
@qoqobolo we are not necessarily doing design reviews on Wallet just yet as many things are likely to adjust and we would like to spare @Francesca-G of unnecessary effort. In some cases devs might request it and that's fine but for now we will leave them. I think after 1.27 release most of the features should be stable enough and we will start adding design reviews to the process then. |
Oh, I didn’t know about this nuance of the Wallet review process, thanks for the explanation @J-Son89. Yes, of course, in this case PR can be merged. |
fix lint errors Add edit action props Add illustration Update bottom_actions test Update usages and docs Fixes Specs Add actions Add tests Update styles Fix naming Roll back derivation path fn Update styles Fix styles Fixes Fix naming
Rende11
force-pushed
the
wallet/edit-derivation-path-screen
branch
from
10837cd
to
9b2cb64
Compare
yevh-berdnyk
pushed a commit
that referenced
this pull request
Dec 8, 2023
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 #17712
Summary
Wallet edit derivation path screen implementation
Review notes
bottom-actionscomponent affected and updatedquo_preview/preview.cljs changed - introduced
:pathparameterdropdown-inputcomponent not implementedTesting notes
Tested only on iOS
Platforms
Before and after screenshots comparison
Figma
status: ready