home: Sort leading emoji first in channel names #1234
Conversation
lakshya1goel
force-pushed
the
issue1202
branch
from
6e0e1f0 to
46ac2f3
Compare
|
Hello, @chrisbobbe I have done the required changes. Please have a look and let me know if anything else is required. |
|
Hi @chrisbobbe @PIG208 |
lakshya1goel
force-pushed
the
issue1202
branch
2 times, most recently
from
6167d68 to
0c567a3
Compare
|
Thanks for the detailed review. I have made the required changes. |
|
Pushed the changes atop #1290 for the sake of CI. |
lakshya1goel
force-pushed
the
issue1202
branch
from
0c567a3 to
79e0820
Compare
|
I have updated the commit message and the description of tests. Let me know if anything else is required. |
|
Thanks! Marking for Greg's review. |
chrisbobbe
added
integration review
There was a problem hiding this comment.
Thanks @lakshya1goel for taking care of this, and @chrisbobbe for the previous reviews!
Generally this looks good. A couple of small comments below.
| @@ -41,10 +41,22 @@ class _SubscriptionListPageBodyState extends State<SubscriptionListPageBody> wit | |||
| }); | |||
| } | |||
|
|
|||
| final _startsWithEmojiRegex = RegExp(r'^\p{Emoji}', unicode: true); | |||
There was a problem hiding this comment.
| final _startsWithEmojiRegex = RegExp(r'^\p{Emoji}', unicode: true); | |
| static final _startsWithEmojiRegex = RegExp(r'^\p{Emoji}', unicode: true); |
That way there's just one of these shared by any number of instances of this class.
| ]); | ||
|
|
||
| check(listedStreamIds(tester)).deepEquals([3, 2, 4, 1]); |
There was a problem hiding this comment.
nit: similar comment to #1193 (comment) (but this one is smaller-scale): compare to the style of neighboring tests and see how you can make this one more compact by following those
| // emoji-prefixed channels first; see #1202. | ||
| // For matching web's ordering completely, see: | ||
| // https://github.com/zulip/zulip-flutter/issues/1165 | ||
| final aStartsWithEmoji = _startsWithEmojiRegex.hasMatch(a.name); |
There was a problem hiding this comment.
commit-message nit:
home: Sort leading emoji first in channel names
The prefix "home:" doesn't seem right for what this commit is doing — it's really about the subscription-list page in particular.
To see previous examples from commits that touched this file, use git log --oneline:
$ git log --oneline --no-decorate lib/widgets/subscription_list.dart
46b30f7 nav: Remove *Page widgets for page bodies that can be reached in HomePage
084cb2f nav [nfc]: Extract some *PageBody widgets
50d39ea app_bar [nfc]: Move ZulipAppBar.isLoading to within widget
a9d9ef8 ui: Extract ZulipAppBar for loading indicator.
f0b3596 subscription_list: Show a dot for unreads if channel is muted
5d80ad5 subscription_list: Ensure bold name applies for unmuted streams only
726ccb6 subscription_list [nfc]: Place provisional dark-theme colors pending design
07131d2 subscription_list: For channel text, use #222222 instead of #262626
84b64fc unreads [nfc]: Rename countInChannel from "stream", to match its twin
8cf3b6a narrow [nfc]: Rename StreamNarrow to ChannelNarrow
b0b8a50 subscription_list: Show muted streams as faded in streams list
b517593 subscription_list: Show muted streams below unmuted ones
fba909b ui: Provide [StreamColorSwatch]es via DesignVariables, not api/model/
db064a0 subscription_list [nfc]: Update a stale issue reference
ef1a876 subscription_list [nfc]: Update a stale issue reference
187ecb0 ui: Rename stream to channel in user-facing strings
d67b1f2 ui [nfc]: Comment with TODO(#95) where we need dark-theme colors
abfaed8 subscription_list [nfc]: Factor out helper for dividing line
74aa25d text: Use proportionalLetterSpacing in the few existing places that want it
51c7b69 subscription_list: Fix stream name sorting case insensitively
582bc44 nav [nfc]: Accept an explicit account ID on account page routes
6784ef9 text [nfc]: Remove some now-redundant fontFamily{,Fallback} attributes
139fc34 text [nfc]: Don't force any callers to specify wghtIfPlatformRequestsBold
51eaddd text: On Android, render emojis in message content with Noto Color Emoji
117670a text [nfc]: Pull out kDefaultFontFamily constant
0d03c8e unread: Apply stream and topic muting to unread counts
93cefca subscription_list: Handle insets with a SafeArea
0618adc subscription_list [nfc]: Remove unneeded [Center]
e4e2e88 subscription_list: Add new SubscriptionListPage
Update the channel sorting logic to ensure streams with leading emojis in their names are listed above those without emojis. The updated sorting respects pinned, muted, and unmuted streams while handling emoji precedence and maintaining alphabetical order for ties.
Fixes: #1202