Community: Request to join - Show addresses shared #18180
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 (12)
|
FFFra
suggested changes
Dec 14, 2023
| [rn/view | ||
| [quo/text | ||
| {:size :paragraph-1 | ||
| :weight :semi-bold} (:name item)] |
There was a problem hiding this comment.
I think (:name item) should be next line
| (map #(assoc % :customization-color (:color %))))] | ||
| [rn/safe-area-view {:style style/container} | ||
|
|
||
| [quo/drawer-top |
There was a problem hiding this comment.
hm, I was not aware about this. Think I will also implement on mine.
Can you see the tag background color? Mine is transparent.
Also, what If we include the info button as part of this PR?
There was a problem hiding this comment.
hm, I was not aware about this. Think I will also implement on mine.
Sure
Can you see the tag background color? Mine is transparent.
Background is there
Also, what If we include the info button as part of this PR?
Done, the quo component requires some updates to fix the button background.
Comment on lines
12
to
21
| [quo/account-avatar (assoc item :size 32)] | ||
| [rn/view | ||
| [quo/text | ||
| {:size :paragraph-1 | ||
| :weight :semi-bold} (:name item)] | ||
| [quo/address-text | ||
| {:address (:address item) | ||
| :format :short}]]]) |
There was a problem hiding this comment.
Do we need this?
We have the [quo.components.list-items.account.view :as account-view] which can be passed as:
(defn- render-item
[item]
[account-view/view
{:account-props item
:emoji (:emoji item)}])
Please let me know if Im wrong so we can follow the same pattern.
There was a problem hiding this comment.
Continue using the existing account component in your implementation, that's the correct component for your screen as per Figma. However, for this specific screen, the component we should use is account-permissions which is not implemented yet. I've started the implementation and will finish it once this PR is merged. Later on, we can replace this custom component with the quo component in a subsequent PR.
It's more efficient to handle the quo implementation in separate PRs. Combining these changes may slow us down, as they necessitate testing and design review.
ajayesivan
force-pushed
the
17977-show-addresses
branch
2 times, most recently
from
1b0b503 to
c6d6083
Compare
|
Great work, and thank you for clarifying my questions! |
FFFra
approved these changes
Dec 15, 2023
ajayesivan
force-pushed
the
17977-show-addresses
branch
from
c6d6083 to
1335abd
Compare
62% of end-end tests have passedFailed tests (13)Click to expandClass TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityMultipleDeviceMerged:
Expected to fail tests (5)Click to expandClass TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMergedTwo:
Passed tests (30)Click to expandClass TestCommunityOneDeviceMerged:
Class TestActivityMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityMultipleDevicePRTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestDeepLinksOneDevice:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMergedTwo:
|
ajayesivan
force-pushed
the
17977-show-addresses
branch
from
1335abd to
2d8ed23
Compare
77% of end-end tests have passedFailed tests (7)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Expected to fail tests (4)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Passed tests (37)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestDeepLinksOneDevice:
Class TestCommunityMultipleDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestActivityMultipleDevicePR:
Class TestActivityMultipleDevicePRTwo:
|
|
Hi @status-im/mobile-qa, this PR doesn't require manual QA since all changes are under a flag. Could you please check the e2e result and let me know if it's okay to merge? Thanks! |
|
Good to go @ajayesivan |
|
@ajayesivan please, do not skip the design review if it is required for this PR, thanks! |
|
@churik, currently, our emphasis is not on the design aspect for this feature. We're taking a different approach by initially concentrating on the functional aspects. In the later stages of development, we plan to align it with the Figma design. Until the feature is fully developed, we'll keep it hidden on the develop branch using a flag. Design reviews will be requested once the feature is ready; for now, we can skip them. |
ajayesivan
force-pushed
the
17977-show-addresses
branch
from
2d8ed23 to
d20a6d2
Compare
ulisesmac
pushed a commit
that referenced
this pull request
Dec 15, 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 #17977
Description
Implements an "Addresses for Permissions" sheet to display the addresses shared with a community when initiating the joining process. This enhancement aims to provide users with visibility into the addresses shared with the community.
Screenshot
Review Note
This PR is not intended to achieve pixel-perfect design. Rather, it focuses on implementing the functionality.
Test Note
To test this, change the
community-accounts-selection-enabled?flag insrc/status_im2/config.cljstotrue.This feature is currently under a flag that is not enabled in the develop branch, so these changes do not require manual QA.