novoda / download-manager

A library that handles long-running downloads, handling the network interactions and retrying downloads automatically after failures
Apache License 2.0
483 stars 63 forks source link

Is there a SINGLE_DISMISSIBLE_NOTIFICATION ? #474

Closed mspmax closed 5 years ago

mspmax commented 5 years ago

Hi Guys,

This is a quick one. Is there a way I can implement single notification with dismissible action using the NotificationCustomizer ?

Thanks in advance!

Mecharyry commented 5 years ago

Hi @mspmax, assuming it can be done with the NotificationCompat.Builder builder found here you shouldn't have a problem that I can foresee. @zegnus is that correct?

mspmax commented 5 years ago

hi @Mecharyry, thank you for your reply, I'm doing something similar,

@Override
    public NotificationDisplayState notificationDisplayState(DownloadBatchStatus payload) {
        switch (payload.status()) {
            case DOWNLOADED:
            case ERROR:
                return NotificationDisplayState.STACK_NOTIFICATION_DISMISSIBLE;
            case DOWNLOADING:
            case PAUSED:
            case WAITING_FOR_NETWORK:
                // user will not be able to dismiss the notification during this state
                return NotificationDisplayState.SINGLE_PERSISTENT_NOTIFICATION;
            case UNKNOWN:
            case QUEUED:
            case DELETING:
            case DELETED:
            default:
                return NotificationDisplayState.HIDDEN_NOTIFICATION;
        }
    }

Basically, I don't want all the completed batches to stack as well as be single persistant cause user should have the option to dismiss when download is completed. Does it make sense ? :D let me know if you need more info.

cheers !

Mecharyry commented 5 years ago

Oh I see @mspmax, you are asking for NotificationDisplayState. SINGLE_DISMISSIBLE_NOTIFICATION right?

I just want to clarify the rest before I formulate a response: You don't want to stack DOWNLOADED statuses? So if you receive two DOWNLOADED the first will be automatically dismissed and the second will be present for the user to dismiss if they want?

mspmax commented 5 years ago

@Mecharyry that's exactly what I meant :)

Mecharyry commented 5 years ago

Excellent @mspmax. Now I can help 😄 so as far as I know client applications would not be able to do this. The class that happens to handle the logic regarding notification dispatching is [here].(https://github.com/novoda/download-manager/blob/release/library/src/main/java/com/novoda/downloadmanager/ServiceNotificationDispatcher.java)

It seems reasonable to assume that we can make a change and have this work. We would need to call NotificationManager.cancelAll whenever we receive NotificationDisplayState.SINGLE_DISMISSIBLE_NOTIFICATION. I don't think this would kill the persistent notification because it is bound to a Service but it is something to be aware of.

If you would like to contribute to adding this feature to the library that would be amazing. In terms of the maintainers working on this fix, we can't guarantee that this will be personally worked on in the short term.

Mecharyry commented 5 years ago

Hi @mspmax I think we might need some more information. There will always be at least one persistent notification, are you aware of this? We absolutely have to keep at least one persistent notification because of the constraints on running a Foreground Service.

If you don't want stacked notifications we can remove the previous but you will always have a minimum of two notifications, one for the ongoing downloads (DOWNLOADING) and one for the DOWNLOADED.

Can you let me know what your intention is, Thanks!

mspmax commented 5 years ago

hi @Mecharyry thanks for the reply, It's exactly what I was referring to. I'm aware of the fact that while in progress we have to have a persistent notification so a minimum of 2 is really fine. Thanks!

Mecharyry commented 5 years ago

@mspmax Let me know if this serves your purpose -> https://github.com/novoda/download-manager/pull/477

Mecharyry commented 5 years ago

@mspmax would you be able to test the snapshot to see if it suits your purpose -> https://bintray.com/novoda/snapshots/download-manager/SNAPSHOT-32

We won't perform a release of it until later this week, after you and I have tested it in production.

Mecharyry commented 5 years ago

Released in 2.3.0, closing.

mspmax commented 5 years ago

Hey @Mecharyry thank's for the feature! really appreciate it 🥇