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.

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

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

Replace the way a search in progress learns it is stale: instead of `shouldStop` comparing the query to the one the search started with, and checking `hasListeners` to notice disposal, count searches with a generation number, as MessageListView does for fetches. A search captures the generation when it starts and stops when it finds the number has moved on; `_startSearch` bumps it, and so does `dispose`. The query comparison could not tell apart two searches for an equal query, which the widgets code does start: it sets `query` on each change to the text field, including changes that leave the query the same, like backspace on iOS transiently expanding the selection (see ComposeContentAutocomplete.autocompleteIntent). Then both searches ran to completion and notified twice. Worse, when a search re-runs over changed data with the query unchanged, as topic-link autocomplete (zulip#2137) and topic-list filtering (zulip#2326) will do, the stale search could finish last and clobber fresh results. A generation tells the two searches apart. Bumping the generation in `dispose` replaces the `hasListeners` check, and `_startSearch` checks it after `computeResults` returns, so an implementation that returns without ever awaiting `shouldStop` cannot notify a disposed view-model. Disposal also has to block a search from starting afterward: TopicAutocompleteView starts one when its topics fetch completes, which can be after disposal, and the `hasListeners` check used to stop that search at its first `shouldStop`.

Replace the way a search in progress learns it is stale: instead of `shouldStop` comparing the query to the one the search started with, and checking `hasListeners` to notice disposal, count searches with a generation number. (This is the same pattern as `MessageListView.generation`, which invalidates stale fetches.) A search captures the generation when it starts and stops when it finds the number has moved on; `_startSearch` bumps it, and so does `dispose`. The query comparison could not tell apart two searches for an equal query, which the widgets code does start: it sets `query` on each change to the text field, including changes that leave the query the same, like backspace on iOS transiently expanding the selection (see ComposeContentAutocomplete.autocompleteIntent). Then both searches ran to completion and notified twice. Worse, when a search re-runs over changed data with the query unchanged, as topic-link autocomplete (zulip#2137) and topic-list filtering (zulip#2326) will do, the stale search could finish last and clobber fresh results. A generation tells the two searches apart. Bumping the generation in `dispose` replaces the `hasListeners` check, and `_startSearch` checks it after `computeResults` returns, so an implementation that returns without ever awaiting `shouldStop` cannot notify a disposed view-model. Disposal also has to block a search from starting afterward: TopicAutocompleteView starts one when its topics fetch completes, which can be after disposal, and the `hasListeners` check used to stop that search at its first `shouldStop`. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Replace the way a search in progress learns it is stale: instead of `shouldStop` comparing the query to the one the search started with, count searches with a generation number. (This is the same pattern as `MessageListView.generation`, which invalidates stale fetches.) A search captures the generation when it starts and stops when it finds the number has moved on, and `_startSearch` checks it again after `computeResults` returns. The query comparison could not tell apart two searches for an equal query, which the widgets code does start: it sets `query` on each change to the text field, including changes that leave the query the same. In the emoji picker and the compose box's topic input, whose query is the whole text, that is any cursor move. Then both searches ran to completion and notified twice. Worse, when a search re-runs over changed data with the query unchanged, as topic-link autocomplete (zulip#2137) and topic-list filtering (zulip#2326) will do, the stale search could finish last and clobber fresh results. A generation tells the two searches apart. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Before this commit, `shouldStop` detected a stale search by checking whether the query had changed. That cannot tell apart two searches for an equal query, which the widgets code does start: it sets `query` on each change to the text field, including changes that leave the query the same. Both searches then ran to completion and notified twice, and once searches re-run over changed data (zulip#2137, zulip#2326), the stale one could finish last and clobber fresh results. So count searches with a generation number instead, like `MessageListView.generation`: capture it when a search starts, and stop when it has moved on. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Before this commit, `shouldStop` detected a stale search by checking whether the query had changed. That cannot tell apart two searches for an equal query, which the widgets code does start: it sets `query` on each change to the text field, including changes that leave the query the same. (For example, any cursor movement in the compose box's topic input.) Both searches would then run to completion and notify twice; the fresh results win only because the newer search happens to be scheduled last. Soon, changes to the data being searched will also start a same-query search (zulip#2137, zulip#2326: e.g. on any change to `Topics`), and we don't want the results to depend on which of two searches over different data finishes first. So count searches with a generation number instead: capture it when a search starts, and stop when it has moved on. (This follows the pattern of `MessageListView.generation` for invalidating stale fetches.) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Before this commit, `shouldStop` detected a stale search by checking whether the query had changed. That cannot tell apart two searches for an equal query, which the widget code does start: it sets `query` on each change to the text field, including changes that leave the query the same. (For example, any cursor movement in the compose box's topic input.) Both searches then run to completion and notify twice. That's harmless today beyond the wasted work, since both see the same data. But soon, changes to the data being searched will also start a same-query search (zulip#2137, zulip#2326: e.g. on any change to `Topics`), and then we don't want the results to depend on which of two searches over different data finishes first. So count searches with a generation number instead: capture it when a search starts, and stop when it has moved on. (This follows the pattern of `MessageListView.generation` for invalidating stale fetches.) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

sm-sayedi · 2026-09-18T14:27:37Z

Pushed a new revision with the following changes:

Tightened up the messages for the following commits:
- c72888f autocomplete: In TopicAutocomplete, fetch topics in computeResults
- 094b8e5 autocomplete: In TopicAutocomplete, redo search when topics change
Added a new commit: af491b2 autocomplete test [nfc]: Move TopicAutocompleteView tests into a group
Added a test case in 094b8e5 autocomplete: In TopicAutocomplete, redo search when topics change

sm-sayedi · 2026-09-18T14:59:09Z

(The CI failure seems to be unrelated to the PR; see discussion: #mobile-team > Multiple CI failures @ 💬.)

Before this commit, `shouldStop` detected a stale search by checking whether the query had changed. That cannot tell apart two searches for an equal query, which the widget code does start: it sets `query` on each change to the text field, including changes that leave the query the same. (For example, any cursor movement in the compose box's topic input.) Both searches then run to completion and notify twice. That's harmless today beyond the wasted work, since both see the same data. But soon, changes to the data being searched will also start a same-query search (zulip#2137, zulip#2326: e.g. on any change to `Topics`), and then we don't want the results to depend on which of two searches over different data finishes first. So count searches with a generation number instead: capture it when a search starts, and stop when it has moved on. (This follows the pattern of `MessageListView.generation` for invalidating stale fetches.) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

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 what Greg suggested 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.

Soon, we'll add more tests into this new group.

Move the topic fetch from TopicAutocompleteView.init to computeResults. This simplifies the implementation: computeResults just awaits Topics.getChannelTopics, which caches its result, instead of running a separate fetch and restarting the search when it completes. This also gives us a place to handle a failed fetch, which was a TODO before. On failure, computeResults returns null, so the results are left unchanged and listeners aren't notified; the user can then re-trigger the fetch by changing the query. The main motivation, though, is to prepare for the next commit, which will register _startSearch as a listener on the Topics model so that the results update when the topics change.

Previously the results only reflected the topics as of when the autocomplete was opened. Now they update as topics change, for example when a message arrives in a new topic, or a topic is moved. This will be needed in upcoming commits, where we reuse the TopicAutocomplete machinery for 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 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 force-pushed the 1384-topic-filtering branch from 400fe2e to b7c5085 Compare August 4, 2026 21:13

sm-sayedi force-pushed the 1384-topic-filtering branch 2 times, most recently from c8995b2 to 5903be4 Compare September 18, 2026 14:18

sm-sayedi force-pushed the 1384-topic-filtering branch from 5903be4 to 64592e6 Compare September 22, 2026 04:16

sm-sayedi force-pushed the 1384-topic-filtering branch from 64592e6 to 129fbbe Compare October 27, 2026 18:11

sm-sayedi added 6 commits October 28, 2026 23:11

autocomplete [nfc]: Change TopicAutocompleteResult.topic type

12ea1d3

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 test [nfc]: Move TopicAutocompleteView tests into a group

93b69b2

Soon, we'll add more tests into this new group.

topic_list: Enable topic filtering

07e740c

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 129fbbe to 07e740c Compare October 28, 2026 18:42

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

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

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!

sm-sayedi commented Jul 28, 2026

Uh oh!

sm-sayedi commented Sep 18, 2026

Uh oh!

sm-sayedi commented Sep 18, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sm-sayedi commented May 24, 2026 •

edited

Loading