MaikuB / flutter_local_notifications

A Flutter plugin for displaying local notifications on Android, iOS, macOS and Linux
2.41k stars 1.36k forks source link

Support for remote images for largeIcon #1122

Closed vytautas-pranskunas- closed 2 years ago

vytautas-pranskunas- commented 3 years ago

Hi,

I am opening this issue in favor of closed one here: https://github.com/MaikuB/flutter_local_notifications/issues/1110

I would like to add remote avatar image to notification which is specified in https://developer.android.com/training/notify-user/expanded#large-style.

However, for now i see only 2 types are supported bu this library:

I would like to ask to add remote image support similar to:

String name = remoteMessage.getNotification().getIcon();
URL url_value = new URL(name);
Bitmap mIcon1 = BitmapFactory.decodeStream(url_value.openConnection().getInputStream());

AndroidNotificationDetails(
              channel.id,
           ...
.setLargeIcon(mIcon1) <-------------------
....
MaikuB commented 3 years ago

If you need support for it then you'll need to work on it. In general, if a feature request is made then I ask those requesting it to work on it and this is one feature that someone else has requested that I've asked for PR to be submitted for (see https://github.com/MaikuB/flutter_local_notifications/issues/479). As I've asked for PRs on this before, if you won't be working on it then I'll be closing this

vytautas-pranskunas- commented 3 years ago

I understand that people contribution to libraries like this is very welcome but what you are saying is: 1) You create library and you do not care if anybody, who trusted your plugin and depend on it, requests new feature which would do good impact on it. 2) You assume that everybdy has knowledge about library, techniques, (especially here channels and push notification behaviour) and everybody can implement it 3) For you to add this support would take for example 1 hour and for others 20 hours (to get familiar wih code base, setup, test) 4) Everybody have enough time - i thought libraries are ment to save time...

I think with such attitude thi slibrary has future only until people realize what is what - but if you do not care so then you do not care :)

p.s. I made multiple PR's to multiple flutter libraries - but I do not know if have enough knowedge and time to add right support for images to notifications. Maybe - but with tide deadlines that i have on my current project it just not possible.

p.p.s - closing feature requests just because reporter did not have enought time or knowledge - does not feel right - because it is still GOOD IDEA keeping in mind that official google docs supports it.

Take Care

MaikuB commented 3 years ago

I had mentioned that I ask people to submit PRs in general (emphasis mine). This means there are cases I will look at features. In this case though, I had pointed out in the previous thread I had linked to that I'm not familiar with what's involved to do implement the feature.

With regard to everything else you wrote, libraries may save time but not cover every scenario so having to look at implement the gaps specific to your scenarios becomes very common. This is something that should be assessed as part of undertaking work (e.g. via technical feasibility analysis). If it can't be done then you need to consider descoping a feature.

If you have a feature you need then seems reasonable to ask you to work on to contribute back when the library is provided freely. Some are likely even using this library for commercial purposes. This is a project I work on in my spare time. In fact, I don't even use this library nor was I was familiar with coding in Java/Kotlin or Objective-C/Swift when I started working on this though I was familiar with some of the APIs due to experience with Xamarin. Which brings me to something related to this that you talked a lot about your own time and priorities and neglected to consider that I have own priorities as well. Rather than trying to find out more out about my reasons, you've made assumptions about them, which includes the assumption that I won't help others with getting their PR to the repo.

p.p.s - closing feature requests just because reporter did not have enough time or knowledge - does not feel right - because it is still GOOD IDEA keeping in mind that official google docs supports it

What docs? The only docs that would be relevant are ones for native Android APIs. Given you expressed your lack of knowledge, how do you know this is even supported? You cannot say for certain unless you have worked with them.

vytautas-pranskunas- commented 3 years ago

What docs? I do not to argu is it my lack of knowledge or not but it was you who pinted out that it is supported:

Sounds like you are still describing the big picture style. If you look at the example where they specify an avatar (https://developer.android.com/training/notify-user/expanded#large-style), it does not make use of that style. The avatar is

in ticket: https://github.com/MaikuB/flutter_local_notifications/issues/1110

Also the code peace I have added above shows that it is supported bu google and flutter.

p.s. I also maitain few libraries during my spare time and fully understand you.

MaikuB commented 3 years ago

To quote your original message in the other thread, it was

I have looked example app it supports only big image. It would be great if you can bring easy way to customize layout or support avatar images

I posted the link only to show you that pictures of people can be shown as it looked like you were looking only looking at the big picture style, which is why the shape and size didn't match your expectation. They show an avatar but there's no mention of where the avatar is sourced from as the code simply calls a method (emailObject.getSenderAvatar()) to get an image and its implementation is unknown

Regarding that code you posted, it's not anything I'm familiar with as I explained before. URL isn't a Flutter/Dart class, so I assume it refers to an Android API. It's not to do with Flutter. Also, if you look at the previous request for this, I had closed it after a month as such I doubt anyone else will work on this as this is the second request for it and occurred a year later. I mentioned that I would close this based on past experience on this request and my own experience from maintaining the repository in general e.g. there was an issue with dealing with scheduled notifications in locations with daylight savings for over a year or so with no assistance in getting it resolved or in getting feedback on what I thought would've solved the issue in a prerelease.

I could leave it open for longer but I doubt others in the community will work on it and there are decent number of members of the community who comment on closed issues

vytautas-pranskunas- commented 3 years ago

Maybe you will have time and will to implement it because showing static images are not so fancy, Fancy is when you can send imageUrl across and display it because then you can display sender avatars.

Nealsoni00 commented 3 years ago

You can display an image from a URL without any code modification by downloading the remote image and saving it to the phone. This is actually done in the example project:

https://github.com/MaikuB/flutter_local_notifications/blob/f9296ca155d6263421da9f637cb77da2aeb075e0/flutter_local_notifications/example/lib/main.dart#L837-L844

This is not the best solution by any means but it works. It would be better to support directly fetching the image from the URL and then displaying the notification — without saving to disk — but this is a workaround.

Complete code:

import 'package:path_provider/path_provider.dart';
import 'package:http/http.dart' as http;

Future<String> _downloadAndSaveFile(String url, String fileName) async {
  final Directory directory = await getApplicationDocumentsDirectory();
  final String filePath = '${directory.path}/$fileName';
  final http.Response response = await http.get(Uri.parse(url));
  final File file = File(filePath);
  await file.writeAsBytes(response.bodyBytes);
  return filePath;
}

Future<void> showNotification() {
  final imageUrl = 'https://via.placeholder.com/48x48'
  final String? largeIconPath =  await _downloadAndSaveFile(imageUrl, 'largeIcon')
  final localNotifPlugin = FlutterLocalNotificationsPlugin();
  localNotifPlugin.show(
      1, "TITLE", "BODY",
      NotificationDetails(
        android: AndroidNotificationDetails(
            //...
            largeIcon: FilePathAndroidBitmap(largeIconPath)
        ),
      )
  );
}
vytautas-pranskunas- commented 3 years ago

@Nealsoni00 I did not mean to have large image. This topic is different. p.s. saving every image to the client - terrible idea. You are basically trashing people phones. But thanks for your input

lexxxel commented 2 years ago

@vytautas-pranskunas- what do you think about my solution using base64 strings? I know base64 generates some overhead, but at least its easy to get that string generated on flutter side

vytautas-pranskunas- commented 2 years ago

@lexxxel I do not think this is good idea because push notifications cannot send large data. If you are talking about local images converted to base63 then also push notifications image idea is different. Push notifications should be able to display remote images (you will not have all your user photos inside of app dont you?). So basically push notification should receive image url and should be able to display it.

lexxxel commented 2 years ago

@vytautas-pranskunas- I'm a bit puzzled. I could not translate all of your comment. FCM allows up to 4kB of payload. This should be enough for JPEGs of at least 128x128 without tuning. For large icons in simple apps this can already be sufficient.

If you are talking about local images converted to base63 then also push notifications image idea is different.

I don't get what you want to say. Of course, one could 'serialize' images received from some URL to base64, e.g. to cache user avatars in a client side sqlite db. To be able to get the base64 string from a local db and then show them without waiting them onto a file make sense to me.

Push notifications should be able to display remote images (you will not have all your user photos inside of app dont you?).

How does WhatsApp or Signal solve this issue? Do they send the photo URL of the user that send you a message every time you get a notification. Maybe, but I doubt that the app does download the image from the URL every single time. For sure there is at least some caching applied. Also remote images are also possible via base64. There is no limitation to where the data for the image Congress from. See my PR:

Future<String> _base64encodedImage(String url) async {
    final http.Response response = await http.get(Uri.parse(url));
    final base64Data = base64Encode(response.bodyBytes);
    return base64Data;
}
vytautas-pranskunas- commented 2 years ago

Hi @lexxxel

How does WhatsApp or Signal solve this issue? Do they send the photo URL of the user that send you a message every time you get a notification. Maybe, but I doubt that the app does download the image from the URL every single time. For sure there is at least some caching applied.

I am not sure how whatsup solves this but whatsup is a bit different because it takes photo from the phone since it is connected to your contacts. But I push notifications supports image property in notification body. You can read more about it here: https://firebase.google.com/docs/cloud-messaging/android/send-image

For sure there is at least some caching applied.

I am sure that when you receive image you can apply caching.

Still not usre why you need base64 images... What is the advantage? I see this as a limitation because imageUrl covers all scenarious: 1) You can suply image from your DB 2) You can supply image from internet 3) ...

lexxxel commented 2 years ago

Still not usre why you need base64 images...

The advantage is, that it was easy to implement within the current code base without larger changes. Everything stays compatible and only a new way is added. For an byte arrays exchange approach, I can't say that for sure right now. When merged, it gives you a way to add images to notifications without writing them into a file. That's it. Yes, it generates overhead to convert from a byte list to base64 string, BUT it solves your initial issue:

I would like to add remote avatar image to notification which is specified in https://developer.android.com/training/notify-user/expanded#large-style.

To get bitmaps into the library requires a lot more time an effort. It might come in the future eventually. The base64 images PR is here, today.

Also if you send an image or an imageUrl makes no difference for the base64 approach. As long as you get a correct string from somewhere it works like you can see in the example.

vytautas-pranskunas- commented 2 years ago

@lexxxel why we cannot add option to pass URL and under the hood convert it to base64 before display by using this?

Future<String> _base64encodedImage(String url) async {
    final http.Response response = await http.get(Uri.parse(url));
    final base64Data = base64Encode(response.bodyBytes);
    return base64Data;
}

If there are some limitations or requirements of huge refactoring then your approach also works for me :) it is better to have something rather then nothing :)

lexxxel commented 2 years ago

why we cannot add option to pass URL and under the hood convert it to base64 before display by using this?

This is the next step, but with byte arrays directly instead of bas64 strings. One would like to do the under the hood image download on the flutter side to be able to react on network errors. I can not give a timeframe because I'm on vacation right now. But I require this feature, too.

vytautas-pranskunas- commented 2 years ago

Ok, so let's merge this then. What stops you from merging? Will you create task for your self to add URL option?

RNerdMine commented 4 months ago

I Got issue like when app in background or terminated state then largeIcon and BigImage not show in notification. Please give Me solution for that.