invertase / notifee

⚛️ A feature rich notifications library for React Native.
https://notifee.app
Apache License 2.0
1.88k stars 226 forks source link

Local notification with local image #1142

Open trajano opened 1 week ago

trajano commented 1 week ago

https://github.com/invertase/notifee/issues/931 talks about a remote image.

For now I just want to download an image store it in the "cache" folder so the attachment looks like this

[
  {
    "id": "633435dcb418833920a16771610ca404", 
    "thumbnailHidden": false,
    "thumbnailTime": 0, 
    "typeHint": "public.png", 
    "url": "file:///var/mobile/Containers/Data/Application/082B40E9-4209-4FC3-9165-BD7C57591E47/Library/Caches/ExponentAsset-633435dcb418833920a16771610ca404.png"
  }
]

I've tried different variants removing values for ID type etc. With no luck

But the thumbnail image is not appearing

requires approach does work though.

Looking at the examples folder you're using a remote image.

Code https://github.com/trajano/expo-experiments/blob/test-expand-idea/packages/my-app/src/stories/Notification.stories.tsx

mikehardy commented 1 week ago

seems to be well supported at least in theory

https://github.com/invertase/notifee/blob/ae2953b7d0a0881f7bb925f0f5d0cf1d3ac9daf4/packages/react-native/src/types/NotificationIOS.ts#L624C69-L624C161

as long as it is a local file of supported type and within size limits (https://developer.apple.com/documentation/usernotifications/unnotificationattachment#1682051)

I assume it works if the file is in the bundle? it's just downloading a local file first then trying to display it that is not working?

Adding instrumentation logs here and running from Xcode may be instructive https://github.com/invertase/notifee/blob/ae2953b7d0a0881f7bb925f0f5d0cf1d3ac9daf4/ios/NotifeeCore/NotifeeCoreUtil.m#L151

There is not a lot of logic between the line where files are resolved and content is packaged then sent to the iOS notification center here https://github.com/invertase/notifee/blob/ae2953b7d0a0881f7bb925f0f5d0cf1d3ac9daf4/ios/NotifeeCore/NotifeeCore.m#L181

trajano commented 1 week ago

It's failing with a copy from the bundle. If we use the "bundle" via require it works. The image itself is small, but there's a small hint in that document you provided

https://github.com/invertase/notifee/blob/ae2953b7d0a0881f7bb925f0f5d0cf1d3ac9daf4/packages/react-native/src/types/NotificationIOS.ts#L616-L626

The key part is absolute path meaning the uri which I normally pass in as file:///... may have an issue

trajano commented 1 week ago

Thar we go

    return assets
      .filter((it) => it)
      .map((it) => ({
        id: it.hash!,
        thumbnailHidden: false,
        typeHint: `public.${it.type}`,
        url: it.localUri!.substring('file://'.length),
      }));

The documentation should note or at least specify that a url is a PATH and file:// shouldn't prefix it.

In case anyone is wondering... "expo-notifications" does not work even with the removal of the file:// prefix

But this will break on the second usage of the notification with the same URL because the file moves as alluded to by iOS documentation, knew that before hand so just have to tweak the thing a bit

trajano commented 1 week ago

So I have a work around right now where I clone it on behalf of notifiee

https://github.com/trajano/expo-experiments/blob/076d342a55da8d01a5815e0bfc51da205ef32cd8/packages/my-app/src/stories/Notification.stories.tsx#L32-L63

https://github.com/trajano/expo-experiments/blob/076d342a55da8d01a5815e0bfc51da205ef32cd8/packages/my-app/src/stories/Notification.stories.tsx#L87-L96

https://github.com/trajano/expo-experiments/blob/076d342a55da8d01a5815e0bfc51da205ef32cd8/packages/my-app/src/stories/Notification.stories.tsx#L126-L127

But it would be nice if notifee itself handles those logistics of copying the files and stripping off file:// on it's end because for Expo devs they use URIs exclusively and not file paths.

mikehardy commented 1 week ago

@trajano that's fantastic you got something working! That's always the first step :-) - I think you're easily the subject matter expert here at the moment, I'd be happy to merge any reasonable PR that improved this area, and the release pipeline has been exercised recently + commit queue is clean so getting it out would be quick