canonical / snapd-desktop-integration

User session helpers for snapd
GNU General Public License v3.0
15 stars 10 forks source link

Udeng 2282 refresh awareness implement new scheme #76

Closed sergio-costas closed 5 months ago

sergio-costas commented 6 months ago

Implements the new Refresh Awareness protocol.

DONE:

TO DO:

sergio-costas commented 6 months ago

@robert-ancell Should I add support for FreeDesktop.org notifications, or it should be enough with the gnome ones?

robert-ancell commented 6 months ago

@robert-ancell Should I add support for FreeDesktop.org notifications, or it should be enough with the gnome ones?

Isn't Gio.Notification doing this for you? You don't seem to be making any explicit fdo/gnome notification calls that I can see.

sergio-costas commented 6 months ago

I don't know if Gio.Notification does abstract up to that point... I'll try to do some tests.

sergio-costas commented 6 months ago

You are right: it is supported. So nothing to do there, then. Thanks!

sergio-costas commented 5 months ago

@robert-ancell This is ready, with the exception of an icon for the application itself (because the notifications show the application icon too, in the top-left corner).

sergio-costas commented 5 months ago

Also, it is "compatible" with the current snapd version. This is: it won't be as "cute" as it will be when all the patches have landed in snapd, but it works. It doesn't show the application icon or the "visible name", and the "application is ready" notification is shown twice, but it works.

sergio-costas commented 5 months ago

Also, the desktop-launch interface is there just for when those patches land, but... I still don't know if that will be the definitive interface for reading the .desktop files, or will have to use a different one. If you want to launch this "as-is", maybe it would be a good idea to remove it until that patch lands... What do you think?

robert-ancell commented 5 months ago

Also, the desktop-launch interface is there just for when those patches land, but... I still don't know if that will be the definitive interface for reading the .desktop files, or will have to use a different one. If you want to launch this "as-is", maybe it would be a good idea to remove it until that patch lands... What do you think?

Yes, if it can be removed now do it an make a second PR to add it.

robert-ancell commented 5 months ago

Not a full review, we can go over again next week.

sergio-costas commented 5 months ago

Done the changes. But I can't merge it, I don't have permissions.

sergio-costas commented 5 months ago

Anyway, there are two things remaining.

robert-ancell commented 5 months ago

Ok, ping me when you're ready to merge.

sergio-costas commented 5 months ago

@robert-ancell I think that it's better to merge this one, and do those other changes in different PRs, so please, merge this one when you have some spare time.