Implement Quo2 Avatar /Wallet Avatar component #17703
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
J-Son89
reviewed
Oct 20, 2023
Jenkins BuildsClick to see older builds (15)
|
J-Son89
approved these changes
Oct 20, 2023
src/quo/core.cljs
Outdated
Comment on lines
159
to
162
| (def user-avatar quo.components.avatars.user-avatar.view/user-avatar) | ||
| (def wallet-user-avatar quo.components.avatars.wallet-user-avatar/wallet-user-avatar) | ||
| (def user-avatar quo.components.avatars.wallet-user-avatar.view/wallet-user-avatar) |
There was a problem hiding this comment.
There's a redefined variable here, is this intended?
|
Code looks good in general, but we should make sure we are not breaking other components or screens as there are some usages of |
J-Son89
reviewed
Oct 23, 2023
| :text-align :center | ||
| :justify-content :center | ||
| :align-items :center | ||
| :background-color (circle-color customization-color)}) |
There was a problem hiding this comment.
btw if you look closely at the designs, you will notice the last variant is not using customization-color - we need to make this the default for when no customization-color prop is set and this needs to be there for the designs.
@briansztamfater recently added this but it seems to have been removed here 👍
tumanov-alex
force-pushed
the
16608-quo-wallet-avatar-component
branch
2 times, most recently
from
b5b6ddb
to
d22ce23
Compare
Francesca-G
approved these changes
Oct 30, 2023
tumanov-alex
force-pushed
the
16608-quo-wallet-avatar-component
branch
from
426c608
to
0568f02
Compare
|
@churik can we run this pr through e2e please? |
hey @tumanov-alex , thanks for the PR! |
84% of end-end tests have passedFailed tests (4)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Expected to fail tests (3)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Passed tests (38)Click to expandClass TestActivityMultipleDevicePRTwo:
Class TestCommunityMultipleDeviceMerged:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
|
0% of end-end tests have passedFailed tests (4)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
|
|
Hey @tumanov-alex, thanks for your work! The only thing I noticed is that an error appears when you clear the video_2023-11-02_16-29-33.mp4 |
tumanov-alex
force-pushed
the
16608-quo-wallet-avatar-component
branch
from
0568f02
to
567aa79
Compare
Rewrite docs
Use get-initials utility
Move wallet-user-avatar to a separate folder (to conform with other adjacent components)
Add tests
Refactor to reflect conventions
Fix lint
Fix lint
Fix color usage
Change "color" to "customization-color"
Remove extra props
Refactor component properties object
Update color usage to comply with the latest conventions
Use correct color for circle-color
Fix require to follow conventions
Fix tests
Remove extra code
Do refactor
Refactor
Add new changes
Simplify two lines
Simplify two lines
Move folder into quo
Fix paths
fix require line
Use one letters instead of two for size 24
tumanov-alex
force-pushed
the
16608-quo-wallet-avatar-component
branch
from
567aa79
to
d86d6f4
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 #16608
Refactor the already existing component
Testing notes