fix: community buttons #18392
Merged
fix: community buttons #18392
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
BalogunofAfrica
force-pushed
the
fix/community-buttons
branch
from
939aa32 to
704bce0
Compare
3 tasks
Jenkins BuildsClick to see older builds (20)
|
J-Son89
reviewed
Jan 4, 2024
| @@ -44,9 +44,10 @@ | |||
| (str title ": " content)]]) | |||
|
|
|||
| (defn view | |||
| [rules] | |||
| [rules scroll-enabled] | |||
There was a problem hiding this comment.
consider appending "?" -> scroll-enabled?
J-Son89
reviewed
Jan 4, 2024
| @@ -1,6 +1,8 @@ | |||
| (ns status-im.contexts.communities.actions.request-to-join.view | |||
| (:require | |||
| [quo.core :as quo] | |||
| [quo.foundations.colors :as colors] | |||
| [quo.theme :as quo.theme] | |||
jo-mut
approved these changes
Jan 4, 2024
J-Son89
reviewed
Jan 4, 2024
|
|
||
| (def community-rule-text | ||
| {:margin-left 6 | ||
| {:margin-left 0 |
J-Son89
reviewed
Jan 4, 2024
| @@ -33,7 +36,7 @@ | |||
| :weight :semi-bold | |||
| :size :heading-2} | |||
| (i18n/label :t/request-to-join)]] | |||
| [rn/view {:style {:margin-right :auto :margin-top 8}} | |||
| [rn/view {:style {:margin-right :auto :margin-top 4}} | |||
There was a problem hiding this comment.
maybe better to put in a style file?
J-Son89
reviewed
Jan 4, 2024
| {:size :paragraph-2 | ||
| :style style/final-disclaimer-text} | ||
| (i18n/label :t/request-to-join-disclaimer)]]]))) | ||
|
|
J-Son89
reviewed
Jan 4, 2024
| @@ -97,7 +97,7 @@ | |||
|
|
|||
| {:name :community-requests-to-join | |||
| :options {:sheet? true} | |||
| :component join-menu/request-to-join} | |||
| :component join-menu/view} | |||
J-Son89
approved these changes
Jan 4, 2024
67% of end-end tests have passedFailed tests (13)Click to expandClass TestActivityMultipleDevicePR:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityMultipleDevicePRTwo:
Expected to fail tests (3)Click to expandClass TestCommunityMultipleDeviceMergedTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Passed tests (32)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestDeepLinksOneDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
|
BalogunofAfrica
force-pushed
the
fix/community-buttons
branch
from
9630fc5 to
4ef8504
Compare
100% of end-end tests have passedPassed tests (13)Click to expandClass TestCommunityMultipleDeviceMergedTwo:
Class TestActivityMultipleDevicePRTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityMultipleDevicePR:
|
79% of end-end tests have passedFailed tests (7)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Expected to fail tests (3)Click to expandClass TestCommunityMultipleDeviceMergedTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Passed tests (38)Click to expandClass TestActivityCenterContactRequestMultipleDevicePR:
Class TestDeepLinksOneDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityMultipleDevicePR:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityMultipleDevicePRTwo:
Class TestCommunityMultipleDeviceMerged:
|
Francesca-G
approved these changes
Jan 5, 2024
There was a problem hiding this comment.
It looks good compared to the feedback given in the 1.25 build.
I think the screen was updated in the past few months, please have a look here :)
https://www.figma.com/file/h9wo4GipgZURbqqr1vShFN/Communities-for-Mobile?type=design&node-id=15331%3A285065&mode=design&t=BQtnGwV6YnF1RMeW-1
You are right, I confirmed that there are new screens for the request to join and it's currently in the works. I'd have to close this PR. cc: @ajayesivan |
ilmotta
approved these changes
Jan 5, 2024
There was a problem hiding this comment.
LGTM @BalogunofAfrica
Indeed, we are working on new screens behind a feature flag for requesting to join communities. But given the PR is already approved by @Francesca-G and the code looks good, I think it's fine to merge and not close this PR. We don't know exactly when we will enable the toggle community-accounts-selection-enabled? in develop, it might still take a few weeks.
BalogunofAfrica
force-pushed
the
fix/community-buttons
branch
from
5f69767 to
c7e67f6
Compare
That makes sense |
BalogunofAfrica
force-pushed
the
fix/community-buttons
branch
from
c7e67f6 to
c796bed
Compare
BalogunofAfrica
force-pushed
the
fix/community-buttons
branch
from
c796bed to
9f61492
Compare
yevh-berdnyk
pushed a commit
that referenced
this pull request
Jan 5, 2024
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 https://github.com/orgs/status-im/projects/90/views/1?groupedBy%5BcolumnId%5D=&filterQuery=is:issue+is:open&pane=issue&itemId=45784928
Areas that maybe impacted
Functional
Before and after screenshots comparison
Before:
After:
Figma link:
https://www.figma.com/file/Tf5nfkYvpbnNCo4rKLK7lS/Feedback-for-Mobile?type=design&node-id=6117-85237&mode=design&t=VrDYIFW0pOuVnDbb-0
status: ready