Wallet: watch address - select random color #17859
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)
|
OmarBasem
changed the title
OmarBasem
requested review from
tumanov-alex,
briansztamfater,
smohamedjavid,
J-Son89 and
mmilad75
Merged
OmarBasem
commented
Nov 9, 2023
OmarBasem
requested review from
vkjr and
smohamedjavid
and removed request for
smohamedjavid
J-Son89
reviewed
Nov 9, 2023
J-Son89
reviewed
Nov 9, 2023
src/status_im2/contexts/wallet/add_address_to_watch/confirm_address/view.cljs
Show resolved
Hide resolved
J-Son89
approved these changes
Nov 9, 2023
84% of end-end tests have passedFailed tests (4)Click to expandClass TestCommunityOneDeviceMerged:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityMultipleDeviceMergedTwo:
Expected to fail tests (3)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestGroupChatMultipleDeviceMergedNewUI:
Passed tests (38)Click to expandClass TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityMultipleDevicePR:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityMultipleDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityMultipleDevicePRTwo:
|
vkjr
reviewed
Nov 9, 2023
| :offset (* (+ 48 8) index) | ||
| :index index}) | ||
|
|
||
| (defn- view-internal | ||
| "Options | ||
| - `default-selected` Default selected color name. |
There was a problem hiding this comment.
I think it would be helpful to add the description of feng-shui? option here, wdyt?
There was a problem hiding this comment.
feng-shui? is explained in the todo below that it is a temporary prop until b&w is developed. Currently, it shouldn't be used.
vkjr
approved these changes
Nov 9, 2023
pavloburykh
force-pushed
the
wallet/watch-color
branch
from
091c041
to
b4daf0c
Compare
71% of end-end tests have passedFailed tests (9)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityOneDeviceMerged:
Expected to fail tests (4)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Passed tests (32)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityMultipleDevicePR:
Class TestActivityMultipleDevicePRTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
|
67% of end-end tests have passedFailed tests (3)Click to expandClass TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Passed tests (6)Click to expandClass TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
|
|
@OmarBasem thank you for the PR! Got one question: shouldn't same behaviour be applied also for creating regular account? I see that for some reason only creating watch only account screen was mentioned in the issue. cc @J-Son89 |
|
Actually you are right, probably a slip up on my side when creating the issue. If it's not added here we can do that as a follow up issue. Whatever you both think is best 👍 |
@J-Son89 thank you for clarification! @OmarBasem it's up to you. Are you okay with implementing same behaviour for creating regular account screen in scope of this PR or prefer to log it separately? |
OmarBasem
force-pushed
the
wallet/watch-color
branch
from
b4daf0c
to
ffb3c3d
Compare
|
@pavloburykh thanks for your review. Fixed the issue 👍 |
Thank you @OmarBasem! No issues from my side. PR is ready for merge. |
OmarBasem
force-pushed
the
wallet/watch-color
branch
from
cdb0f1f
to
927d6c9
Compare
yevh-berdnyk
pushed a commit
that referenced
this pull request
Dec 8, 2023
Wallet: randomize color picker
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: #17806
This PR implements selecting a random color when adding an address to watch, and centering the color picker on that color. It also implements auto-scroll for the color picker to keep the selected color centered.
Demo:
Screen_Recording_20231109_125953_Status.Debug.mp4