Skip to content

Conversation

@nsamuelreddy
Copy link

This PR updates the channel list to display an archive SVG icon for channels marked as archived.

Changes:

  • Shows the archive icon for archived channels and the normal icon for others
  • Registers the SVG asset in pubspec.yaml and places it in assets/icons/archive.svg
  • Adds a widget test to verify the archive icon appears for archived channels and a normal icon for non-archived channels

Testing:

  • Ran the new widget test to confirm correct icon display

Fixes #1992

@gnprice
Copy link
Member

gnprice commented Dec 13, 2025

This PR makes a large number of extraneous changes. Please see our contributor guidelines, and revise this as best you can to be a highly reviewable PR with clear and coherent commits.

@nsamuelreddy nsamuelreddy force-pushed the archived-channel-icon branch 5 times, most recently from dd9bb6a to 39fa711 Compare December 13, 2025 17:35
@nsamuelreddy
Copy link
Author

Thanks for pointing this out. I see that the PR currently contains unrelated and generated changes. I’ll reset the branch, reapply only the archived-channel icon change, and split it into clean, reviewable commits following the contributor guidelines. I’ll post an update once done.

@nsamuelreddy
Copy link
Author

Thanks! I’ve cleaned up the branch, removed the unrelated changes, and kept the PR focused on the archived-channel icon and its test. All checks are passing and the branch is now up to date.
Please let me know if any further adjustments are needed.

Copy link
Collaborator

@chrisbobbe chrisbobbe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Please add the icon in a separate commit, saying where it came from. For similar commits which you can use as examples, please browse the commit history (instructions here), and be sure to format both commit messages according to the project style: https://github.com/zulip/zulip-mobile/blob/master/docs/style.md#git-commits-commit-messages

size: 20,
color: colorSwatchFor(context, subscription).iconOnPlainBackground,
iconDataForStream(channel)),
channel.isArchived ? ZulipIcons.archive : iconDataForStream(channel)),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's give this responsibility to the iconDataForStream function itself.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I’ll move the icon addition into a separate commit and include its source, and update the commit messages to follow the project style.

@nsamuelreddy
Copy link
Author

Thanks for the feedback! I've updated the PR:

  • Split into 2 commits: one for adding the icon (with attribution), one for using it
  • Moved the logic to iconDataForStream() function as suggested

Please let me know if there's anything else to adjust!

@chrisbobbe
Copy link
Collaborator

Thank you! We now have a custom icon to use; please copy-paste the SVG from here and update the PR: #mobile-design > archived-channel icon @ 💬

@nsamuelreddy
Copy link
Author

Thanks , I will replace it and update pr

@nsamuelreddy
Copy link
Author

Thanks, I've updated the archive icon with Karl's custom SVG from the mobile-design discussion and regenerated the icon font. The PR is now updated with the custom Zulip design.

Copy link
Collaborator

@chrisbobbe chrisbobbe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Comments below. Please also post "before" and "after" screenshots of the change, as required; this is mentioned in our contributing guides.

// BEGIN GENERATED ICON DATA

/// The Zulip custom icon "archive".
static const IconData archive = IconData(0xf101, fontFamily: "Zulip Icons");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How did you create the link to the design discussion, in the commit message? The channel-ID part is wrong, so it doesn't take me to the intended message.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the commit message, please link to the specific message that contains the SVG. Currently, it links to a message much earlier in the conversation.

// see this message and the one after it:
// https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/design.3A.20.23F117.20.22Inbox.22.20screen/near/1680637
return switch(stream) {
ZulipStream(isArchived: true) => ZulipIcons.archive,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't start showing the icon in the same commit that adds the asset to the project. I mentioned this in my last review: #2037 (review)

Comment on lines 319 to 325
final archivedIcon = tester.widgetList<Icon>(find.byType(Icon))
.firstWhere((icon) => icon.icon == ZulipIcons.archive);
check(archivedIcon).isNotNull();

final activeIcons = tester.widgetList<Icon>(find.byType(Icon))
.where((icon) => icon.icon != ZulipIcons.archive);
check(activeIcons).isNotEmpty();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use check(…).findsOne() for both of these; see other tests for examples.

@nsamuelreddy nsamuelreddy force-pushed the archived-channel-icon branch 3 times, most recently from 812ece9 to 32cccf5 Compare December 20, 2025 16:20
@nsamuelreddy
Copy link
Author

Thanks for the review! I've fixed all the code issues:

Fixed the channel link (530-mobile-design)
Split commits properly - first adds icon asset, second uses it
Updated test to use check(...).findsOne() pattern
Fixed formatting for CI
For screenshots - I'm having trouble getting archived channels to sync to the app. I archived a test channel on the web, but it's not appearing in the mobile app even after restarting. The feature works correctly in the automated tests though. Any suggestions on how to get an archived channel to show up for screenshots?

@chrisbobbe
Copy link
Collaborator

For screenshots - I'm having trouble getting archived channels to sync to the app. I archived a test channel on the web, but it's not appearing in the mobile app even after restarting. The feature works correctly in the automated tests though. Any suggestions on how to get an archived channel to show up for screenshots?

One option is to make temporary code changes, to pretend that a non-archived channel is actually archived.

// BEGIN GENERATED ICON DATA

/// The Zulip custom icon "archive".
static const IconData archive = IconData(0xf101, fontFamily: "Zulip Icons");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the commit message, please link to the specific message that contains the SVG. Currently, it links to a message much earlier in the conversation.

@nsamuelreddy
Copy link
Author

nsamuelreddy commented Dec 25, 2025

Fixed the commit message to link to Karl's specific message containing the SVG code (near/2331344).

Screenshots:

Before (current behavior - normal channel icons):
[BEFORE screenshot]
Screenshot_2025-12-25-18-34-20-46_e2eb6dad386ea79a8c34fb979200da81.jpg

After (with this PR - archive icon on channels):
[AFTER screenshot]
Screenshot_2025-12-25-18-00-23-33_e2eb6dad386ea79a8c34fb979200da81.jpg

The archive icon now displays correctly for archived channels.

@chrisbobbe
Copy link
Collaborator

Thanks. Could you please also post before-and-after screenshots of how this looks in the channel action sheet?

Those screenshots take a lot of vertical space when I load the page in the GitHub web app. A GitHub markdown table is a good format to post screenshots in GitHub comments. For an example, see #1999 (comment) .

@nsamuelreddy
Copy link
Author

Sure, I’ll add before-and-after screenshots of the channel action sheet and format them in a GitHub Markdown table to save vertical space, like in #1999

@nsamuelreddy
Copy link
Author

nsamuelreddy commented Jan 2, 2026

Screenshots (Channel action sheet)

Before After

@chrisbobbe
Copy link
Collaborator

Those screenshots don't show the channel action sheet.

@nsamuelreddy
Copy link
Author

Screenshots (Channel action sheet)

Before After

@chrisbobbe
Copy link
Collaborator

The icon looks very slightly too high in the channel action sheet, to me; can you nudge it down just a little bit?

@nsamuelreddy
Copy link
Author

nsamuelreddy commented Jan 7, 2026

Sure — I’ll nudge the archive icon down slightly in the channel action sheet and update the alignment.

@nsamuelreddy nsamuelreddy force-pushed the archived-channel-icon branch 2 times, most recently from cb40efc to 338c1d9 Compare January 7, 2026 04:00
@nsamuelreddy
Copy link
Author

Screenshots (Channel Action Sheet)

Before After

@nsamuelreddy
Copy link
Author

nsamuelreddy commented Jan 13, 2026

Hi chrisbobbe,
Just following up on my PR regarding the icon size change. I’ve been waiting for a few days and wanted to check if it can be merged or if any changes are needed.
Thanks!

@gnprice gnprice added the maintainer review PR ready for review by Zulip maintainers label Jan 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintainer review PR ready for review by Zulip maintainers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use new channel icon for archived channels

3 participants