Better management of archived channel names #1344

chimnayajith · 2025-02-11T16:06:19Z

Pull Request

Description

This PR improves archived stream support in the mobile app to match the web app’s behavior. Key updates include:

Appending " (archived)" to stream names when viewing messages.
Updating UI elements (e.g., stream headers) to clearly indicate archived streams.
Filtering out archived channels from relevant pages.
Displaying the archived tag in a distinct font and color, consistent with the web app.
Replacing the compose box with a "no permission" error banner instead of offering message composition.

Related Issues

Closes Support better management of names of archived channels #800

Screenshots

PIG208

Thanks for working on this! One more thing that I think needs to be handled is the topic actions sheet. None of the supported actions (follow/mute/unmute/resolve/unresolve) works on a topic in an archived channel. Not sure what's the best way to deal with this, though. It would be a good question to ask on #mobile-design.

PIG208 · 2025-02-15T00:22:49Z

lib/widgets/compose_box.dart

        if (channel == null || !store.hasPostingPermission(inChannel: channel,
-            user: selfUser, byDate: DateTime.now())) {
+            user: selfUser, byDate: DateTime.now()) || channel.isArchived) {


nit: let's reformat this to show that these expressions are parallel:

Suggested change

if (channel == null || !store.hasPostingPermission(inChannel: channel,

user: selfUser, byDate: DateTime.now())) {

user: selfUser, byDate: DateTime.now()) || channel.isArchived) {

if (channel == null

|| !store.hasPostingPermission(inChannel: channel,

user: selfUser, byDate: DateTime.now())

|| channel.isArchived) {

PIG208 · 2025-02-15T00:24:07Z

test/widgets/compose_box_test.dart

+        testWidgets('error banner is shown in $narrowType narrow', (tester) async {
+          await prepareComposeBox(tester,
+            narrow: narrow,
+            selfUser: eg.user(role: UserRole.administrator),


Is the user role relevant here? If not, this can be removed.

PIG208 · 2025-02-15T00:25:25Z

lib/widgets/subscription_list.dart

-        pinned.add(subscription);
-      } else {
-        unpinned.add(subscription);
+      if(!subscription.isArchived) {


nit: we can use continue here, so that the ifs don't need to be nested

PIG208 · 2025-02-15T00:26:09Z

lib/widgets/subscription_list.dart

@@ -187,10 +189,13 @@ class _SubscriptionList extends StatelessWidget {

  @override
  Widget build(BuildContext context) {
+    // Filtering out the archived subscriptions.
+    final activeSubscriptions = subscriptions.where((sub) => !sub.isArchived).toList();


I think this line of code is pretty straightforward, and the comment doesn't add much here, so let's just leave it out.

Additionally, is it possible for subscriptions to contain archived subscriptions, or is it the job of SubscriptionListPageBody to ensure that no archived subscriptions are passed here?

If the latter, it might be better performance-wise to have an assertion that none of these subscriptions is archived, so that we don't need to re-filter the subscriptions.

Ah now that I check, its already being handled. removed this part.

PIG208 · 2025-02-15T00:27:23Z

test/widgets/subscription_list_test.dart

+      eg.subscription(eg.stream(streamId: 1, isArchived: true), pinToTop: true),
+      eg.subscription(eg.stream(streamId: 2), pinToTop: true),
+      eg.subscription(eg.stream(streamId: 3, isArchived: true), pinToTop: false),
+      eg.subscription(eg.stream(streamId: 4,), pinToTop: false),


nit: extra comma

Let's also reorder the list to make it easier to read:

await setupStreamListPage(tester, subscriptions: [ eg.subscription(eg.stream(streamId: 1, isArchived: true), pinToTop: true), eg.subscription(eg.stream(streamId: 2, isArchived: true), pinToTop: false), eg.subscription(eg.stream(streamId: 3), pinToTop: true), eg.subscription(eg.stream(streamId: 4), pinToTop: false), ]);

PIG208 · 2025-02-25T19:42:01Z

test/widgets/message_list_test.dart

@@ -1062,7 +1085,7 @@ void main() {
        .initNarrow.equals(DmNarrow.withUser(eg.otherUser.userId, selfUserId: eg.selfUser.userId));
      await tester.pumpAndSettle();
    });
-    
+


This looks unintentional.

PIG208 · 2025-02-25T19:44:04Z

lib/widgets/message_list.dart

+                fontStyle: FontStyle.italic,
+              ),
+            ),
+          ),


If there is a specific design that we are following here (and other UI changes), it would be helpful to have a link in the commit message.

There isn't a particular design that i could reference. I'm trying to keep it consistent with web.

PIG208 · 2025-02-25T19:52:19Z

test/widgets/inbox_test.dart

@@ -595,6 +616,36 @@ void main() {
          check(rectAfterTap).equals(rectBeforeTap);
        });

+        testWidgets('shows archived label for archived streams', (tester) async {
+          final zulipLocalizations = GlobalLocalizations.zulipLocalizations;


For tests, it is fine to not use ZulipLocalizations and have plain strings unless we are specifically testing something related to l10n. This way we have a better idea of what exactly we are expecting in tests.

PIG208 · 2025-02-25T19:56:39Z

test/widgets/subscription_list_test.dart

+    ]);
+
+    // Only non-archived streams
+    check(getItemCount()).equals(2);


This is a bit ambiguous because it doesn't check if the remaining streams are the ones that are not archived.

PIG208 · 2025-02-25T20:01:53Z

lib/api/model/model.dart

@@ -347,6 +347,8 @@ class ZulipStream {
  // TODO(server-8): added in FL 199, was previously only on [Subscription] objects
  int? streamWeeklyTraffic;

+  final bool isArchived;


nit: Make sure to match the order of these fields in the API documentation. See the dartdoc of ZulipStream to find the relevant piece.

this seems to be the order in the API documentation as I checked. Please let me know if its not

https://zulip.com/api/register-queue and go to subscriptions return value

I see. The location the dartdoc points to has a different order. Let's match that:

/// For docs, search for "if stream" /// in <https://zulip.com/api/register-queue>. @JsonSerializable(fieldRename: FieldRename.snake) class ZulipStream {

This enables the client to receive metadata about archived streams in the register response and events, allowing it to handle them properly.

Fixes zulip#800

chimnayajith force-pushed the 800-archived-channels branch 2 times, most recently from 24ed3b6 to 432b103 Compare February 11, 2025 19:37

PIG208 self-assigned this Feb 12, 2025

PIG208 added the maintainer review PR ready for review by Zulip maintainers label Feb 12, 2025

PIG208 self-requested a review February 12, 2025 20:46

PIG208 reviewed Feb 25, 2025

View reviewed changes

chimnayajith force-pushed the 800-archived-channels branch from 432b103 to 2760c82 Compare February 28, 2025 01:17

register: Include archived_channels in register request

c3abe2b

This enables the client to receive metadata about archived streams in the register response and events, allowing it to handle them properly.

chimnayajith force-pushed the 800-archived-channels branch 2 times, most recently from 6a37c0c to 1a8ae0f Compare February 28, 2025 02:30

chimnayajith added 4 commits February 28, 2025 10:26

api: Add isArchived to ZulipStream

40a60db

compose: Replace compose box with error banner for archived channels

c029f84

subscription_list: Exclude archived channels from list

8a0c4da

inbox: Add label for archived channels in headers

1bc6ec1

Fixes zulip#800

chimnayajith force-pushed the 800-archived-channels branch from 1a8ae0f to 1bc6ec1 Compare February 28, 2025 04:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Better management of archived channel names #1344

Better management of archived channel names #1344

chimnayajith commented Feb 11, 2025

PIG208 left a comment

PIG208 Feb 15, 2025

PIG208 Feb 15, 2025

PIG208 Feb 15, 2025

PIG208 Feb 15, 2025

PIG208 Feb 25, 2025

chimnayajith Feb 28, 2025

PIG208 Feb 15, 2025

PIG208 Feb 15, 2025

PIG208 Feb 25, 2025

PIG208 Feb 25, 2025

chimnayajith Feb 28, 2025

PIG208 Feb 25, 2025

PIG208 Feb 25, 2025

PIG208 Feb 25, 2025

chimnayajith Feb 26, 2025

PIG208 Feb 28, 2025

Better management of archived channel names #1344

Are you sure you want to change the base?

Better management of archived channel names #1344

Conversation

chimnayajith commented Feb 11, 2025

Pull Request

Description

Related Issues

Screenshots

PIG208 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment