msglist: Colorize channel icon in the app bar, following Figma #1524
Conversation
|
This will avoid an inconsistency with the topic-list page (PR #1500), which colorizes the icon. |
|
| iconColor = colorSwatchFor(context, store.subscriptions[stream.streamId]) | ||
| .iconOnBarBackground; |
There was a problem hiding this comment.
Should we fall back to the default base color kDefaultChannelColorSwatchBaseColor (by passing a possibly null subscription) until we get to fully implementing #188?
There was a problem hiding this comment.
I think that's what this code already does; am I reading too quickly? store.subscriptions[stream.streamId] might be null, and if it is, colorSwatchFor uses a swatch based on kDefaultChannelColorSwatchBaseColor.
There was a problem hiding this comment.
Oh right. I think I meant the case when stream is null, i.e. we got a "(unknown channel)".
| } | ||
| testChannelIconInChannelRow(ZulipIcons.globe, isWebPublic: true, inviteOnly: false); | ||
| testChannelIconInChannelRow(ZulipIcons.lock, isWebPublic: false, inviteOnly: true); | ||
| testChannelIconInChannelRow(ZulipIcons.hash_sign, isWebPublic: false, inviteOnly: false); |
There was a problem hiding this comment.
Thanks! It's nice to have this tested.
chrisbobbe
force-pushed
the
pr-colorize-msglist-app-bar-channel-icon
branch
from
e745ebf to
d94b011
Compare
|
Updated to fix the analyzer warning; PTAL at my reply above: #1524 (comment) |
chrisbobbe
added
integration review
The Figma: https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=6089-28394&m=dev And while we're adding a test for it, also check that the chosen channel icon is the intended one.
gnprice
force-pushed
the
pr-colorize-msglist-app-bar-channel-icon
branch
from
d94b011 to
245d9d9
Compare
|
Thanks — looks good, merging. |
Following the Figma:
https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=6089-28394&m=dev
And while we're adding a test for it, also check that the chosen channel icon is the intended one.