adds search icon to inbox and combined feed pages #1864

komal-uniqode · 2025-09-20T08:53:29Z

Added a search icon on inbox and the combined feed screen which opens up the usual search page.

Fixes : #1854

Screen.Recording.2025-09-20.at.2.11.43.PM.mov

Screen.Recording.2025-09-20.at.2.17.36.PM.mov

komal-uniqode · 2025-09-20T09:40:58Z

@gnprice , @chrisbobbe

Need your review on this. Do let me know if tests are also required for this.

gnprice · 2025-09-22T19:12:41Z

Thanks! Yeah, let's have a few quick test cases for this: just checking that

on each of these two pages, a button with the search icon appears, and tapping it leads to the search page;
on some pages that could have accidentally gotten a search button if this logic weren't right — so in particular the subscribed-channels page, and another message-list page — there's no search button.

komal-uniqode · 2025-09-22T19:16:34Z

Sure, will add the tests and update the PR.

alya · 2025-09-23T07:12:00Z

Thanks for the detailed screenshots! The placement looks good to me.

komal-uniqode · 2025-09-24T18:52:22Z

Hey, @gnprice , @chrisbobbe . I have added the following tests for the search icon button in this PR :

Positive Cases - Search Button Should Appear
Inbox Tab (home_test.dart)

Search button appears in app bar on inbox tab (default home screen state)
Tapping search button navigates to search page with empty KeywordSearchNarrow
Combined Feed Screen (message_list_test.dart)
Search button appears in app bar on CombinedFeedNarrow
Tapping search button navigates to search page with empty KeywordSearchNarrow

Negative Cases - Search Button Should NOT Appear
Home Screen Other Tabs (home_test.dart)

Search button does not appear on channels tab
Search button does not appear on direct messages tab
Other Message List Screens (message_list_test.dart)
Search button does not appear on ChannelNarrow (single channel view)
Search button does not appear on TopicNarrow (single topic view)
Search button does not appear on DmNarrow (direct message conversation)
Search button does not appear on MentionsNarrow (mentions view)
Search button does not appear on StarredMessagesNarrow (starred messages)
Search button does not appear on KeywordSearchNarrow (search page itself)
Subscribed Channels Page (subscription_list_test.dart)
Search button does not appear on subscribed channels/streams page

komal-uniqode · 2025-09-25T17:29:13Z

Hey Guys, just a sincere remainder. This PR needs a review.

Thanks.

cc : @gnprice , @chrisbobbe

chrisbobbe

Thanks, this looks helpful! Comments below, and please organize the commits (and write their commit messages) according to our project style.

chrisbobbe · 2025-09-25T21:10:45Z

lib/widgets/message_list.dart

+        actions.add(IconButton(
+          icon: const Icon(ZulipIcons.search),
+          onPressed: () => Navigator.push(context,
+            MessageListPage.buildRoute(context: context,
+              narrow: KeywordSearchNarrow(''))),
+        ));


Could you add a tooltip: here, like on the "Channel feed" button for topic narrows?

Also, let's add this in a separate commit from the commit that changes the inbox page. Please ask in #git help in the development community if you need help doing that.

Added the tooltip: zulipLocalizations.searchMessagesPageTitle, because this same tooltip was being used for the search button (_SearchButton) inside the main menu.

chrisbobbe · 2025-09-25T21:13:09Z

lib/widgets/message_list.dart

+              narrow: KeywordSearchNarrow(''))),
+        ));


Also nit:

Suggested change

narrow: KeywordSearchNarrow(''))),

));

narrow: KeywordSearchNarrow('')))));

(like in similar code)

chrisbobbe · 2025-09-25T21:15:43Z

lib/widgets/home.dart

+        actions: _tab.value == _HomePageTab.inbox ? [
+          IconButton(
+            icon: const Icon(ZulipIcons.search),
+            onPressed: () => Navigator.of(context).push(MessageListPage.buildRoute(
+              context: context, narrow: KeywordSearchNarrow(''))),
+          ),
+        ] : null),


For the value we pass as actions, I think it would be neater as _currentTabAppBarActions, on the pattern of _currentTabTitle.

Added _currentTabAppBarActions, following the pattern of _currentTabTitle

chrisbobbe · 2025-09-25T21:31:16Z

test/widgets/home_test.dart

  //   to an ancient server:
  //     https://github.com/zulip/zulip-flutter/pull/1410#discussion_r1999991512
+
+  group('search button', () {


I think a neater way to cover the inbox page in tests would be:

Don't add this new 'search button' group. (The search button isn't really a feature of the whole home page, just the inbox.)

To test that the button is present on the "Inbox" tab and not the other tabs, update and rename the existing test 'update app bar title when switching between views' in the 'bottom nav navigation' group.

In test/widgets/inbox_test.dart, add a test that the button does the correct thing when tapped.

Made the changes you suggested.
Thanks for pointing out, made me go through tests again which helped me understand the structures of these tests even more.

chrisbobbe · 2025-09-25T21:39:16Z

test/widgets/message_list_test.dart

+  group('app bar search button', () {
+    // Tests for the search icon button that should appear in the app bar
+    // on the combined feed screen but not on other message list screens.
+    // We check the app bar's actions list directly to avoid confusion with
+    // the search icon that appears in the search input field.
+
+    testWidgets('search icon button appears on CombinedFeedNarrow', (tester) async {
+      // Set up the combined feed (all messages) view
+      await setupMessageListPage(tester, narrow: const CombinedFeedNarrow(), messages: []);
+
+      // Verify that the search button is present in the app bar actions
+      final appBar = tester.widget<ZulipAppBar>(find.byType(ZulipAppBar));
+      final actions = appBar.actions ?? [];
+      final hasSearchButton = actions.any((widget) =>
+        widget is IconButton &&
+        widget.icon is Icon &&
+        (widget.icon as Icon).icon == ZulipIcons.search);
+      check(hasSearchButton).isTrue();
+    });


I think we don't need tests to check that the button is absent for various narrows. I think there's almost no risk that we'll accidentally create a bug that such tests would catch, which means the tests aren't very useful. Here's a helpful talk by Greg covering that kind of advice and more: #learning > Greg's talk at FlutterCon USA 2025 @ 💬

I think we just need one test that looks for the search button in "Combined feed" and checks that it does the correct thing when tapped. That test should go in the existing "app bar" group.

Well, above in the thread @gnprice mentioned to check for the cases that the button should be absent for various narrows. But what you mentioned also made sense and I removed these absent check tests. Although if you think we should have them back, will add them again.

Thanks.

Adds a search icon and tooltip to the icon in the home page app bar that appears on the inbox tab. Uses the same localized 'Search' text for consistency with the message list search button. Also refactors the app bar actions logic by extracting it into a _currentTabAppBarActions getter. Updates the existing bottom nav navigation test inside home_test to verify search button presence across different tabs: present on inbox, absent on channels and direct messages tabs. Also updates the existing InboxPage tests to verify the search button navigates to search page when tapped

Adds a search icon button in the message list app bar that appears on the combined feed narrow. Uses the same localized 'Search' text for consistency with the message list search button. Also adds a focused test in the existing 'app bar' group inside message_list_test that verifies the search button appears on combined feed and navigates correctly to the search page when tapped.

komal-uniqode · 2025-09-27T20:02:30Z

@chrisbobbe , just a heads up!
After your review I went through the commit discipline documentation and it helped me a lot in understanding the project commit style and how should I manage and make my changes more reviewable. So I refactored my PR completely to follow the commit style of the project.

Thank you so much for the valuable feedback!!

komal-uniqode mentioned this pull request Sep 20, 2025

Search button in app bar on inbox and combined feed #1854

Open

chrisbobbe requested changes Sep 25, 2025

View reviewed changes

gnprice added the maintainer review PR ready for review by Zulip maintainers label Sep 26, 2025

Komal Singh added 2 commits September 28, 2025 01:14

komal-uniqode force-pushed the fix-add-search-icon branch from 3453701 to 3608a3e Compare September 27, 2025 19:47

	narrow: KeywordSearchNarrow(''))),
	));
	narrow: KeywordSearchNarrow('')))));

adds search icon to inbox and combined feed pages #1864

Are you sure you want to change the base?

adds search icon to inbox and combined feed pages #1864

Conversation

komal-uniqode commented Sep 20, 2025

Uh oh!

komal-uniqode commented Sep 20, 2025

Uh oh!

gnprice commented Sep 22, 2025

Uh oh!

komal-uniqode commented Sep 22, 2025

Uh oh!

alya commented Sep 23, 2025

Uh oh!

komal-uniqode commented Sep 24, 2025

Uh oh!

komal-uniqode commented Sep 25, 2025

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!

komal-uniqode commented Sep 27, 2025

Uh oh!

Uh oh!