Improved how muted streams are displayed on Streams screen #5522
Conversation
|
Please squash your commits to provide a clean commit history and make sure your commit message follows the guidelines detailed here. Also, some images would be great. 😃 Thanks! |
Syed-Ansar
force-pushed
the
feat/sortMutedStreams
branch
from
9a618e0 to
59087ac
Compare
|
On second thought, I think I'd like to see my pinned and muted streams grouped together below my pinned streams. I'm no product designer though. @alya What do you think? This would no longer exactly mimic the webapp behavior. |
You mean another section below the pinned comments for the pinned and muted comments orelse just below the pinned streams without having a divider. Right. |
|
I'm confused about how the proposal above differs from the webapp UI. |
|
In any case, I don't want to re-summarize the details of the reasoning, but you can search chat.zulip.org for extensive prior discussions of how these sections should be arranged. I think all the reasoning applies equally well to the mobile app. |
| if (isMuted) { | ||
| Component = IconMute; | ||
| } else if (isPrivate) { | ||
|
|
||
| if (isPrivate) { |
There was a problem hiding this comment.
This component StreamIcon is used in several other places besides the Streams screen. So this change affects them too.
It should therefore go in its own commit, with a commit message that describes the changes it's making and why.
That commit can come before the commit that changes the sorting.
| return [ | ||
| { key: 'Pinned', data: sortedSubscriptions.filter(x => x.pin_to_top) }, | ||
| { key: 'Unpinned', data: sortedSubscriptions.filter(x => !x.pin_to_top) }, | ||
| { key: 'Unpinned', data: sortedSubscriptions.filter(x => !x.pin_to_top && x.in_home_view) }, | ||
| { key: 'Muted', data: sortedSubscriptions.filter(x => !x.pin_to_top && !x.in_home_view) }, |
There was a problem hiding this comment.
Let's avoid going through the list of subscriptions three times here. We can do it with just one pass, by using a plain old loop instead of .filter.
That will also let us deduplicate the conditionals on .pin_to_top and .in_home_view.
With the old code it's a bit of a closer call between those advantages of a loop, and the declarative neatness of this .filter version. But when there's three filters instead of two, and the conditions get further duplicated between them like this, it's time to make it a loop.
Let's have one commit that's NFC (https://github.com/zulip/zulip-mobile/blob/main/docs/glossary.md#nfc ) and just changes the existing code to use a loop and make a single pass. Then a subsequent commit can make the substantive change here.
Fixes: #5514
Improved how muted streams are displayed on Streams screen. Resolved the issue #5514
Muted Streams sorted at the bottom of the Stream List and icon of muted Stream is changed
Muted stream and pinned stream is faded.