Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Notification dbus refactor #36

Merged
merged 39 commits into from
Jan 14, 2022
Merged

Notification dbus refactor #36

merged 39 commits into from
Jan 14, 2022

Conversation

EbonJaeger
Copy link
Member

Description

This pull request refactors the entire notification system. Some benefits of this are outlined in #23, #25, and #21. The main thing is that there is a dedicated notification server implementing the Freedesktop spec, and having a separate dedicated DBus server to "repeat" notifications and closings. This second server can then be listened to by, e.g. Raven to display notifications in other ways. This work makes it much easier to work with notifications.

This PR needs significant testing before merging to ensure that everything works properly and that there are no regressions compared to current Budgie.

Testing

If you wish to help test this work, checkout and build the notification-dbus-refactor branch. Then either re-log or open a terminal and run budgie-daemon --replace &! and budgie-panel --replace &!.

Changes

Some changes were made to notification handling to better conform to the Freedesktop spec and/or to make development life easier:

Submitter Checklist

  • Squashed commits with git rebase -i (if needed)
  • Built budgie-desktop and verified that the patch worked (if needed)

EbonJaeger and others added 30 commits January 6, 2022 20:40
There is a bunch of stuff missing, but this should be a good start.

The main issue I'm running into so far is I can't quite come up with a way to allow other parts to interact with this server.
This should be the same or super close to the existing notification popup widget in use.

This also adds tracking in the DBus server for active popups.

The idea for this is that there is a central place (here) to show notification popups. That way, Raven can focus on its own view for stored notifications without having to deal with the popup logic.

Hopefully, Raven will somehow be able to use the new notification class without having to duplicate a ton of logic.
This is really a part 1 or 2, because the next step is to change everything to use the control center DND setting.

Signed-off-by: Evan Maddock <[email protected]>
This marks the first time that the new notification system can actually work. Many initial issues with the popups and popup system have already been fixed as a result of this.

There seems to be some weirdness with certain notifications, primarily from Discord, where the notifications will appear on top of each other.
This seems to mostly prevent Discord popups being weird.
This gives a better way to replace a notification.
Meson can get really pedantic with link order. Also fixed NotificationPosition being used in Budgie Desktop Settings.
Much of this code is the same as before the refactor, just modified to use the new DBus names and classes.
There is a bunch of stuff missing, but this should be a good start.

The main issue I'm running into so far is I can't quite come up with a way to allow other parts to interact with this server.
This should be the same or super close to the existing notification popup widget in use.

This also adds tracking in the DBus server for active popups.

The idea for this is that there is a central place (here) to show notification popups. That way, Raven can focus on its own view for stored notifications without having to deal with the popup logic.

Hopefully, Raven will somehow be able to use the new notification class without having to duplicate a ton of logic.
This is really a part 1 or 2, because the next step is to change everything to use the control center DND setting.

Signed-off-by: Evan Maddock <[email protected]>
This marks the first time that the new notification system can actually work. Many initial issues with the popups and popup system have already been fixed as a result of this.

There seems to be some weirdness with certain notifications, primarily from Discord, where the notifications will appear on top of each other.
This seems to mostly prevent Discord popups being weird.
This gives a better way to replace a notification.
EbonJaeger and others added 7 commits January 13, 2022 23:10
Meson can get really pedantic with link order. Also fixed NotificationPosition being used in Budgie Desktop Settings.
Much of this code is the same as before the refactor, just modified to use the new DBus names and classes.
…ie/budgie-desktop into notification-dbus-refactor
This also makes sure we put a notification into Raven when popups are paused or disabled.
@EbonJaeger EbonJaeger added the enhancement New feature or request label Jan 14, 2022
@EbonJaeger EbonJaeger added this to the 10.6 milestone Jan 14, 2022
@EbonJaeger
Copy link
Member Author

Oh yeah, I forgot: the notification server name still reports as Raven. Should this be changed to something more agnostic?

Copy link
Member

@JoshStrobl JoshStrobl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Also don't know what this is about.

Otherwise excellent work, seriously. Even learned about the widget property setting syntax in this commit!

src/daemon/notifications/dbus.vala Outdated Show resolved Hide resolved
src/daemon/notifications/dbus.vala Outdated Show resolved Hide resolved
src/daemon/notifications/popup.vala Outdated Show resolved Hide resolved
src/daemon/notifications/popup.vala Show resolved Hide resolved
src/daemon/notifications/popup.vala Show resolved Hide resolved
src/lib/notification.vala Show resolved Hide resolved
src/lib/notification.vala Show resolved Hide resolved
src/lib/notification.vala Show resolved Hide resolved
src/panel/settings/settings_style.vala Outdated Show resolved Hide resolved
src/raven/notifications_group.vala Outdated Show resolved Hide resolved
@EbonJaeger
Copy link
Member Author

EbonJaeger commented Jan 14, 2022

I'm not sure about the subproject thing either... Or where it came from. I don't see that commit in this changeset O.o hmm...
And yeah, the syntax for the widget properties is super nice!

@JoshStrobl JoshStrobl merged commit f4c29e7 into master Jan 14, 2022
@JoshStrobl JoshStrobl deleted the notification-dbus-refactor branch January 14, 2022 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

notifications: Given ID should be reused when replacing notification
2 participants