Support Share to Zulip on Android #1774

rajveermalviya · 2025-07-31T00:24:10Z

Screenshots

Channels (Light)	Channels (Dark)	Direct Messages (Light)	Direct Messages (Dark)	Long Tab Name	Error dialog

Screen recordings

	Single File	Multiple Files	Text	Text + File
Channels	channels-single-file.mp4	channels-multiple-files.mp4	channels-text.mp4	channels-text+image.mp4
DMs	dms-single-file.mp4	new-dms-multiple-file.mp4	dms-text.mp4	new-dms-text+image.mp4

Fixes: #53

gnprice · 2025-07-31T01:37:47Z

Great, glad to see this!

I've watched the three videos. The UX generally looks good. Comments there:

When choosing a channel, it looks like focus goes to the main compose input rather than the topic input. The spec called for the latter. Can you easily make it do that instead? (If not, that can be a follow-up issue; not a blocker for shipping this.)
This version uses the first account in the list. That's fine for an initial version. Let's file a follow-up issue (in M6, the milestone that starts next week) for allowing the user to select an account from the list if there are several of them, as described in the spec.
The "channels" and "direct messages" tabs are labeled by icons. I think the usual convention for top tabs like these would be to use text labels instead.
Can you post a screenshot and a video demonstrating using the "New DM" button? Because that does some navigation of its own, I'm curious to see how it interacts.

/cc @alya also for her feedback on the UX.

I haven't yet looked much at the code (since this is a draft), just briefly skimmed. It looks like one key thing still for you to do is to split the changes out into distinct commits, which will help with reading them.

gnprice

OK, and here's comments from a high-level scan of the code. Excited to see this feature!

gnprice · 2025-07-31T04:04:50Z

android/app/src/main/kotlin/com/zulip/flutter/MainActivity.kt


-class MainActivity: FlutterActivity() {
+class MainActivity : FlutterActivity() {


There's a fair amount of code added here. Was there a particular place (or handful of places) you looked for working out what this code should look like? That'll be helpful to point to in commit messages.

For example, specific files in the legacy app; or places in the Flutter tree; or Android documentation; or other sources.

gnprice · 2025-07-31T04:09:00Z

android/app/src/main/kotlin/com/zulip/flutter/MainActivity.kt

+      bytes = bytes)
+  }
+
+  private fun handleSend(intent: Intent) {


Can this method (and its helper getIntentSharedFileInfo) move onto a class other than MainActivity? Perhaps on AndroidIntentEventListener, since that seems closely related.

That would help for making it explicit what information or methods they're using, if any, from FlutterActivity or its base classes.

Even if the methods have to be passed this as a FlutterActivity (or some supertype of that), I think the explicitness of accessing FlutterActivity members on that argument would be helpful.

gnprice · 2025-07-31T04:17:01Z

lib/widgets/app.dart

@@ -161,6 +165,40 @@ class _ZulipAppState extends State<ZulipApp> with WidgetsBindingObserver {
    super.initState();
    WidgetsBinding.instance.addObserver(this);
    UpgradeWelcomeDialog.maybeShow();
+
+    // Move to a AndroidShareService or similar.


Agreed, this would be a helpful bit of code organization.

I think a good home for that would be in lib/widgets/share.dart . It's fundamentally closely tied to the SharePage widget.

gnprice · 2025-07-31T04:17:33Z

lib/widgets/app.dart

+    ZulipBinding.instance.androidIntentEvents.forEach((androidIntentEvent) async {
+      assert(debugLog('androidIntentEvent.action: ${androidIntentEvent.action}'));


Let's also make this callback a named function. Can be a private static method on the relevant class.

gnprice · 2025-07-31T04:19:44Z

lib/widgets/compose_box.dart

+  @override
+  void insertText(String newText) {
+    final contentController = controller.content;
+    contentController.value = contentController.value.replaced(contentController.insertionIndex(), '$newText\n');
+  }


How about inlining this at the call site? The caller can use the controller getter, which is already exposed in the ComposeBoxState interface.

(That's effectively what we do for things like quote-and-reply.)

gnprice · 2025-07-31T04:28:51Z

lib/widgets/new_dm_sheet.dart

+          Navigator.pop(context); // TODO why doesn't this work?
+          onSelect!(narrow);


Not sure if this TODO is just a note to yourself or if you'd like input on it — if the latter, I'd be curious to hear what behavior you're seeing, and what behavior you expected 🙂

gnprice · 2025-07-31T04:29:35Z

lib/widgets/recent_dm_conversations.dart

-          Navigator.push(context,
-            MessageListPage.buildRoute(context: context, narrow: narrow));
+          if (onSelect != null) {
+            onSelect!(narrow);
+          } else {
+            Navigator.push(context,
+              MessageListPage.buildRoute(context: context,
+                narrow: narrow));
+          }


Similar comment as for the onSelect on _NewDmHeader above.

gnprice · 2025-07-31T04:36:24Z

lib/widgets/subscription_list.dart

-        onLongPress: () => showChannelActionSheet(context, channelId: subscription.streamId),
+        onLongPress: !disableChannelActionSheet
+          ? () => showChannelActionSheet(context, channelId: subscription.streamId)
+          : null,


Interesting. What goes wrong without this change, if the user does try to open a channel action sheet?

The issue was that user could open the Topic List page from that action sheet, and when selected a topic from that topic list, the compose box of the topic message list wouldn't be populated with the content.

See discussion: #mobile-design > share to Zulip @ 💬

The latest revision now replaces this flag with one to disable just the topic list button from the channel action sheet, rather than disabling whole action sheet.

gnprice · 2025-07-31T04:37:11Z

pigeon/android_intents.dart

+// TODO separate out API calls for resolving file name, getting mimetype, getting bytes?
+class IntentSharedFile {


That sounds potentially useful but definitely a follow-up task, if this version already seems to work correctly 🙂

gnprice · 2025-07-31T04:44:49Z

lib/widgets/share.dart

+      if (filesToUpload != null) {
+        await composeBoxState.uploadFiles(filesToUpload!);
+      } else if (sharedText != null) {
+        composeBoxState.insertText(sharedText!);
+      }


Does this need the else? It seems like this could work just fine with both if blocks acting independently.

Probably then put the text before the uploads.

I'm imagining that there might be sharing sources that include both some text and some images or other files. I'm not sure if the platform APIs support that; but it's definitely a natural thing for people to want to do, so perhaps they might. (After all, even when the thing shared from the other app is purely an image, people often/usually want to add to add some text before sending it, in Zulip as well as in many other apps.)

alya · 2025-07-31T06:32:07Z

Yeah, I also wanted to note that it would be more clear to label the tabs as "Channels" and "Direct messages" (or "DMs" if that fits better). Looks promising!

rajveermalviya · 2025-08-01T06:07:11Z

Thanks for the review @gnprice! Pushed an update and also updated the screenshots and recording, PTAL.

rajveermalviya · 2025-08-01T06:41:30Z

When choosing a channel, it looks like focus goes to the main compose input rather than the topic input. …

Current revision tries to handle this.

This version uses the first account in the list. That's fine for an initial version. Let's file a follow-up issue (in M6, the milestone that starts next week) for allowing the user to select an account from the list if there are several of them, as described in the spec.

Filed #1779.

The "channels" and "direct messages" tabs are labeled by icons. I think the usual convention for top tabs like these would be to use text labels instead.

Fixed.

Can you post a screenshot and a video demonstrating using the "New DM" button? …

Added the video in the description, specifically it's the one labelled as "Text (New DM)".

alya · 2025-08-01T17:10:13Z

In your screenshots, why does the list of channels start with "Unpinned"? I would expect that to appear only if there are pinned channels above.

rajveermalviya · 2025-08-01T17:33:51Z

Looks like we show the "Unpinned" header unconditionally on Subscription list page:

zulip-flutter/lib/widgets/subscription_list.dart

Lines 111 to 114 in d5dddb3

    
           if (unpinned.isNotEmpty) ...[ 
        
             _SubscriptionListHeader(label: zulipLocalizations.unpinnedSubscriptionsLabel), 
        
             _SubscriptionList(unreadsModel: unreadsModel, subscriptions: unpinned), 
        
           ],

Screenshot

gnprice · 2025-08-01T17:34:16Z

Thanks for the revision!

From the updated videos, the one UX comment I have is: after choosing a stream, it looks like focus starts out in the content input, and gets moved to the topic input only after the upload completes. Can we instead move it to the topic input immediately? That way the user can be choosing the topic while the upload is happening.

gnprice · 2025-08-01T17:34:57Z

lib/widgets/share.dart

+      // Then upload the files and populate the compose box with their links.
+      if (sharedFiles != null) {
+        await composeBoxState.uploadFiles(sharedFiles!);
+      }
+
+      // Try to focus on the topic compose box if there is one,
+      // else focus on content compose box, if not already focused.


I think specifically that means this latter step should happen before doing an await on the former step.

Thanks! Fixed in current revision, and also updated videos, PTAL.

chrisbobbe

Thanks, this is exciting! I didn't use the legacy app's implementation at all in my regular Zulip use, but from what I remember, this feels much smoother. Comments below.

chrisbobbe · 2025-08-01T22:05:40Z

lib/widgets/compose_box.dart

@@ -928,7 +928,7 @@ class FileToUpload {
 Future<void> _uploadFiles({
  required BuildContext context,
  required ComposeContentController contentController,
-  required FocusNode contentFocusNode,
+  required FocusNode? contentFocusNode,


compose [nfc]: Expose uploadFiles on ComposeBoxState This will be used soon to allow uploading files directly via using `MessageListPageState`.

I don't love this change to _uploadFiles's interface: on the surface, it looks like contentController and contentFocusNode disagree about whether the content input always or only sometimes exists.

From the later commit, I can infer that this change isn't actually about the presence/absence of the content input; it's about a detail of how the function interacts with the input. Instead of doing this, how about making a flag that's specific to that behavior detail, like bool shouldRequestFocus = true?

Relatedly (but I wouldn't block on it), _uploadFiles could get its contentController and contentFocusNode from the ComposeBoxController, couldn't it? That could be a single param to replace those two params. Or it could even come from the context param, via ComposeBoxInheritedWidget.of, with some prep work to add a field for it to ComposeBoxInheritedWidget.

chrisbobbe · 2025-08-01T22:06:37Z

lib/widgets/compose_box.dart

@@ -1869,6 +1869,24 @@ abstract class ComposeBoxState extends State<ComposeBox> {

  /// Switch the compose box back to regular non-edit mode, with no content.
  void endEditInteraction();
+
+  /// Uploads the provided files, populating the compose box with their links.


nit: s/compose box/content input/

chrisbobbe · 2025-08-01T22:09:39Z

assets/l10n/app_en.arb

+  "errorSharingAccountNotLoggedIn": "There was no account logged in. Please login to an account and try again.",
+  "@errorSharingAccountNotLoggedIn": {
+    "description": "Error title when sharing content received from other apps fails, when there was no account logged in"
+  },


nits:

"There is" instead of "There was", right?

"log in" for the verb, as here; "login" for the noun

chrisbobbe · 2025-08-01T22:22:17Z

lib/widgets/share.dart

+      // Try to focus on the topic compose box if there is one,
+      // else focus on content compose box, if not already focused.
+      if (composeBoxController is StreamComposeBoxController) {
+        if (!composeBoxController.topicFocusNode.hasFocus) {
+          composeBoxController.topicFocusNode.requestFocus();
+        }
+      } else {
+        if (!composeBoxController.contentFocusNode.hasFocus) {
+          composeBoxController.contentFocusNode.requestFocus();
+        }
+      }


How about composeBoxController.requestFocusIfUnfocused() for this? 🙂

chrisbobbe · 2025-08-01T22:25:46Z

lib/widgets/subscription_list.dart

  });

  final bool disableChannelActionSheet;
  final bool hideChannelsIfUserCantPost;
+  final OnChannelSelectCallback? onChannelSelect;


How about adding TODO(#412) add onTopicSelect? In share-to-Zulip, that will probably be more useful to people, when we get around to it :) especially if it includes a "new topic" button as I've just suggested on that issue: #412 (comment)

…Actually, trying this out now, I think we might (eventually) want to force the user to choose a topic or start a new topic. When I land on the interleaved channel view with an empty topic input, the first thing I tried was to tap a topic recipient header, which brought me to a new page (the topic page) without the content I wanted to share.

chrisbobbe · 2025-08-01T22:54:41Z

android/app/src/main/kotlin/com/zulip/flutter/IntentListener.kt

+import android.net.Uri
+import android.provider.OpenableColumns
+
+class IntentListener : AndroidIntentEventsStreamHandler() {


nit: How about:

naming this IntentEventListener or even AndroidIntentEventListener, corresponding to AndroidIntentsEventChannelApi.androidIntentEvents()

and below, rename onAndroidIntentEvent to just onEvent?

This way feels more streamlined in loading the "event" abstraction into the reader's brain, and also making it clear that "android intent event" is the only kind of event this class is responsible for.

chrisbobbe · 2025-08-01T23:27:11Z

android/app/src/main/kotlin/com/zulip/flutter/IntentListener.kt

+    // App can receive both an EXTRA_TEXT and EXTRA_STREAM (files)
+    // in the same intent. And the documentation states that EXTRA_TEXT
+    // should always be "text/plain", and it also states that it can be
+    // a mime type of the file/s in EXTRA_STREAM, but while testing
+    // Chrome seems to always set this as the source URL for the shared
+    // image, and Firefox sets this to null when sharing an image.
+    // So, we use this string as-is, assuming the documented later part
+    // isn't observed in the wild.
+    //
+    // See: https://developer.android.com/reference/android/content/Intent#ACTION_SEND
+    val extraText = intent.getStringExtra(Intent.EXTRA_TEXT)
+
+    val event = when (intent.action) {
+      Intent.ACTION_SEND -> {
+        if ("text/plain" == intent.type) {
+          AndroidIntentSendEvent(
+            action = Intent.ACTION_SEND,
+            extraText = extraText
+          )
+        } else {
+          // TODO(android-sdk-33) Remove the use of deprecated API.
+          @Suppress("DEPRECATION") val url = intent.getParcelableExtra<Uri>(Intent.EXTRA_STREAM)
+            ?: throw Exception("Could not extract URL from File Intent")
+          val sharedFile = getIntentSharedFile(context, url)
+          AndroidIntentSendEvent(
+            action = Intent.ACTION_SEND,
+            extraText = extraText,
+            extraStream = listOf(sharedFile)
+          )
+        }
+      }
+
+      Intent.ACTION_SEND_MULTIPLE -> {
+        // TODO(android-sdk-33) Remove the use of deprecated API.
+        @Suppress("DEPRECATION") val urls =
+          intent.getParcelableArrayListExtra<Uri>(Intent.EXTRA_STREAM)
+            ?: throw Exception("Could not extract URLs from File Intent")
+        val extraStream = mutableListOf<IntentSharedFile>()
+        for (url in urls) {
+          val sharedFile = getIntentSharedFile(context, url)
+          extraStream.add(sharedFile)
+        }
+        AndroidIntentSendEvent(
+          action = Intent.ACTION_SEND_MULTIPLE,
+          extraText = extraText,
+          extraStream = extraStream
+        )
+      }


I think this simplifies to the following:

Comment that the doc is wrong to imply that EXTRA_TEXT and EXTRA_STREAM can't both be present ("get*Extra can have either a EXTRA_TEXT or EXTRA_STREAM field"); this happens empirically

First, check for EXTRA_STREAM. If it's present, include the file(s).

Add EXTRA_TEXT, too, if that's present. No mime-type check (the current revision defeats sharing a plain-text file).

Maybe a comment that EXTRA_TEXT might be text that the user doesn't want, like the URL of an image on the web. But, shrug, we're including it anyway because that's what Chrome/etc. put in the intent, and it's the other app's responsibility to choose what it intends to be shared.

Is that right?

Correct. Also, sent a new revision further simplifying and eliminating the if ("text/plain" == intent.type) { branch, PTAL.

chrisbobbe · 2025-08-01T23:32:01Z

lib/widgets/share.dart

+        sharedFiles: intentSendEvent.extraStream?.map((sharedFile) {
+          return FileToUpload(
+            content: Stream.value(sharedFile.bytes),
+            length: sharedFile.bytes.length,
+            filename: sharedFile.name,
+            mimeType: sharedFile.mimeType);
+        }),


Do we want to call lookupMimeType when sharedFile.mimeType is null? (See example in compose_box.dart)

If that's complicated, we could file a followup for it.

chrisbobbe · 2025-08-01T23:49:40Z

lib/widgets/share.dart

+            SubscriptionListPageBody(
+              disableChannelActionSheet: true,
+              hideChannelsIfUserCantPost: true,
+              onChannelSelect: _handleNarrowSelect),
+            RecentDmConversationsPageBody(
+              hideDmsIfUserCantPost: true,
+              onDmSelect: _handleNarrowSelect),


It's nice to do a tearoff for _handleNarrowSelect, i.e. onChannelSelect: _handleNarrowSelect vs. onChannelSelect: (…) => _handleNarrowSelect(…). But is that the only reason that _handleNarrowSelect is getting passed a BuildContext from much deeper than this in the widget tree?

If _handleNarrowSelect adds some async work up front (which, true, it doesn't have right now), then we'll need an isMounted check after the await…and then I think we need to investigate if something like a channel-rename can cause the context to unmount, if the context is on a per-channel widget.

Simpler to just use this widget's context, if we can:

onChannelSelect: (narrow) => _handleNarrowSelect(narrow, context)

right?

Hmm, I had added the BuildContext because in the initial draft I was observing a bug that pressing back from the message list page, when opened from new dm conversation action sheet. I would be led to the share page again instead of the home page. Even with the extra Navigator.pop for the action sheet. And I had thought that adding the BuildContext and passing that for Navigator was what fixed it.

But for some reason after I've removed this BuildContext now, I do not observe that bug again, so looks like it was something else in the initial draft that was not working.

Anyway removed the BuildContext in the latest revision.

chrisbobbe · 2025-08-02T00:00:26Z

lib/widgets/share.dart

+              Tab(text: zulipLocalizations.channelsPageTitle),
+              Tab(text: zulipLocalizations.recentDmConversationsPageTitle),
+            ])),
+        body: SafeArea(


This calls for a

// Don't pad the bottom here; we want the list content to do that. bottom: false,

. And if we're settled on repurposing SubscriptionListPageBody and RecentDmConversationsPageBody¹, then a few more changes are needed:

Revert the changes to those widgets in 742320c

Move the "New DM" button up by the height of the bottom inset (the new-DM button landed after we added the bottom nav)

Find a factoring or comment to explain why the FooPageBody widgets aren't uniform in these details

Footnotes

I wonder if this is a case of "prefer duplication over the wrong abstraction": https://sandimetz.com/blog/2016/1/20/the-wrong-abstraction ↩

I think the "hide channels/conversations where the user can't post" flags, and the "don't offer channel action sheet" flag, are definitely signs that these existing "page body" widgets are probably not the abstractions we'd ideally want to be sharing, and that the ideal way to factor this would probably involve two new widgets and a certain amount of duplication.

OTOH there's quite a bit of logic in both of these pages, and the bulk of it I wouldn't want to duplicate: we really do mean for the the bulk of that behavior and UI to be uniform between this sharing context and the home-tabs context, and would risk divergence if we started having two copies of it. I think getting to the point where we could duplicate the right bits without duplicating all the rest would require some nontrivial refactoring.

So I think repurposing those widgets is the right solution for the present.

For the SafeArea: the page-body widgets are already taking care of that, right? As Chris says, those need bottom: false added. Then they'll continue to take care of the left and right insets; and the top inset is handled by the app bar.

So I think this SafeArea can be omitted. Is there something that goes wrong if it's left out?

For the SafeArea: the page-body widgets are already taking care of that, right?

Taking care of what specifically? 🙂

In this revision, in the emulator I'm running, the middle of the list looks like this:

No change if I remove this SafeArea.

It should look like this, in the middle of the list:

And this, at the bottom of the list:

But it doesn't because 742320c stopped delegating to list content the job of detecting and padding the bottom inset, and instead relied on the assumption that that's handled externally, by the bottom tabs.

Right. So the page-body widgets are already taking care of the left and right insets (and, wrongly for this purpose, a bottom inset). There's nothing for a SafeArea here to do.

And those other SafeArea calls need to be adjusted to not try to handle a bottom inset.

Sent this as c9391df, PTAL.

Responded there—for some reason, as Greg pointed out, my comment didn't end up showing on the PR thread: c9391df#r163401974

rajveermalviya · 2025-08-03T02:10:28Z

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

This reverts part of 742320c for RecentDmConversationsPageBody and SubscriptionListPageBody, as these widgets will soon be used outside the context of home page, specifically for the upcoming share page. The other *PageBody widgets, currently only InboxPageBody, don't need these as they aren't used outside the context of the home page.

chrisbobbe

Thanks! Small comments below, and this continues to work in my manual testing. I guess we'll want a TODO and an issue to write tests, if we're aiming to ship this before tests are written? (Like #1620 etc.)

chrisbobbe · 2025-08-04T19:21:29Z

android/app/src/main/kotlin/com/zulip/flutter/AndroidIntentEventListener.kt

+    // App can receive both an EXTRA_TEXT and EXTRA_STREAM (files)
+    // in the same intent. And the documentation states that EXTRA_TEXT
+    // should always be "text/plain", and it also states that it can be
+    // a mime type of the file/s in EXTRA_STREAM, but while testing
+    // Chrome seems to always set this as the source URL for the shared
+    // image, and Firefox sets this to null when sharing an image.
+    // So, we use this string as-is, assuming the documented later part
+    // isn't observed in the wild.
+    //
+    // See: https://developer.android.com/reference/android/content/Intent#ACTION_SEND
+    val extraText = intent.getStringExtra(Intent.EXTRA_TEXT)


Following up from #1774 (comment) , this comment is still confusing to me:

It makes it sound like the doc implies that the EXTRA_TEXT string itself represents a mime type, but the doc doesn't imply that. (It mentions getType giving a mime type.)

About the possibility that EXTRA_TEXT and EXTRA_STREAM are both present:

It should make clear why we believe this behavior is possible: because we've seen it. (IOW it's not from reading code or seeing a StackOverflow thread or something)

It should make clear why that's interesting/important: because it contradicts the Android doc, and we've created our own interpretation of what it means.

It would be helpful to make clear what interpretation we're relying on: that files and text are both included in the share-to intent, not just one or the other.

How about:

assert( intentAction == Intent.ACTION_SEND || intentAction == Intent.ACTION_SEND_MULTIPLE ) // EXTRA_TEXT and EXTRA_STREAM are the text and file components of the content, respectively. // The ACTION_SEND{,_MULTIPLE} docs say "either" / "or" will be present: // https://developer.android.com/reference/android/content/Intent#ACTION_SEND // But empirically both can be present, commonly, so we accept that form, interpreting it as // an intent to share both kinds of data. // // Empirically, sometimes EXTRA_TEXT isn't something we think needs to be shared, like the URL // of a file that's present in EXTRA_STREAM…but we shrug and include it anyway because we don't // want to second-guess other apps' decisions about what to include; it's their responsibility. val extraText = intent.getStringExtra(Intent.EXTRA_TEXT) val extraStream = when (intentAction) { Intent.ACTION_SEND -> { // etc.

chrisbobbe · 2025-08-04T19:28:24Z

lib/widgets/subscription_list.dart

+  final bool disableChannelActionSheet;
+  final bool hideChannelsIfUserCantPost;
+  final OnChannelSelectCallback? onChannelSelect;
+  // TODO(#412) add onTopicSelect


In this comment, could we mention the idea of forcing the user to choose a topic or start a new topic? (Or maybe somewhere more specific to the share-to code.) As I mentioned in https://github.com/zulip/zulip-flutter/pull/1774/files#r2248956083 , the current behavior can be pretty frustrating:

When I land on the interleaved channel view with an empty topic input, the first thing I tried was to tap a topic recipient header, which brought me to a new page (the topic page) without the content I wanted to share.

chrisbobbe · 2025-08-04T19:29:59Z

lib/widgets/compose_box.dart

+  /// If any of the file is larger than maximum file size allowed by the
+  /// server, an error dialog is shown mentioning their names and actual
+  /// file sizes.


Suggested change

/// If any of the file is larger than maximum file size allowed by the

/// server, an error dialog is shown mentioning their names and actual

/// file sizes.

/// If any of the files are larger than maximum file size allowed by the

/// server, an error dialog is shown mentioning their names and actual

/// file sizes.

chrisbobbe · 2025-08-04T19:40:14Z

android/app/src/main/kotlin/com/zulip/flutter/AndroidIntentEventListener.kt

+    }
+
+    if (extraText == null && extraStream == null) {
+      throw Exception("Got unexpected ACTION_SEND* intent, without both EXTRA_TEXT and EXTRA_STREAM")


Suggested change

throw Exception("Got unexpected ACTION_SEND* intent, without both EXTRA_TEXT and EXTRA_STREAM")

throw Exception("Got unexpected ACTION_SEND* intent, with neither EXTRA_TEXT nor EXTRA_STREAM")

(It's OK if the intent doesn't have both things, but not OK if it has zero of them.)

alya · 2025-08-04T22:53:26Z

Looks like we show the "Unpinned" header unconditionally on Subscription list page:

I don't think we want that on that page, either. That particular UI is going away anyway in #1765, so we can leave it be for now.

rajveermalviya · 2025-08-05T14:59:47Z

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

gnprice · 2025-08-05T17:47:43Z

Just pushed one additional commit written on my call with @rajveermalviya just now:
fd72c44 (squash) move onChannelSelect default up to public widget

rajveermalviya · 2025-08-05T22:55:27Z

Thanks for demonstrating that @gnprice! Squashed that commit into 8e077ee.

chrisbobbe · 2025-08-06T22:48:02Z

Bump on #1774

Interesting. What goes wrong without this change, if the user does try to open a channel action sheet?

What if we just let the user open the channel action sheet? In future it'll show the channel's description, which could help the user decide which channel is the right one.

Also, if we're suppressing the channel action sheet, I think we'll want a TODO(#1272) to suppress the DM-conversation action sheet too, once that exists (#1272)? (But also, what if that's the only way to see the full identity of the conversation, i.e. all the recipients…?)

chrisbobbe

Thanks! Small comments on the safe-area commits, and a bump above on the question about the channel action sheet.

chrisbobbe · 2025-08-06T23:14:10Z

lib/widgets/subscription_list.dart

+          // This ensures last item in scrollable can settle in an unobstructed area.
+          const SliverSafeArea(sliver: SliverToBoxAdapter(child: SizedBox.shrink())),


How about

// This ensures last item in scrollable can settle in an unobstructed area. // (Noop in the home-page case; see comment on `bottom: false` above.

When related safe-area code is separated by 20+ lines of code, it can be hard to see how it works together. When looking at a given place in the code, it can be hard to know the safe-area requirements exactly, and it gets harder when we make incoherent or incorrect changes by mistake…which was the situation that led to 742320c, and which we almost made in this PR by forgetting to do the partial-revert-and-amend of that commit :)

chrisbobbe · 2025-08-06T23:15:40Z

lib/widgets/recent_dm_conversations.dart

+            //
+            // Other *PageBody widgets don't handle this because they aren't
+            // planned to be (re-)used outside the context of the home page.
+            bottom: false,


recent dms [nfc]: Handle bottom insets explicitly in recent DMs page This reverts part of https://github.com/zulip/zulip-flutter/commit/742320ce7f190d8506a15b13deb8e00421eb0658 for RecentDmConversationsPageBody, and also handles bottom insets for the list view and the new DMs button. As this widget is planned to be used outside the context of home page, specifically for the upcoming share page.

Commit-message nit: delete "bottom insets for the list view"; this is covered by "This reverts part of [etc.]".

chrisbobbe · 2025-08-06T23:17:18Z

lib/widgets/recent_dm_conversations.dart

            child: ListView.builder(
-              padding: EdgeInsets.only(bottom: 90),
+              padding: EdgeInsets.only(bottom: MediaQuery.paddingOf(context).bottom + 90),


nit: As in previous commit, comment on MediaQuery.paddingOf(context).bottom saying it's 0 in the case with bottom tabs (maybe make a helper variable, since this value is used twice?) and referring to your longer explanation

gnprice · 2025-08-07T00:05:20Z

lib/widgets/recent_dm_conversations.dart

+          child: _NewDmButton(onDmSelect: (narrow) {
+            if (widget.onDmSelect case final onDmSelect?) {
+              // Pop the new DMs action sheet.
+              Navigator.pop(context);
+              onDmSelect(narrow);
+            } else {
+              _handleDmSelectForNewDms(narrow);
+            }


nit: clearest to have this logic live on a method, I think.

Specifically can move this logic into _handleDmSelectForNewDms, since that method doesn't have much other complexity.

This will be used soon to access `MessageListPageState` after routing to `MessageListPage`.

… action sheet This will be used soon to avoid unintended flows when sharing content received from other apps.

…an't post This will be used soon to avoid showing conversations from the channel list where the current user can't post, specifically when they don't have the permission.

…the selected channel This will be used soon to provide specific behaviour when selecting a channel, where if specified it will replace the default behaviour of routing to the message list page of the selected channel narrow.

This will be used soon to avoid showing conversations from the recent DMs list where the current user can't post, specifically when a conversation has one or more deactivated user.

This will be used soon to provide a specific behaviour when selecting a DM, where if specified it will replace the default behaviour of routing to the message list page of the selected DM narrow.

This will be used soon to allow uploading files directly via using `MessageListPageState`.

…ion list page This reverts part of 742320c for SubscriptionListPageBody, as this widget is planned to be used outside the context of home page, specifically for the upcoming share page.

This reverts part of 742320c for RecentDmConversationsPageBody, and also handles bottom insets for the new DMs button. As this widget is planned to be used outside the context of home page, specifically for the upcoming share page.

Enables the app to receive arbitrary content from other apps via advertising Android Intent filters in AndroidManifest. It allows the OS to list our app in the platform share sheet. Adds handlers for the two Intent actions, namely SEND and SEND_MULTIPLE. Handling all three possible combinations: - Receiving only a text - Receiving only a file (or multiple files in case of SEND_MULTIPLE) - Receiving both the file (or multiple) and the accompanying text. The Android side Kotlin implementation is adapted from the legacy app's implementation, with only difference being that the legacy app didn't handle the 3rd case mentioned above, see: https://github.com/zulip/zulip-mobile/blob/eb8505c4a/android/app/src/main/java/com/zulipmobile/sharing/SharingHelper.kt To allow sending Android Intent events from Kotlin to Dart, Pigeon's EventChannelApi is used. For which the registration happens in `MainActivity.configureFlutterEngine`, this bit of code was adapted from Pigeon's Android example, see: https://github.com/flutter/packages/blob/b2aef15c1/packages/pigeon/example/app/android/app/src/main/kotlin/dev/flutter/pigeon_example_app/MainActivity.kt#L109-L121

rajveermalviya · 2025-08-11T11:40:36Z

@chrisbobbe said:

Bump on #1774 (comment)

Interesting. What goes wrong without this change, if the user does try to open a channel action sheet?

What if we just let the user open the channel action sheet? …

The reason for disabling the channel action sheet, was that user could open the topic list from there and if tapped on a topic, the compose box in the topic narrow message list wouldn't be populated with the shared content. See discussion: #mobile-design > share to Zulip @ 💬.

The latest revision now hides the topic list button from the channel action sheet (when on share page), instead of disabling the whole channel action sheet.

rajveermalviya · 2025-08-11T11:43:41Z

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

gnprice reviewed Jul 31, 2025

View reviewed changes

rajveermalviya force-pushed the pr-share-to-zulip branch 7 times, most recently from 78f40a3 to 0c75b06 Compare August 1, 2025 05:48

rajveermalviya marked this pull request as ready for review August 1, 2025 06:06

rajveermalviya requested a review from gnprice August 1, 2025 06:07

rajveermalviya mentioned this pull request Aug 1, 2025

[Android] Allow selecting an account when sharing to Zulip from other apps #1779

Open

rajveermalviya force-pushed the pr-share-to-zulip branch from 0c75b06 to 8f0e582 Compare August 1, 2025 06:41

gnprice reviewed Aug 1, 2025

View reviewed changes

rajveermalviya added maintainer review PR ready for review by Zulip maintainers integration review Added by maintainers when PR may be ready for integration labels Aug 1, 2025

rajveermalviya requested a review from chrisbobbe August 1, 2025 17:51

rajveermalviya force-pushed the pr-share-to-zulip branch from 8f0e582 to 93f4417 Compare August 1, 2025 18:34

chrisbobbe reviewed Aug 2, 2025

View reviewed changes

rajveermalviya force-pushed the pr-share-to-zulip branch from 93f4417 to 7bbee53 Compare August 3, 2025 01:48

rajveermalviya requested a review from chrisbobbe August 3, 2025 02:10

chrisbobbe reviewed Aug 4, 2025

View reviewed changes

rajveermalviya force-pushed the pr-share-to-zulip branch from 7bbee53 to 9080b78 Compare August 5, 2025 14:53

rajveermalviya requested a review from chrisbobbe August 5, 2025 14:59

rajveermalviya mentioned this pull request Aug 5, 2025

Write tests for Share-to-Zulip (bindings, and UI) #1787

Open

rajveermalviya force-pushed the pr-share-to-zulip branch from fd72c44 to 09a4f30 Compare August 5, 2025 22:52

chrisbobbe reviewed Aug 6, 2025

View reviewed changes

gnprice reviewed Aug 7, 2025

View reviewed changes

rajveermalviya added 2 commits August 11, 2025 13:32

compose [nfc]: Export and rename _File to FileToUpload

8a3bd63

msglist [nfc]: Allow passing key in MessageListPage.buildRoute

299bd86

This will be used soon to access `MessageListPageState` after routing to `MessageListPage`.

rajveermalviya force-pushed the pr-share-to-zulip branch 3 times, most recently from c7d3804 to 66baf97 Compare August 11, 2025 10:37

rajveermalviya added 8 commits August 11, 2025 16:30

subscription_list [nfc]: Allow disabling topic list button in channel…

0abdd74

… action sheet This will be used soon to avoid unintended flows when sharing content received from other apps.

subscription_list [nfc]: Add a flag to hide channels where the user c…

166d110

…an't post This will be used soon to avoid showing conversations from the channel list where the current user can't post, specifically when they don't have the permission.

recent dms [nfc]: Add a flag to hide DMs where user can't post

ef6dcf6

This will be used soon to avoid showing conversations from the recent DMs list where the current user can't post, specifically when a conversation has one or more deactivated user.

recent dms [nfc]: Allow onDmSelect callback, notifying the selected DM

412f911

This will be used soon to provide a specific behaviour when selecting a DM, where if specified it will replace the default behaviour of routing to the message list page of the selected DM narrow.

compose [nfc]: Expose uploadFiles on ComposeBoxState

fb4d254

This will be used soon to allow uploading files directly via using `MessageListPageState`.

subscription_list [nfc]: Handle bottom insets explicitly in subscript…

37f71c6

…ion list page This reverts part of 742320c for SubscriptionListPageBody, as this widget is planned to be used outside the context of home page, specifically for the upcoming share page.

rajveermalviya force-pushed the pr-share-to-zulip branch from 66baf97 to 45b9b60 Compare August 11, 2025 11:12

rajveermalviya force-pushed the pr-share-to-zulip branch from 45b9b60 to 22afc6b Compare August 11, 2025 11:20

rajveermalviya requested a review from chrisbobbe August 11, 2025 11:43


		class MainActivity: FlutterActivity() {
		class MainActivity : FlutterActivity() {

		ZulipBinding.instance.androidIntentEvents.forEach((androidIntentEvent) async {
		assert(debugLog('androidIntentEvent.action: ${androidIntentEvent.action}'));

		Navigator.pop(context); // TODO why doesn't this work?
		onSelect!(narrow);

		// TODO separate out API calls for resolving file name, getting mimetype, getting bytes?
		class IntentSharedFile {

	throw Exception("Got unexpected ACTION_SEND* intent, without both EXTRA_TEXT and EXTRA_STREAM")
	throw Exception("Got unexpected ACTION_SEND* intent, with neither EXTRA_TEXT nor EXTRA_STREAM")

		// This ensures last item in scrollable can settle in an unobstructed area.
		const SliverSafeArea(sliver: SliverToBoxAdapter(child: SizedBox.shrink())),

Support Share to Zulip on Android #1774

Are you sure you want to change the base?

Support Share to Zulip on Android #1774

Conversation

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

Uh oh!

Screenshots

Screen recordings

Uh oh!

gnprice commented Jul 31, 2025

Uh oh!

gnprice 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!

alya commented Jul 31, 2025

Uh oh!

rajveermalviya commented Aug 1, 2025

Uh oh!

rajveermalviya commented Aug 1, 2025

Uh oh!

alya commented Aug 1, 2025

Uh oh!

rajveermalviya commented Aug 1, 2025

Uh oh!

gnprice commented Aug 1, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

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!

chrisbobbe Aug 1, 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

rajveermalviya commented Jul 31, 2025 •

edited

Loading

chrisbobbe Aug 1, 2025 •

edited

Loading

chrisbobbe Aug 2, 2025 •

edited

Loading