share: Support switching accounts #1883

rajveermalviya · 2025-09-30T17:49:19Z

Stacked on top of ea5cd48 and b8e0143 from #1937.

Separated from #1816 to preserve some drafts commits there from @chrisbobbe.

Screenshots coming..

Tests are TODO and will happen as part of #1787 in a follow-up PR.

Figma: https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=12853-76543&p=f&t=oBRXWxFjbkz1yeI7-0
Fixes: #1779

rajveermalviya · 2025-10-03T16:27:01Z

Screenshots:

Channels	DMs	Choose account

chrisbobbe

Thanks! Comments below.

chrisbobbe · 2025-10-07T21:55:19Z

lib/widgets/share.dart

          SubscriptionListPageBody(
            showTopicListButtonInActionSheet: false,
            hideChannelsIfUserCantSendMessage: true,
-            allowGoToAllChannels: false,


This removal looks unintentional.

chrisbobbe · 2025-10-07T22:00:42Z

lib/widgets/share.dart

+    // We should already have the `store.realmIcon` after the
+    // PerAccountStore has completed loading, hence the `!` here.
+    final realmIconUrl = store.realmUrl.resolveUri(store.realmIcon!);


If a PerAccountStore is guaranteed to have non-null .realmIcon, then why is it typed as optional? If we can make it required, we should do so, right? (Ditto .realmName.) Then if there's some subtlety in explaining that, we can do it just once where the field is declared, instead of all the places that try to consume it.

chrisbobbe · 2025-10-07T22:13:39Z

lib/widgets/share.dart

+          SizedBox.square(
+            dimension: 42,
+            child: Padding(
+              padding: const EdgeInsets.all(7),
+              child: RealmContentNetworkImage(realmIconUrl))),


Following the link to the "company-logo" component in Figma—

https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=421-14693&m=dev

—it looks like this is supposed to have a 4px border radius:

Would the AvatarShape widget be appropriate here? Its dartdoc says "A rounded square shape, to wrap an [AvatarImage] or similar.".

I also see an additional 4px horizontal padding in the Figma that's not implemented in this revision:

https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=12853-76635&m=dev

chrisbobbe · 2025-10-07T22:21:39Z

lib/widgets/share.dart

+            labelStyle: labelStyle,
+            labelColor: designVariables.iconSelected,
+            unselectedLabelColor: designVariables.icon,
+            indicatorWeight: 0,


Hmm, two things stand out to me in the dartdoc of indicatorWeight:

/// The value of this parameter must be greater than zero.

/// If [indicator] is specified […] this property is ignored.

What happens if we don't pass indicatorWeight?

Added a bit of context in a comment on why the indicatorWeight: 0 is needed here.

chrisbobbe · 2025-10-07T22:23:42Z

lib/widgets/share.dart

+    final labelStyle = TextStyle(
+      fontSize: 18,
+      height: 24 / 18,
+      letterSpacing: 0,


Isn't letterSpacing: 0 the default, from zulipTypography?

chrisbobbe · 2025-10-07T23:36:51Z

lib/widgets/share.dart

+                child: RealmContentNetworkImage(realmIconUrl))),
+          ),


Suggested change

child: RealmContentNetworkImage(realmIconUrl))),

),

child: RealmContentNetworkImage(realmIconUrl)))),

chrisbobbe · 2025-10-08T20:59:54Z

lib/widgets/share.dart

+    ShareDialog.show(
+      pageContext: context,
+      initialAccountId: accountId,
+      sharedFiles: sharedFiles,
+      sharedText: intentSendEvent.extraText);


share: Re-design from page to a dialog

This commit breaks the "New DM" button in the "Direct messages" tab, because a PageRoot ancestor is no longer found when the button is tapped:

======== Exception caught by gesture =============================================================== The following assertion was thrown while handling a gesture: No PageRoot ancestor 'package:zulip/widgets/page.dart': Failed assertion: line 26 pos 12: 'element != null' When the exception was thrown, this was the stack: #2 PageRoot.contextOf (package:zulip/widgets/page.dart:26:12) #3 showNewDmSheet (package:zulip/widgets/new_dm_sheet.dart:17:32) #4 _NewDmButtonState.build.<anonymous closure> (package:zulip/widgets/recent_dm_conversations.dart:270:20) [etc.]

chrisbobbe · 2025-10-08T22:06:07Z

lib/widgets/share.dart

+      unawaited(() async {
+        final GetServerSettingsResult serverSettings;
+        final connection = globalStore.apiConnection(
+          realmUrl: account.realmUrl,
+          zulipFeatureLevel: null);
+        try {
+          serverSettings = await getServerSettings(connection);
+        } catch (_) {
+          return;
+        } finally {
+          connection.close();
+        }
+
+        if (globalStore.getAccount(accountId) != null) {
+          await globalStore.updateRealmData(
+            accountId,
+            realmName: serverSettings.realmName,
+            realmIcon: serverSettings.realmIcon);
+        }
+      }());


The goal is to make sure the realm name and icon are present and up-to-date, right?

I think my ideal way to do this would be more centralized and a bit more rigorous:

The choose-account page/dialog doesn't yet show the name and icon, but it will, and it'll be equally useful there to refresh them as here.

We've written careful "refuse-to-connect-to-ancient-servers" logic that should eliminate ancient-server behavior as a possible source of bugs, which should help simplify debugging. This revision puts a small hole in that logic, which we could fix by checking ZulipVersionData.isUnsupported before interpreting realmName and realmIcon. Like we do around the other getServerSettings call.

I don't think any ancient servers format realmName and realmIcon in weird, interfering ways…but the point is being confident that it wouldn't matter even if they did.

The realm name and icon are also provided via the event system, and we should defer to that: if a PerAccountStore finishes loading before we've received and recorded the data from get-server-settings, we should abort instead of clobbering the store with this data that arrived out-of-band.

more centralized

I'm not sure yet where's the most helpful place to put this. Maybe in a stateful widget that gets passed to PerAccountStoreWidget.placeholder? I'd be happy to think about that when I revisit my draft commits in #1816, and in the meantime perhaps you can just address my second and third bullet points above, which are correctness issues.

I think a good home for this would be as a method on GlobalStore, perhaps named like ensureFreshAccountData. (Or ensureFreshRealmMetadata? The point is it's the data (a) that we store in the Account record but (b) about the realm as a whole.)

Then that method will have access to _perAccountStoresLoading as well as _perAccountStores. It can look at those in order to ensure that it doesn't do anything if there's already a PerAccountStore or one being loaded.

chrisbobbe · 2025-10-08T22:23:57Z

lib/widgets/share.dart

+          return const SizedBox.shrink();
+        }
+
+        return ListTile(


Perhaps add a TODO(#1038) for aligning this dialog with other choose-account dialogs (link: #1038)

chrisbobbe · 2025-10-08T22:28:22Z

lib/widgets/share.dart

+            onTap: hasMultipleAccounts
+              ? () {
+                ChooseAccountForShareDialog.show(
+                  pageContext: context,
+                  selectedAccountId: store.accountId,
+                  sharedFiles: sharedFiles,
+                  sharedText: sharedText);
+              }


I wonder if there's some way we can make it clearer that the org icon can be tapped to switch accounts. Like the chevron-down icon on the emoji selector on the set-status page:

I dunno, at least as an Android user, I feel like one is pretty well trained that clicking on your avatar opens the account switcher.

chrisbobbe · 2025-10-08T22:32:53Z

@alya curious for your thoughts on #1883 (comment) too.

chrisbobbe · 2025-12-02T22:59:59Z

@rajveermalviya gentle bump on this; there's material from my draft commits in #1816 that I'd like to refresh and send as a PR after this PR settles.

chrisbobbe · 2025-12-10T21:43:58Z

I see you've pushed changes but I'm not sure if you're intending them to be ready for review 🙂 please let me know when ready.

rajveermalviya · 2025-12-11T16:49:20Z

Thanks for the review @chrisbobbe! Sorry for the delay. I've pushed a new revision, PTAL.

A helper which ensures realm metadata (name and icon) are up-to-date for each account.

Figma: https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=12853-76543&p=f&t=oBRXWxFjbkz1yeI7-0

Co-Authored-By: Chris Bobbe <[email protected]> Fixes: zulip#1779

rajveermalviya mentioned this pull request Sep 30, 2025

share: Support choosing an account, if there are multiple #1816

Closed

rajveermalviya added the maintainer review PR ready for review by Zulip maintainers label Sep 30, 2025

rajveermalviya assigned chrisbobbe Sep 30, 2025

rajveermalviya requested a review from chrisbobbe September 30, 2025 17:51

chrisbobbe reviewed Oct 8, 2025

View reviewed changes

rajveermalviya force-pushed the pr-switch-account-share branch from 01bc09d to db3de4b Compare December 10, 2025 18:28

rajveermalviya force-pushed the pr-switch-account-share branch 2 times, most recently from f02fe4a to c02e265 Compare December 11, 2025 16:36

rajveermalviya requested a review from chrisbobbe December 11, 2025 16:49

rajveermalviya force-pushed the pr-switch-account-share branch from c02e265 to 79ceed6 Compare December 11, 2025 16:52

rajveermalviya and others added 3 commits December 12, 2025 12:18

store: Add ensureFreshRealmMetadata

463f92e

A helper which ensures realm metadata (name and icon) are up-to-date for each account.

share: Re-design from page to a dialog

91126ef

Figma: https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=12853-76543&p=f&t=oBRXWxFjbkz1yeI7-0

share: Support switching accounts

955d505

Co-Authored-By: Chris Bobbe <[email protected]> Fixes: zulip#1779

rajveermalviya force-pushed the pr-switch-account-share branch from 79ceed6 to 955d505 Compare December 12, 2025 06:49

	child: RealmContentNetworkImage(realmIconUrl))),
	),
	child: RealmContentNetworkImage(realmIconUrl)))),

share: Support switching accounts #1883

Are you sure you want to change the base?

share: Support switching accounts #1883

Uh oh!

Conversation

rajveermalviya commented Sep 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rajveermalviya commented Oct 3, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chrisbobbe Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chrisbobbe commented Oct 8, 2025

Uh oh!

chrisbobbe commented Dec 2, 2025

Uh oh!

chrisbobbe commented Dec 10, 2025

Uh oh!

rajveermalviya commented Dec 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

rajveermalviya commented Sep 30, 2025 •

edited

Loading

chrisbobbe Oct 8, 2025 •

edited

Loading