Communities - Channel list should not be shown for token-gated communities #17901
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 (20)
|
FFFra
force-pushed
the
hide-channels-at-view
branch
from
674241e
to
24b2ced
Compare
FFFra
force-pushed
the
hide-channels-at-view
branch
from
24b2ced
to
e4c8da2
Compare
clauxx
approved these changes
Nov 15, 2023
Comment on lines
277
to
279
| (when (or (and token-permissions joined) | ||
| (not token-permissions)) | ||
| [channel-list-component |
There was a problem hiding this comment.
Indentation looks a bit weird here. I guess a format will fix it.
There was a problem hiding this comment.
tks, I will take a look also a good opportunity to fix again my auto format!
FFFra
force-pushed
the
hide-channels-at-view
branch
2 times, most recently
from
23ba6e3
to
9b9c2ac
Compare
ilmotta
reviewed
Nov 15, 2023
| :community-color color | ||
| :on-first-channel-height-changed on-first-channel-height-changed} | ||
| (add-handlers-to-categorized-chats id chats-by-category joined)]]))) | ||
| (when (or (and token-permissions joined) |
There was a problem hiding this comment.
Even if this code works, since token-permissions is a sequence, it's more robust and idiomatic to use seq to check something is there.
(and (seq token-permissions) joined)See from example below that if a collection is empty, it is truthy. If you check using just (and token-permissions ...) you are checking the value is either not nil or not false, but an empty sequence is truthy.
(or [] :default-value) ; => []We do the same thing here
FFFra
force-pushed
the
hide-channels-at-view
branch
from
9b9c2ac
to
24b382a
Compare
ilmotta
approved these changes
Nov 15, 2023
| :on-first-channel-height-changed on-first-channel-height-changed} | ||
| (add-handlers-to-categorized-chats id chats-by-category joined)]]))) | ||
| (when (or (and (seq token-permissions) joined) | ||
| (not (seq token-permissions))) |
There was a problem hiding this comment.
(not (seq can be replaced by empty? https://github.com/clojure/clojure/blob/clojure-1.11.1/src/clj/clojure/core.clj#L6246
FFFra
force-pushed
the
hide-channels-at-view
branch
from
24b382a
to
f73d07f
Compare
78% of end-end tests have passedFailed tests (7)Click to expandClass TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestGroupChatMultipleDeviceMergedNewUI:
Expected to fail tests (3)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestGroupChatMultipleDeviceMergedNewUI:
Passed tests (35)Click to expandClass TestActivityMultipleDevicePR:
Class TestCommunityMultipleDeviceMerged:
Class TestActivityMultipleDevicePRTwo:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
|
82% of end-end tests have passedFailed tests (5)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityOneDeviceMerged:
Expected to fail tests (3)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestGroupChatMultipleDeviceMergedNewUI:
Passed tests (37)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityMultipleDevicePRTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityMultipleDeviceMerged:
Class TestActivityMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
|
|
Thanks for your amazing work! Devices:
Please, cherry-pick your fix to release branch too, thank you! |
|
Thank you so much, team! |
FFFra
force-pushed
the
hide-channels-at-view
branch
from
f73d07f
to
d2d008a
Compare
FFFra
pushed a commit
that referenced
this pull request
Nov 16, 2023
This commit hides the categorized channel list for token-gated communities overview.
yevh-berdnyk
pushed a commit
that referenced
this pull request
Dec 8, 2023
This commit hides the categorized channel list for token-gated communities overview.
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.
fix: #17836
Figma design:
https://www.figma.com/file/h9wo4GipgZURbqqr1vShFN/Communities-for-Mobile?type=design&node-id=14009-569565&mode=design&t=CYSZhcyPISfU2ds1-0
Demo
Simulator.Screen.Recording.-.iPhone.11.Pro.-.2023-11-14.at.15.04.33.mp4
Summary
This approach only displays the channels if the conditions are satisfied.
Platforms
status: ready