[#17965] Create token component #18018
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
ulisesmac
requested review from
vkjr,
tumanov-alex,
briansztamfater,
ajayesivan,
smohamedjavid,
J-Son89,
OmarBasem,
mmilad75 and
mohsen-ghafouri
ulisesmac
commented
Nov 29, 2023
ulisesmac
commented
Nov 29, 2023
Jenkins BuildsClick to see older builds (41)
|
ulisesmac
force-pushed
the
17965-create-token-component
branch
from
c942524 to
3562047
Compare
ulisesmac
commented
Nov 29, 2023
ilmotta
reviewed
Nov 29, 2023
J-Son89
reviewed
Nov 29, 2023
J-Son89
reviewed
Nov 29, 2023
yqrashawn
reviewed
Nov 30, 2023
ulisesmac
force-pushed
the
17965-create-token-component
branch
2 times, most recently
from
9e83d34 to
b86249d
Compare
|
@J-Son89 I've pushed the branch with the following changes:
@ilmotta @yqrashawn Thanks for your reviews 😃 |
mohsen-ghafouri
approved these changes
Dec 4, 2023
smohamedjavid
approved these changes
Dec 4, 2023
J-Son89
reviewed
Dec 4, 2023
src/quo/core.cljs
Outdated
| @@ -378,6 +379,9 @@ | |||
| (def text-combinations quo.components.text-combinations.view/view) | |||
| (def channel-name quo.components.text-combinations.channel-name.view/view) | |||
|
|
|||
| ;;;; Utilities | |||
There was a problem hiding this comment.
can you leave a comment here - something like "outside of design system" 👍
|
I checked this on all tokens that we have for our testing purposes, no difference - the only difference that I noticed is in sorting, take a look: Also, I checked components from the description + #17873
|
58% of end-end tests have passedNot executed tests (1)Failed tests (14)Click to expandClass TestActivityMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestDeepLinksOneDevice:
Expected to fail tests (6)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMerged:
Passed tests (28)Click to expandClass TestActivityMultipleDevicePRTwo:
Class TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestDeepLinksOneDevice:
|
churik
force-pushed
the
17965-create-token-component
branch
from
84f0c73 to
1cbe067
Compare
69% of end-end tests have passedFailed tests (9)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityMultipleDevicePRTwo:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityOneDeviceMerged:
Expected to fail tests (6)Click to expandClass TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityOneDeviceMerged:
Passed tests (34)Click to expandClass TestDeepLinksOneDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityMultipleDevicePR:
Class TestActivityMultipleDevicePRTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityOneDeviceMerged:
|
84% of end-end tests have passedFailed tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Expected to fail tests (7)Click to expandClass TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMerged:
Passed tests (41)Click to expandClass TestActivityMultipleDevicePRTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Class TestActivityMultipleDevicePR:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMerged:
Class TestDeepLinksOneDevice:
|
|
@churik |
|
Hey @churik
This is not related to my PR, but I checked the code and we are not sorting those tokens, so I'd say that right now, yes, it's random.
I asked to designers, and they said there's still no design for it, when it exists, we can add it 👍
No, we don't need it :) It's just a code change related. This implementation supports b64 strings to be passed to the token component and also makes it easier to get the image of a token, so I can understand it's fixing But I'd say to properly ensure it's working is better to keep the issue open and address it separately. @mohsen-ghafouri wdyt? |
ulisesmac
force-pushed
the
17965-create-token-component
branch
from
1cbe067 to
c0d04aa
Compare
|
@ulisesmac I can see you already resolved it and we just need to check it for custom token. feel free and reassign that issue to yourself if you want. :) |
The new component is being used in the following components: - token-input - network-amount - token-tag - summary-tag - context-tag - token-value - token-network - preview-list
ulisesmac
force-pushed
the
17965-create-token-component
branch
from
19799ad to
ab3c45e
Compare
yevh-berdnyk
pushed a commit
that referenced
this pull request
Dec 8, 2023
* Create token component * Replace token images by new token component The new component is being used in the following components: - token-input - network-amount - token-tag - summary-tag - context-tag - token-value - token-network - preview-list * Rename token image file: eth-token -> eth * Remove resources/get-token function
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 #17965
Summary
This PR creates a new token component, a similar abstraction to our icon component.
Everytime we want to draw a token, we should use this new implementation.
Features
This new component is defined in
quo.components.tokenand is also available in the quo library (same asiconcomponent). I created it to be very flexible, since it'll replace existing token images in the app.Receives:
:size-24,:size-2000or just2000:eth,:ETH,"ETH","eth"or even:my-token/eTh, all of them resolve to the same.token, the component will use this value, useful to render b64 strings. The issue: #17873 will benefit from it.Changes
Besides adding this component, it is being used in all places in the app where we rendered a token image.
So, inside the quo library the new component is being used in:
token-inputnetwork-amounttoken-tagsummary-tagcontext-tagtoken-valuetoken-networkpreview-listAll of these components received an image-source for the token to render, in this PR their API has been changed since they don't need that image-source anymore; it also means every component that used any of these components has also been updated in this PR
Also the usages of these components outside quo library have been updated.
Review notes
Some additional small changes were performed, just small refactors, I'll mention what they are in code comments below.
QA Testing notes
Please test all token images in the app. All of them should render exactly as before. Keep in mind this issue is not solving #17873 , but it can be solved after this one.
Platforms
status: ready