kalu icon indicating copy to clipboard operation
kalu copied to clipboard

If a notification already exists, it should be replaced instead of a new one created

Open marmistrz opened this issue 6 years ago • 12 comments

Steps to reproduce

  1. Double click the kalu icon, wait for the notification to disappear
  2. Double click the kalu icon, wait for the notification to disappear

Expected outcome

The second notifications should have replaced the first

Actual outcome

There are two notifications in the notification list

Configuration

DE: Cinnamon

marmistrz avatar Sep 30 '18 09:09 marmistrz

hmm, I'm not sure about that. When you re-run checks (or re-show notifications) it means to create new notifications. Especially since whether or not the previous ones still exist/are kept in a list or not really depends on your notification daemon -- for instance, for me once a notification disappeared it is gone, so there's nothing to be replaced.

jjk-jacky avatar Sep 30 '18 17:09 jjk-jacky

I mean using notification.update() on the existing notification, see this link, chapter Re-using Notification Object This (at least on my system) updates the existing notification if has not been dismissed and creates a new one otherwise.

marmistrz avatar Oct 01 '18 18:10 marmistrz

Yes, I understood what you meant, I'm just not sure this would be the right thing to do. What I'm (trying to) say is that it doesn't seem to me like a good fit in this case; The way I see things, kalu runs checks & shows notifications. If you run more checks - or run the same ones again - you get new notifications. And if they're gone and you wanna see them again, you can have it re-show them.

Having kalu now update previous notifications doesn't seem like a good flow in this instance, so I'm not too sure it would be a good idea to do so.

Can I ask what's the real issue here? I mean, why do you want notifications to be updated in the first place?

jjk-jacky avatar Oct 01 '18 20:10 jjk-jacky

The issue: often kalu will spawn a lot of "these packages can be updated" notifications and I will react to them after a long time, which really clutters the notification list. Similarly, if I'm not updating today because the updates are too big, I don't want a new notification every Δt time.

I don't insist that's the only right behavior, but maybe this could be configurable?

marmistrz avatar Oct 02 '18 17:10 marmistrz

Well, in Preferences you can set a timeout after which notifications expire/should be auto-closed, so you shouldn't get a pile of notifications that way -- unless your notification daemon keeps expired notifications open, but then that's an issue of said daemon.

Also, if you know you're going away/afk for some time or just don't want to have notifications for a while, you can always easily pause kalu (e.g. middle click on its tray icon, can be set in Preferences as well) so it won't run checks until you manually unpause it.

jjk-jacky avatar Oct 02 '18 18:10 jjk-jacky

Well, this seems like a workaround to me. Btw. I've just realized that I have at least 5 notifications that kalu could not check for updates.

For me, one notification per intent is better UX. If news were fetched, then I'm not interested about the fact that previously it failed (because I had no connectivity). If I have new update list, I'm not interested about the old. It should be enough to keep the notification object for every intent in some data structure.

Some people may prefer the current behavior (thus configurable), but the proposed is the cleanest for me.

marmistrz avatar Oct 03 '18 18:10 marmistrz

What you seem to be missing here, is that I for instance never have had that many notifications, because I've set them (in kalu's preferences) to expire after a few seconds, so my notification daemon does what it is supposed to do: after a few seconds, it auto-closes them.

If kalu runs new checks it will create new notifications, as expected. If notifications got (auto-)closed and I want to see them again (or if I missed them), I simply use kalu's "Re-show last notifications" and voilà! :)

It seems either you're not setting kalu's notifications to expire - in which case what you get is expected behavior - or your notification daemon is keeping expired notifications around - in which case that would be an issue with it -- which case is it?

Seems you're looking for a different workflow/behavior, where you don't want notifications to expire, so they stick around, and instead wishes for kalu to try and update any previous notifications. But I'm still not sure if it's such a good idea...

jjk-jacky avatar Oct 03 '18 19:10 jjk-jacky

I can't set the notifications to expire after an hour, i.e. after the time the next check is going to happen. I want to see that there are new updates even if I'm not in front of the computer at the very time kalu checks the updates.

Well, the good idea is the one that works best for an individual. I think I can help a little with a PR if you tell me where the relevant code resides.

marmistrz avatar Oct 11 '18 13:10 marmistrz

I can't set the notifications to expire after an hour, i.e. after the time the next check is going to happen. I want to see that there are new updates even if I'm not in front of the computer at the very time kalu checks the updates.

Right, but that's pretty much exactly why there's the "Re-show last notifications" feature. So that you can, at any given time, just click on kalu's icon (or use a keyboard shortcut to trigger it via kalu's FIFO) and have the last notifications instantly re-shown. What's wrong with that approach?

jjk-jacky avatar Oct 11 '18 15:10 jjk-jacky

I'm not affected by the following, but: there are DEs where the system tray is not present, such as Gnome, which rely completely upon notifications.

I'll turn it around: what's wrong with an alternative, opt-in notification mode?

marmistrz avatar Oct 13 '18 09:10 marmistrz

I'll turn it around: what's wrong with an alternative, opt-in notification mode?

Not sure what you mean by that though? Because the systray is the way to interact with kalu, so without it things would be quite limited I guess.

But, you might be able to do things thanks to kalu's FIFO, allowing you to popup its menu and trigger other commands at will. But then we're back to the current situation: You get notifications whenever needed, you can have them pop back up via "Re-show last notifications" any time you want, what's missing?

jjk-jacky avatar Oct 13 '18 13:10 jjk-jacky

The idea is to have persistent notifications, but only one of them for each type. This is so as to minimize user interaction. I'm installing applications using yay anyway, I only need kalu to notify me about new updates. Or, in other words, if the notifications are to expire, I don't want any notifications, as I'd mostly have to trigger them manually anyway.

For future reference:

  • the function to be modified is show_notif in gui.c. It could accept a NotifyNotification*. If the extra argument is non-null, the function would reuse the last notification instead of creating new one.
  • the only non-trivial use of show_notif is in main.c, notify_updates. This function would have to accept a array of NotifyNotification*, of length _NB_TPL, one pointer for each variant of tpl_t.
  • notify_updates is extensively used by kalu_check_work, this function can't store the state
  • kalu_check_work is used from the main function, but also from

In other words, the difficult part is where to store the state. The other part is mostly trivial argument passing. How about a static mutex-protected variable in notify_updates?

marmistrz avatar May 27 '19 08:05 marmistrz