Implement Wallet - Account Switcher #18003
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
smohamedjavid
requested review from
tumanov-alex,
briansztamfater,
J-Son89,
OmarBasem and
ulisesmac
Jenkins BuildsClick to see older builds (23)
|
|
just a heads up this pr will be merged shortly- not sure if it affects this pr but it moved the account switcher to a centralised location 👍 - #17946 |
smohamedjavid
force-pushed
the
feature/account-switcher
branch
from
5f45859 to
b4ec617
Compare
|
@smohamedjavid it's still marked as darft. is that by accident? if not, please let us know when it's ready to review 👍 |
smohamedjavid
force-pushed
the
feature/account-switcher
branch
2 times, most recently
from
7977bcb to
ad9ce1e
Compare
@ulisesmac - I was adding tests and fixing bugs yesterday. So, marked it as a draft. |
J-Son89
reviewed
Nov 28, 2023
J-Son89
reviewed
Nov 28, 2023
J-Son89
reviewed
Nov 28, 2023
J-Son89
approved these changes
Nov 28, 2023
smohamedjavid
force-pushed
the
feature/account-switcher
branch
from
ad9ce1e to
ac9a6bb
Compare
82% of end-end tests have passedFailed tests (4)Click to expandClass TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Expected to fail tests (4)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Passed tests (37)Click to expandClass TestActivityMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityMultipleDevicePRTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
|
ulisesmac
approved these changes
Nov 30, 2023
src/status_im2/contexts/wallet/common/sheets/account_options/view.cljs
Outdated
Show resolved
Hide resolved
smohamedjavid
force-pushed
the
feature/account-switcher
branch
from
ac9a6bb to
c0b627c
Compare
|
@smohamedjavid amazing work, thank you! |
smohamedjavid
force-pushed
the
feature/account-switcher
branch
from
c0b627c to
5025a50
Compare
@qoqobolo - Thank you for testing this PR. |
|
Thanks @smohamedjavid! One question: On Android, when the height of the bottom sheet is max, the top margin differs from iOS and design, is this expected? video_2023-12-01_16-47-53.mp4Designs/iOS 
|
@qoqobolo - This is odd, as I could not see this issue on my end.
We didn't have any full-height bottom sheets before this PR. So, it's hard to test with other bottom sheets. Can you help with the device model you are using to test? We will try to debug. |
Sure, it's reproducible on Google Pixel 7a |
@qoqobolo - Thank you for sending the device modal. I tried it on Google Pixel 7a, and it's reproducible. I checked the existing max height calculation (window height less (-) OS's top (bar) safe area). It works as expected on iOS devices and for Android devices that don't have a punch-hole selfie camera. The sheet's max height behaves differently on devices with a punch-hole selfie camera than the devices without it. Please help me create a separate issue, as adjusting the bottom sheet max-height calculation should not affect existing sheets in the app. |
Will do!👌 No more questions from my side, PR can be merged. |
Signed-off-by: Mohamed Javid <19339952+smohamedjavid@users.noreply.github.com>
Signed-off-by: Mohamed Javid <19339952+smohamedjavid@users.noreply.github.com>
Signed-off-by: Mohamed Javid <19339952+smohamedjavid@users.noreply.github.com>
Signed-off-by: Mohamed Javid <19339952+smohamedjavid@users.noreply.github.com>
Signed-off-by: Mohamed Javid <19339952+smohamedjavid@users.noreply.github.com>
Signed-off-by: Mohamed Javid <19339952+smohamedjavid@users.noreply.github.com>
smohamedjavid
force-pushed
the
feature/account-switcher
branch
from
5025a50 to
503b250
Compare
yevh-berdnyk
pushed a commit
that referenced
this pull request
Dec 8, 2023
This commit: - Implements the Wallet Account Switcher for easy switching between wallet accounts - Updates the About tab in the accounts screen to display the correct account address and derivation path along with the profile. - Updates the account-item component to pass the state from the parent and refactors the depreciated color functions - Moves the handle-bar in the bottom sheet to a standalone component - Adds customization-color in account-origin component --------- Signed-off-by: Mohamed Javid <19339952+smohamedjavid@users.noreply.github.com>
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 #17368
Summary
This PR:
abouttab in the accounts screen to display the correct account address and derivation path along with the profile.account-itemcomponent to pass thestatefrom the parent and refactors the depreciated color functionshandle-barin the bottom sheet to a standalone component Figma LinkAccount.Switcher.mp4
Review notes
The
handle-barused in the bottom sheet is actually a separate component and it's moved to its respective ns. This helps us to create the gradient effect along with account options and a handlebar to be sticky on top of the sheet when the user scrolls through the accounts list. We have to make the sticky area to be positioned absolute to receive the sheet gesture.Platforms
Steps to test
User having only one account
User having more than one account
status: ready