Skip to content

home: Show organization icon in app bar - #2365

Open
rajveermalviya wants to merge 5 commits into
zulip:mainfrom
rajveermalviya:pr-home-org-icon-switch
Open

rajveermalviya wants to merge 5 commits into
zulip:mainfrom
rajveermalviya:pr-home-org-icon-switch

Conversation

@rajveermalviya

@rajveermalviya rajveermalviya commented Jul 10, 2026 •

Copy link
Copy Markdown
Member

Fixes #1606.
Fixes #1162.

With the icon as the leading widget, the home page drops its
titleSpacing: 16 for the theme's 4px, as on pages with a back
button, so the title lines up with the titles on those pages.

Inbox Channels DMs Main menu Tooltip Another org
Screenshot 2026-11-02 at 11 07 55 Screenshot 2026-11-02 at 11 08 02 Screenshot 2026-11-02 at 11 08 08 Screenshot 2026-11-02 at 11 08 23 Screenshot 2026-11-02 at 11 08 36 Screenshot 2026-11-02 at 12 19 17
Screenshot 2026-11-02 at 11 08 58 Screenshot 2026-11-02 at 11 09 03 Screenshot 2026-11-02 at 11 09 06 Screenshot 2026-11-02 at 11 09 16 Screenshot 2026-11-02 at 11 09 21 Screenshot 2026-11-02 at 12 19 25
Screenshot 2026-11-02 at 11 56 55 Screenshot 2026-11-02 at 11 57 01 Screenshot 2026-11-02 at 11 57 05 Screenshot 2026-11-02 at 11 57 10 Screenshot 2026-11-02 at 11 57 44 Screenshot 2026-11-02 at 12 01 39
Screenshot 2026-11-02 at 11 58 13 Screenshot 2026-11-02 at 11 58 16 Screenshot 2026-11-02 at 11 58 21 Screenshot 2026-11-02 at 11 58 25 Screenshot 2026-11-02 at 11 58 31 Screenshot 2026-11-02 at 12 01 27

@rajveermalviya rajveermalviya added the integration review Added by maintainers when PR may be ready for integration label Jul 10, 2026
@rajveermalviya
rajveermalviya force-pushed the pr-home-org-icon-switch branch from 026440c to ba463a5 Compare August 4, 2026 12:50
@rajveermalviya
rajveermalviya force-pushed the pr-home-org-icon-switch branch from ca508ff to 56eb765 Compare October 22, 2026 11:15
@chrisbobbe

Copy link
Copy Markdown
Member

Thanks! Would you go through the following points Claude offered in a review? They look right to me:

Details

1. The test fakes only work because of test order

The prepareBoringImageHttpClient() calls added in 61464a9 work by filling
the shared imageCache with the realm-icon URL. Every later test in the same
file then gets the icon from that cache, with no fake client. That's why the
choice of tests looks arbitrary: inbox_test "page builds; empty" gets a fake
but "page builds; not empty" doesn't, and subscription_list_test "empty" gets
one but "basic subscriptions" doesn't.

Checked, at 56eb765:

  • One test that didn't get a fake, run on its own:
    flutter test test/widgets/inbox_test.dart --plain-name 'page builds; not empty'
    → fails with NetworkImageLoadException ... statusCode: 400, https://chat.example/user_avatars/2/realm/icon.png?version=3.
  • flutter test test/widgets/inbox_test.dart --test-randomize-ordering-seed=12345
    → 25 of 32 tests fail.
  • With the PR's changes to the 11 test files other than home_test.dart
    removed: 136 of 553 tests in those files fail.

This follows existing practice. The main-menu tests in home_test.dart
(L317-L473)
already fake the client for _MainMenuHeader's identical realm-icon image.

Likely better fix: errorBuilder on the icon

In debug mode, Image throws a load error only when errorBuilder is null
(_ImageState._getListener in Flutter's widgets/image.dart).
AvatarImage passes an errorBuilder
(user.dart:92).
That's why tests that only show avatars along the way
(inbox, recent DMs, new-DM sheet) don't need fakes.
user_test.dart does fake the client, because it checks that the image loads.

Checked, at 56eb765: I added
errorBuilder: (_, _, _) => const SizedBox.shrink()
to _OrgIcon's RealmContentNetworkImage,
and removed the PR's changes to the 11 test files other than home_test.dart.
All 586 tests in those 12 files pass,
and so does the single "not empty" test run on its own.
That would drop almost all of the ~165 added test lines.
It also stops debug builds from drawing Flutter's red error box
when the icon can't load.
For what the placeholder should look like, see the TODO(#1558) at
user.dart:100.
The test paragraph in 61464a9's commit message would then need to go.

2. Tooltip / semantic label after 56eb765

The IconButton tooltip is store.realmName
(home.dart:158).
Once tapping it opens ChooseAccountPage, that becomes the button's label for
screen readers too. The main-menu header, which does the same thing, uses
switchAccountButtonTooltip ("Switch account")
(home.dart:502).
Which do we want?

My own notes on each of those:

  1. Claude's "For what the placeholder should look like […]" is wrong, I think; that would make us use a person-shaped icon. Either find an icon representing the concept of a realm/org (maybe web or desktop has one?) or just omit the icon from the placeholder (but otherwise keep it the same; I think maybe it's a gray rounded square).
  2. Let's say maybe "Switch account ()" for this button, so e.g. "Switch account (Recurse Center)". Could also switch the main-menu header to say that too, for consistency.

This will be used to show the organization icon in the home page
app bar.
@rajveermalviya
rajveermalviya force-pushed the pr-home-org-icon-switch branch from 56eb765 to b625593 Compare October 30, 2026 11:13
rajveermalviya and others added 4 commits November 2, 2026 10:42
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Without an `errorBuilder`, a failed load leaves an empty space,
or in debug builds a red error placeholder.

The placeholder is the avatar placeholder's gray rounded square,
without its person icon, which would be wrong for an organization.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Fixes zulip#1606.

With the icon as the leading widget, the home page drops its
`titleSpacing: 16` for the theme's 4px, as on pages with a back
button, so the title lines up with the titles on those pages.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Fixes zulip#1162.

The button's tooltip, which is also its label for screen readers,
names the organization, since on the home page the icon is all
that identifies it.

The tooltip goes through `IconButton.tooltip`, with a `TooltipTheme`
to keep it below the icon, because a `Tooltip` around the button
would put that label outside the button's semantics node.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rajveermalviya
rajveermalviya force-pushed the pr-home-org-icon-switch branch from b625593 to be630e7 Compare November 2, 2026 05:14
@rajveermalviya

Copy link
Copy Markdown
Member Author

Thanks for the review @chrisbobbe! Pushed an update, PTAL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

integration review Added by maintainers when PR may be ready for integration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

home: Show org icon in app bar (Inbox, Channels, Direct messages) One-tap access to list of accounts from home

2 participants