Refactor tag preview screens to new api #17549
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
ajayesivan
force-pushed
the
refactor-tag-previews-to-new-api
branch
2 times, most recently
from
4771dde
to
4b437b1
Compare
Jenkins BuildsClick to see older builds (51)
|
ajayesivan
requested review from
FFFra,
smohamedjavid,
jo-mut and
mohsen-ghafouri
ajayesivan
changed the title
J-Son89
reviewed
Oct 5, 2023
| @@ -33,7 +33,7 @@ | |||
| opts | |||
| { | |||
| :size :small/:big | |||
| :token-img-src :token-img-src | |||
| :img-src :img-src | |||
| [rn/view | ||
| {:margin-top 20 | ||
| :align-self :flex-end} | ||
| [permission-tag/tag |
There was a problem hiding this comment.
Suggested change
| [permission-tag/tag | |
| please use public quo2.core namespace | |
| [quo2.core :as quo] | |
| ``` | |
| [quo/tag |
| @@ -1,10 +1,9 @@ | |||
| (ns status-im2.contexts.quo-preview.tags.status-tags | |||
| (:require [quo2.components.tags.status-tags :as quo2] | |||
There was a problem hiding this comment.
Suggested change
| (:require [quo2.components.tags.status-tags :as quo2] | |
| (:require [quo2.core :as quo] |
| [quo2.foundations.colors :as colors] | ||
| [quo2.components.tags.tag :as tag] | ||
| [status-im.ui.components.react :as react] | ||
| (:require [quo2.components.tags.tag :as tag] |
There was a problem hiding this comment.
Suggested change
| (:require [quo2.components.tags.tag :as tag] | |
| (:require [quo2.core :as quo] |
| @@ -1,50 +1,40 @@ | |||
| (ns status-im2.contexts.quo-preview.tags.tags | |||
| (:require [quo2.components.tags.tags :as tags] | |||
There was a problem hiding this comment.
Suggested change
| (:require [quo2.components.tags.tags :as tags] | |
| (:require [quo2.core :as quo] |
| (merge @state | ||
| {:token-img-src (if (= (get-in @state [:token]) "ETH") eth-token snt-token)})]]]]))) | ||
| [rn/view {:style {:align-items :center}} | ||
| [quo2/token-tag |
There was a problem hiding this comment.
please use public quo2.core namespace
[quo2.core :as quo]
```
[quo/token-tag
jo-mut
approved these changes
Oct 5, 2023
ajayesivan
force-pushed
the
refactor-tag-previews-to-new-api
branch
from
4b437b1
to
022d113
Compare
|
@mohsen-ghafouri Thanks for the review! I have made the suggested changes. |
51% of end-end tests have passedFailed tests (21)Click to expandClass TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityMultipleDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Passed tests (22)Click to expandClass TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityMultipleDevicePRTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
|
86% of end-end tests have passedFailed tests (6)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMergedTwo:
Passed tests (37)Click to expandClass TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityMultipleDevicePRTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityMultipleDeviceMerged:
Class TestActivityMultipleDevicePR:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestGroupChatMultipleDeviceMergedNewUI:
|
ajayesivan
force-pushed
the
refactor-tag-previews-to-new-api
branch
from
022d113
to
b4ce131
Compare
|
@ajayesivan any clues where in app this component can be used? |
|
Hi @churik! The token-tag component is being used in
|
|
@ajayesivan component itself looks great on preview, but likely not used yet on the implementation (the same as in nightly, so I do not think it is something related to this PR) so I'd suggest merging this PR after passing the design review (cc @Francesca-G ) Thank you! |
Francesca-G
reviewed
Oct 12, 2023
ajayesivan
force-pushed
the
refactor-tag-previews-to-new-api
branch
from
b4ce131
to
a689c33
Compare
ajayesivan
force-pushed
the
refactor-tag-previews-to-new-api
branch
3 times, most recently
from
b4d9bde
to
0b65b68
Compare
|
@Francesca-G I have fixed the component review issues. Could you please have another look? Thanks! |
Francesca-G
approved these changes
Oct 24, 2023
ajayesivan
force-pushed
the
refactor-tag-previews-to-new-api
branch
3 times, most recently
from
7764977
to
32edaf5
Compare
ajayesivan
force-pushed
the
refactor-tag-previews-to-new-api
branch
from
32edaf5
to
1f77331
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.
Part of Preview Screen Migration: #17288
Migrated all the tags preview screens to the new API.
Alongside the migration, I've also addressed two minor issues:
token-tagcomponent was not displaying the token image as expected.token-tagcomponent prop usage.Testing Notes
status: ready