[flutter_local_notifications] Implement options to hide or crop attachments in the thumbnail on iOS #1785
Conversation
|
@MaikuB would you be so kind and take a look at this PR? I know that some tests are failing, but I believe it's not caused by this commit. |
|
Can you merge the latest changes from master to your branch/fork? This would fix the Kotlin plugin issue and allow seeing if tests are failing or not though you should also be able to run the unit tests locally without updating and from what I can see a number of them are failing |
flutter_local_notifications/lib/src/platform_specifics/darwin/notification_attachment.dart
Show resolved
Hide resolved
noinskit
force-pushed
the
darwin-attachments
branch
5 times, most recently
from
afe186a
to
b18e74d
Compare
|
All tests are passing now (after rebase and also actually modifying the tests). There's one more thing: I modified |
|
Good call out as I believe the API being used is also supported on macOS and my stance is that macOS support should be added for situations like this. On that note, are you able to make the macOS changes too or would you need support for that? |
I wanted to just implement it, but I need help after all. I started the "example" app on my Mac (Ventura 13.0) and all the buttons silently do nothing, including |
|
I'd add that I've found #1610 and tried a bunch of things to see a system dialog to ask me for permissions, incl. deleting |
|
Figured it out and will submit a PR to fix this soon. It looks like requesting critical alert permissions has prevented the permissions dialog appearing on macOS but was fine for me on iOS. Not sure why the behaviour is inconsistent though I know this permission requires a special entitlement from Apple so this may be related |
|
If you update your branch/fork from the master branch then this should be in the change since #1809 is merged in |
noinskit
force-pushed
the
darwin-attachments
branch
from
b18e74d
to
f6c4328
Compare
Thanks, I got the permission dialog now and I can trigger notifications! I'm looking at the thumbnail options on macOS now... |
noinskit
force-pushed
the
darwin-attachments
branch
from
f6c4328
to
cc0373b
Compare
|
I pushed a version with macOS support. BUT it's not really working for me on my laptop, even though as far as I can tell it's being passed correctly (I looked at debug prints of the constructed |
|
No worries, I'll take a look and see if I spot anything |
|
Whilst I take a look at this, could you add public API docs? The linter rules should highlight the ones that are missing |
|
Took a look and ran into same problem. I don't think there's anything wrong with what was implemented. The only thing that came to mind that didn't change the result is that the official docs for UNNotificationAttachmentOptionsThumbnailClippingRectKey say to use this to create the dictionary representation. Although it seems to have not made a difference, perhaps the code should change to use that function? |
noinskit
force-pushed
the
darwin-attachments
branch
2 times, most recently
from
5ab53c1
to
1bf870f
Compare
|
Sure, I made both changes! Still no impact on how the notification is shown on macOS, though :( |
|
Or maybe not, looks like tests are now failing on: I'm almost sure it worked for me locally; I'm guessing it's about compiler settings. |
|
Yeah it worked locally for me too. Perhaps the docs are out of date given the message. Looks like that particular change would need to be reverted as it sounds like what you did is more correct. On a different note, in case you were squashing commits out of concern that it would affect the commit history when merged then don't worry about that GitHub provides the ability to do the maintainer's end before merging and it's what I select as the option too |
noinskit
force-pushed
the
darwin-attachments
branch
from
1bf870f
to
de5a023
Compare
|
Sure, it's back to |
flutter_local_notifications/ios/Classes/FlutterLocalNotificationsPlugin.m
Show resolved
Hide resolved
…_flutter_local_notifications * commit '4b723e750d1371206520b10a122a444c4bba7475': (76 commits) [flutter_local_notifications] update docs on initialize method to describe callbacks more (MaikuB#1841) enable usePubspecOverrides with melos (MaikuB#1822) replaced placeholder.com with dummyimage.com in example app due to Cloudflare blocking requests (MaikuB#1821) Google Java Format [flutter_local_notifications] Fixes MaikuB#1486 :Adds ability to count down chronometer with chronometerCountDown (MaikuB#1778) Implement options to hide or crop attachments in the thumbnail on iOS (MaikuB#1785) bump linux plugin's Flutter version requirement to 3.0.0 and add explicit ffi dependency (MaikuB#1812) [flutter_local_notifications] re-add imports needed for Flutter 3.0 and add build tasks that use Flutter 3.0 (MaikuB#1811) updated example app to not request the ability to display critical alerts (MaikuB#1809) Add note about keeping `@mipmap/ic_launcher` resource (MaikuB#1804) Recommend WindowManager to fix Android 12L+ bugs (MaikuB#1803) [flutter_local_notifications] Upgraded Android Gradle Plugin to fix Android build (MaikuB#1781) [flutter_local_notifications] fixes parsing of callback handles for notification actions on Android (MaikuB#1798) [flutter_local_notifications] fix Kotlin version used in example app (MaikuB#1791) Updates readme for iOS Setup (MaikuB#1776) added missing kudos for contributions relating to flutter_local_notifications 12.0.3 release release flutter_local_notifications 12.0.3 updated code snippet in readme on requesting permissions on Android (MaikuB#1754) Google Java Format Drop registerWith in Android plugin (MaikuB#1745) ...
This PR implements access to two platform-specific features for customizing attachments to notifications (c.f. https://developer.apple.com/documentation/usernotifications/unnotificationattachment?language=objc). Without them it's very hard to use image attachments at all, because by default the first attachment is used both for a tiny thumbnail (smaller than app logo) and a potentially full screen image on an expanded notification.