flutter_local_notifications icon indicating copy to clipboard operation
flutter_local_notifications copied to clipboard

[flutter_local_notifications] Add support for Android Tiramisu' `POST_NOTIFICATIONS` permission

Open bartekpacia opened this issue 3 years ago • 6 comments

Implements two new methods on AndroidFlutterLocalNotificationsPlugin, requestPermission and isPermissionGranted, to let developers work with the new notification runtime permission added in Android 13 Tiramisu (API level 33)

Fixes #1597

bartekpacia avatar Jul 30 '22 01:07 bartekpacia

Locally Android integration tests pass when I use Flutter v3.0.5, but fail on Flutter v2.10.5.

bartekpacia avatar Jul 30 '22 13:07 bartekpacia

What's missing is returning a boolean result from requestPermission(). I'll implement this as well in this PR :)

bartekpacia avatar Jul 31 '22 11:07 bartekpacia

Thanks for the PR, was going to comment that it looks like that was missing when it comes to dealing with the listener.

Also the the Java code from the isPermissionGranted method remains after you had removed the code on the Flutter/Dart side.

I'll take a closer look once the changes are done

MaikuB avatar Jul 31 '22 11:07 MaikuB

Also just remembered about https://github.com/flutter/flutter/issues/12561 that can impact testing. A workaround is mentioned there

MaikuB avatar Jul 31 '22 11:07 MaikuB

@MaikuB Now AndroidFlutterLocalNotificationsPlugin.requestPermission() returns a Future<bool?> :)

bartekpacia avatar Aug 01 '22 00:08 bartekpacia

@MaikuB Thanks the for review, I'll make the changes soon :)

bartekpacia avatar Aug 04 '22 11:08 bartekpacia

Thanks for making the change. I pushed a grammar fix for the error message you added in so I can try to merge sooner. One thing that this made me think about though is the ability to use the existing initialize() method to also request permissions too. This is something that's possible for iOS and macOS. I can try to look at that after this PR is merged

MaikuB avatar Aug 17 '22 10:08 MaikuB

Thank you for your help with getting this merged @MaikuB :)

Regarding your idea with requesting permissions on initialize(), you have the example app in mind, not the plugin itself, right?

bartekpacia avatar Aug 17 '22 15:08 bartekpacia

No I'm referring to the plugin itself

MaikuB avatar Aug 17 '22 21:08 MaikuB

Thanks both for working on this! 👏🏻

Weighing in from the product side for requesting on initialize(), it's often important to prompt the user for notif permissions at a very specific step in the new user flow to optimize conversion for accepting. So I'd wanna make sure the plugin doesn't automatically prompt users for permission in a way that isn't within our control.

stephanie-finch avatar Aug 18 '22 03:08 stephanie-finch

If you look at how iOS/macOS works then there is control over that. This should be clearer if you look at the example app where it decides not to have permissions requested when calling initialize

MaikuB avatar Aug 18 '22 03:08 MaikuB