topic_list: Enable topic filtering by sm-sayedi · Pull Request #2326 · zulip/zulip-flutter

sm-sayedi · 2026-05-24T13:10:03Z

Figma design: https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=6821-37083&t=IUNKC8IkYmIvIb5d-0

Screenshots

Before	After	After (Dark)

Focused	Text entered	Text entered (Dark)

Empty filtered topics	Empty filtered topics (Dark)

Screen recording

Screen.Recording.2026-05-24.at.5.08.12.PM.mov

rajveermalviya

Thanks @sm-sayedi! Comments below, including one about the query implementation, haven't looked at the tests yet.

rajveermalviya · 2026-05-25T14:55:15Z

nit: commit message

icons: Edit `search` icon to remove the additional space around it The icon was edited in Inkspace and cleaned up using the following guide: https://zulip.readthedocs.io/en/latest/subsystems/icons.html#preparing-icons-for-use-with-zulip

s/Inkspace/Inkscape

rajveermalviya · 2026-05-25T15:06:19Z

@@ -161,6 +167,9 @@ class _TopicListState extends State<_TopicList> with PerAccountStoreAwareStateMi

  @override
  Widget build(BuildContext context) {
+    final store = PerAccountStoreWidget.of(context);
+    final zulipLocalization = ZulipLocalizations.of(context);


nit: zulipLocalizations (not zulipLocalization).
Also remove another instance of final zulipLocalizations = ZulipLocalizations.of(context); below.

rajveermalviya · 2026-05-25T15:07:08Z

+  @override
+  Widget build(BuildContext context) {
+    final designVariables = DesignVariables.of(context);
+    final zulipLocalization = ZulipLocalizations.of(context);


nit: zulipLocalizations (not zulipLocalization).

rajveermalviya · 2026-05-25T15:26:44Z

+    final filteredTopics = _filterQuery == null
+      ? channelTopics
+      : channelTopics.where((e) =>
+          (e.name.displayName ?? store.realmEmptyTopicDisplayName).toLowerCase()
+            .contains(_filterQuery!.toLowerCase()));


I think we want this search to behave in the same way the current topic autocomplete behaves. See #1384 (comment).

So we should probably use relevant bits from lib/model/autocomplete.dart for topic autocomplete, just like emoji picker search.

sm-sayedi · 2026-05-27T15:53:43Z

Thanks @rajveermalviya for the review. Changes pushed, PTAL.

rajveermalviya

Thanks for the revision @sm-sayedi! All LGTM, some small comments below.

After addressing them please mark it for the integration review.

rajveermalviya · 2026-06-10T13:14:32Z

  void _handleViewModelUpdate() {
    setState(() {
-      _resultsToDisplay = List.unmodifiable(_viewModel!.results);
+      _resultsToDisplay = List.unmodifiable(_viewModel?.results ?? []);


nit: _handleViewModelUpdate will always have non-null _viewModel, right?

Suggested change

_resultsToDisplay = List.unmodifiable(_viewModel?.results ?? []);

_resultsToDisplay = List.unmodifiable(_viewModel!.results ?? []);

rajveermalviya · 2026-06-10T13:18:09Z


 class _TopicListState extends State<_TopicList> with PerAccountStoreAwareStateMixin {
-  Topics? topicsModel;
+  final TextEditingController _controller = TextEditingController();


nit: let's move this instantiation inside initState, and make this a late declaration.

Suggested change

final TextEditingController _controller = TextEditingController();

late TextEditingController _controller;

rajveermalviya · 2026-06-10T13:32:02Z

+    _controller.dispose();
+    _topicAutocompleteModel?.dispose();
+    unreadsModel?.removeListener(_unreadsModelChanged);


The order of these should be reverse of their initialization.

sm-sayedi · 2026-06-10T20:18:22Z

Thanks Rajesh for the review. Changes pushed, marking for Chris's review.

chrisbobbe · 2026-07-15T23:39:45Z

Looks like this has gathered some conflicts; would you rebase please? I hope to get to this soon.

sm-sayedi · 2026-07-16T07:08:27Z

(Conflicts resolved.)

chrisbobbe

Thanks! Comments below.

chrisbobbe · 2026-07-23T20:44:50Z

-            child: Icon(size: 24, ZulipIcons.search)),
+            child: SizedBox.square(dimension: 24,
+              child: Icon(size: 20, ZulipIcons.search))),


icons: Edit `search` icon to remove the additional space around it

This change looks like it adds, or restores, some amount of additional space around the icon? What's the actual intent of this commit? If this change is part of that intent, why isn't the ZulipIcons.search in lib/widgets/home.dart also touched?

Thanks for the catch. The search icon in other places should've been changed too; done in the new revision. The only exception is the search icon in the main-menu sheet. The Figma design seems to be inconsistent with the icon sizes of the menu buttons, ranging from 20px to 24px. We use 24px for all of them.

The actual intent of the commit is to have the search icon without any surrounding space, so we can exactly match the Figma design where the icon is used. Discovered the need for this while implementing the search field in the topic list page.

The Figma design seems to be inconsistent with the icon sizes of the menu buttons, ranging from 20px to 24px.

Worth being precise about this, I think: I don't actually see an inconsistency here. I'm seeing consistent 24px squares for all the menu buttons, at the icons/24/foo level:

There's a nested node on all of them that's less than 24px, e.g. 22px with the "clock" icon:

where the square box wraps the icon's glyph tightly, leaving no surrounding whitespace. But the icon asset we actually export from the Figma—linked by the "go to main component" button—is the 24px version with the surrounding whitespace:

That looks to be the case with the "search" icon too:

So I don't think there's a change to be made here, and we should just keep the asset unchanged, without adding padding in the widget tree to compensate anywhere. Am I missing something, though?

The inconsistency that I was referring to was about the nested node inside the 24px square box. But now that you mentioned that we export the main icon component that has internal padding, we can drop the commit.

chrisbobbe · 2026-07-23T20:48:43Z

-    }
-    return topic.displayName != raw
-      && AutocompleteQuery.lowercaseAndStripDiacritics(topic.displayName!).contains(_normalized);
+    return AutocompleteQuery.lowercaseAndStripDiacritics(


autocomplete: Include the result when the query matches the topic

Commit-message nit: let's mention "topic autocomplete" in the summary line. Maybe:

autocomplete: In topic autocomplete, include exact matches

chrisbobbe · 2026-07-23T20:51:30Z

  final int channelId;

-  Iterable<TopicName> _topics = [];
+  Iterable<GetChannelTopicsEntry> _topics = [];


autocomplete: Change TopicAutocompleteResult.topic type

Can this be marked NFC, or is there some behavior change I'm missing?

Yep, that's NFC. Thanks for bringing it up.

chrisbobbe · 2026-07-23T21:10:20Z

    required this.channelId,
-  });
+  }) {
+    store.topics.addListener(_fetch);


autocomplete: Search again when there is a change in topics

This code is more indirect than it needs to be: a topics change triggers _fetch, which doesn't actually fetch anything (the data is already present, right), and then that calls _startSearch. This would read more transparently if it were store.topics.addListener(_startSearch).

How about:

In a prep commit, remove the mutable _topics field and the .._fetch() in TopicAutocompleteView.init. Instead, put final topics = await store.topics.getChannelTopics(channelId) at the top of computeResults.

Then this line in this commit can change to store.topics.addListener(_startSearch).

?

That's a nice improvement, thanks.

chrisbobbe · 2026-07-23T21:18:54Z

+        ],
+      ),
+    );


nit: coalesce closing parens

sm-sayedi · 2026-07-24T17:11:11Z

Thanks for the review, Chris. Revision pushed.

Also, dropped the commit autocomplete: Make AutocompleteView.results nullable as it wasn't needed anymore after the new commit autocomplete [nfc]: In TopicAutocomplete, fetch topics in computeResults.

chrisbobbe

Thanks! Small comment below with a finding from reading Git history, and see my reply above: #2326 (comment)

chrisbobbe · 2026-07-24T20:48:14Z

-      return AutocompleteQuery.lowercaseAndStripDiacritics(store.realmEmptyTopicDisplayName)
-        .contains(_normalized);
-    }
-    return topic.displayName != raw


autocomplete: In topic autocomplete, include exact matches

Interesting: in code review, it looks like Greg requested the != raw condition be dropped, for the same reason you give—we want to match web—but it seems that request wasn't acted on, and it slipped through and got merged. So you could additionally link to that review in the commit message, to support the change: #627 (comment)

sm-sayedi · 2026-07-25T04:30:35Z

Thanks for the review. Changes pushed with that icon commit dropped!

chrisbobbe

Thanks! Here's a partial review before I have to run to dinner; more later.

I noticed something this time in manual testing: this has a bug like #1175, which I think should be fixed the same way: when you change your query, it should jump back to the top (the "zero" scroll position) so you can see the most relevant results.

chrisbobbe · 2026-07-28T01:14:56Z

  Future<List<TopicAutocompleteResult>?> computeResults() async {
+    // TODO: handle fetch failure
+    final topics = await store.topics.getChannelTopics(channelId);


autocomplete [nfc]: In TopicAutocomplete, fetch topics in computeResults

This isn't NFC, because now we're letting the user re-trigger the fetch by changing the query, if it failed the first time.

sm-sayedi · 2026-07-28T11:48:24Z

Thanks for those points. New changes pushed. Also addressed some feedback from a review Claude did.

Previously, in topic autocomplete, the query "t" would not include a topic named "t" (an exact match). I think this is not desired and it's not what the web does. This also matches a suggestion Greg made in an earlier review of this part of the code: zulip#627 (comment)

Change it from TopicName to GetChannelTopicsEntry. This allows future commits to access maxId when the TopicAutocomplete machinery is used for topic filtering on the topic-list page.

Move the topic fetch from TopicAutocompleteView.init to computeResults. This simplifies the implementation by awaiting the cached Topics.getChannelTopics call directly, instead of restarting the search after the fetch completes. Another benefit is that user can re-trigger the topic-list fetch by changing the query; if initially failed. The main motivation for this change is that it will enable us to cleanly search again when the topics change.

This enables the UI to reflect changes to the topics. This will be needed in the next commit(s), where we use the TopicAutocomplete machinery for topic filtering on the topic-list page.

Fixes zulip#1384. Figma design: https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=6821-37083&t=IUNKC8IkYmIvIb5d-0

sm-sayedi added the maintainer review PR ready for review by Zulip maintainers label May 24, 2026

sm-sayedi requested a review from chrisbobbe May 24, 2026 13:10

sm-sayedi assigned chrisbobbe and rajveermalviya and unassigned chrisbobbe May 24, 2026

sm-sayedi requested review from rajveermalviya and removed request for chrisbobbe May 24, 2026 13:11

rajveermalviya requested changes May 25, 2026

View reviewed changes

sm-sayedi force-pushed the 1384-topic-filtering branch 2 times, most recently from c385b09 to 3ce261b Compare May 27, 2026 15:53

sm-sayedi requested a review from rajveermalviya May 27, 2026 15:53

rajveermalviya approved these changes Jun 10, 2026

View reviewed changes

sm-sayedi force-pushed the 1384-topic-filtering branch from 3ce261b to 85d085a Compare June 10, 2026 20:17

sm-sayedi unassigned rajveermalviya Jun 10, 2026

sm-sayedi added integration review Added by maintainers when PR may be ready for integration and removed maintainer review PR ready for review by Zulip maintainers labels Jun 10, 2026

sm-sayedi requested a review from chrisbobbe June 10, 2026 20:19

sm-sayedi assigned chrisbobbe Jun 10, 2026

sm-sayedi force-pushed the 1384-topic-filtering branch from 85d085a to b85fd69 Compare June 10, 2026 20:24

sm-sayedi force-pushed the 1384-topic-filtering branch from b85fd69 to 82e2889 Compare July 16, 2026 07:06

sm-sayedi force-pushed the 1384-topic-filtering branch from 82e2889 to 3ebabb0 Compare July 18, 2026 04:21

chrisbobbe reviewed Jul 23, 2026

View reviewed changes

sm-sayedi force-pushed the 1384-topic-filtering branch 3 times, most recently from 7602c9e to f467175 Compare July 24, 2026 17:09

sm-sayedi requested a review from chrisbobbe July 24, 2026 17:11

chrisbobbe reviewed Jul 24, 2026

View reviewed changes

sm-sayedi force-pushed the 1384-topic-filtering branch from f467175 to d21a92c Compare July 25, 2026 04:22

sm-sayedi requested a review from chrisbobbe July 25, 2026 04:30

sm-sayedi force-pushed the 1384-topic-filtering branch from d21a92c to 35a5c57 Compare July 25, 2026 04:40

chrisbobbe reviewed Jul 28, 2026

View reviewed changes

sm-sayedi force-pushed the 1384-topic-filtering branch 4 times, most recently from 0c2ae40 to 400fe2e Compare July 28, 2026 11:46

sm-sayedi requested a review from chrisbobbe July 28, 2026 11:48

sm-sayedi added 5 commits August 5, 2026 01:41

autocomplete [nfc]: Change TopicAutocompleteResult.topic type

37b2433

Change it from TopicName to GetChannelTopicsEntry. This allows future commits to access maxId when the TopicAutocomplete machinery is used for topic filtering on the topic-list page.

autocomplete: Search again when there is a change in topics

14d65e2

This enables the UI to reflect changes to the topics. This will be needed in the next commit(s), where we use the TopicAutocomplete machinery for topic filtering on the topic-list page.

topic_list: Enable topic filtering

b7c5085

Fixes zulip#1384. Figma design: https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=6821-37083&t=IUNKC8IkYmIvIb5d-0

sm-sayedi force-pushed the 1384-topic-filtering branch from 400fe2e to b7c5085 Compare August 4, 2026 21:13

	_resultsToDisplay = List.unmodifiable(_viewModel?.results ?? []);
	_resultsToDisplay = List.unmodifiable(_viewModel!.results ?? []);

	final TextEditingController _controller = TextEditingController();
	late TextEditingController _controller;

Uh oh!

Conversation

sm-sayedi commented May 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rajveermalviya left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sm-sayedi commented May 27, 2026

Uh oh!

rajveermalviya left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sm-sayedi commented Jun 10, 2026

Uh oh!

chrisbobbe commented Jul 15, 2026

Uh oh!

sm-sayedi commented Jul 16, 2026

Uh oh!

chrisbobbe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sm-sayedi commented Jul 24, 2026

Uh oh!

chrisbobbe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sm-sayedi commented Jul 25, 2026

Uh oh!

chrisbobbe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sm-sayedi commented Jul 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

sm-sayedi commented May 24, 2026 •

edited

Loading