Fixed Issues mentioned in https://phabricator.kde.org/T6729
This patch also fixes a crash when the filetransfer of the icon fails
Details
- Reviewers
apol - Group Reviewers
KDE Connect - Commits
- R224:fbf8852d6fd4: Fix Notifications in Plasmoid
R224:1d78a2d288d9: Fix Notifications in Plasmoid
Receive a notification with a previously unknown icon -> icon in Plasmoid is displayed correctly
Receive a notification with an already existing id -> notification in Plasmoid is updated
Diff Detail
- Repository
- R224 KDE Connect
- Lint
Lint Skipped - Unit
Unit Tests Skipped
plugins/notifications/notificationsdbusinterface.h | ||
---|---|---|
61 | Please avoid a Boolean trap, use an enum of some sort instead. If you see a removeNotification("foo", false) you cannot tell at a glance what false means. |
plugins/notifications/notificationsdbusinterface.h | ||
---|---|---|
43 | Use full names, camel-cased: |
This change was overridden by D7312. I discovered that when observing a bug I fixed months ago :D
Why is it needed to remove and re-add the notification instead of just calling update?
Isn't there something that we can emit so the model knows the data has been updated? @apol master of Qt models, do you know a solution to this problem?
Otherwise looks good to me
plugins/notifications/notification.cpp | ||
---|---|---|
153 | QStringLiteral | |
plugins/notifications/notificationsdbusinterface.cpp | ||
148 | Unrelated change, remove before committing. | |
153 | Unrelated change, remove before committing. | |
plugins/notifications/notificationsdbusinterface.h | ||
42 | Unrelated change, remove before committing. |