Replace flutter_local_notifications with a thin API binding using Pigeon

[gnprice]

For our initial implementation of notification UI #122 (introduced in #344), we use the package flutter_local_notifications. That's because it's what's currently recommended by Flutter upstream for showing a notification, and because I looked around a bit a few months ago and concluded that it was the best existing option for that.

As discussed at https://github.com/zulip/zulip-flutter/pull/344#discussion_r1375012273 and in chat, though, that package has quite a bit of baggage for functionality we'll never need, like timers. It also goes for an unfortunate pattern — reminiscent of React Native and its ecosystem, in fact — of providing a bunch of logic of its own, so that the API it presents is different from that of the underlying platform and adds its own quirks, bugs, and lack of documentation. For example, the issue I filed which is discussed at https://github.com/zulip/zulip-flutter/pull/344#discussion_r1375041646 .

So I don't find this package very satisfying, and I think it's likely that we'll ultimately want to rip it out in favor of something we write ourselves. The plan would be:

• Expose the platform APIs as thinly and faithfully as possible. Let the logic of deciding how to use those APIs stay in Dart.
• Use Pigeon to make the writing of those API bindings as easy and automated as possible.

[gnprice]

As described at https://github.com/zulip/zulip-flutter/issues/341#issuecomment-2014183497 , I started trying Pigeon today and wrote a draft branch that does this for the method to show a notification, as well as a couple of other methods we don't yet use in main. So I'll plan to clean that up and send it.

I might go ahead and cover the rest of the library (since what I already have is the most complex of the methods we use), or might leave that for later.

[rajveermalviya]

(Just another data point of why it would be better to have our custom platform integration for some APIs)

Yesterday I experimented with #128 and my draft branch uses flutter_local_notification because it provides all the functionality that is needed to implement #128.

But while implementing it, I thought it would also be great to have the notifications be in the "Conversations" space of the notification panel (available since Android 11), including with the new notification UI that "Conversations" space provides. To implement that we would first need to create dynamic "Conversation Shortcuts" (could be per Realm or per Stream) using ShortcutManagerCompat and then pass the shortcut-id when creating notification using setShortcutId, but flutter_local_notification doesn't yet provide this ability of setShortcutId.

You can read more about Conversation Shortcuts & Conversation Notifications here.

There are these two packages I found that provide a wrapper around ShortcutManagerCompat to manage conversation shortcuts: android_conversation_shortcut & flutter_shortcuts. But they aren't maintained anymore, so including the NotificationCompat we would also need custom platform integration for ShortcutManagerCompat.

[gnprice]

Yesterday I experimented with #128 and my draft branch uses flutter_local_notification because it provides all the functionality that is needed to implement #128.

Interesting, thanks! I'd be very glad to see that as a PR :smile:

(On a quick read-through, the one significant thing it looks like it needs is tests.)

But while implementing it, I thought it would also be great to have the notifications be in the "Conversations" space of the notification panel (available since Android 11), including with the new notification UI that "Conversations" space provides. […] You can read more about Conversation Shortcuts & Conversation Notifications here.

Hmm, yeah, that would be neat. And I agree, the way to do that will probably be via whipping up the platform integration ourselves.

[rajveermalviya]

I'd be very glad to see that as a PR

Sure, I plan to send it after your PR for replacing flutter_local_notification lands, to avoid re-doing that work on main.

Conversation Notification

Created a discussion for it on here

[gnprice]

OK, sent #592 for that conversion to Pigeon.

That PR doesn't completely remove package:flutter_local_notifications — but it replaces our use of its show method, which is by far our most complicated call site. It also sets up a pattern within which I think it should be fairly straightforward to add more methods in our new Pigeon-based plugin (at least where the underlying API is simple, anyway).

So I think that's ripe for you to experiment with rebasing your draft branch atop #592 and using the new mechanism: use androidNotificationHost.notify instead of notifications.show, add to that method any missing functionality you need, and also add to the new plugin any other methods you need. (Like getActiveNotifications.)

If you find rough edges in that flow or any questions come up, please comment on the PR or in a thread in #mobile-team. (For any back-and-forth discussion, probably a chat thread is best.)

[gnprice]

When we complete this, as a followup we should also try to remove the coreLibraryDesugaringEnabled line that was added in https://github.com/zulip/zulip-flutter/pull/887/commits/baed78b4224b33fd6cfef499b61e5ae29d89addd / #887. That's required by this package:flutter_local_notifications plugin, but due to a bug in com.android.tools:desugar_jdk_libs it causes warning noise for us at build time: https://github.com/zulip/zulip-flutter/pull/887#issuecomment-2287653388