[Device Notifier] Restore Solid notification messages
Needs RevisionPublic

Authored by thsurrel on Mar 15 2019, 9:35 PM.

Details

Reviewers
broulik
bruns
Group Reviewers
Plasma
Summary

Commit 17380886 in the devicenotifications data engine brought in
two problems with the Device Notifier:

  • the notifications are now cleared once a device is unmounted,

this results in the "You can now safely remove this device"
message not to be shown anymore.

  • since each notification is no longer its own data source, it

is possible to get into a situation where one device would only
receive a first notification but not any subsequent ones.

This patch attempts to fix these two problems.

Test Plan

Mount a removable device and unmount it through the Device
Notifier. A message should appear for a few seconds saying you
can safely remove that device.

Diff Detail

Repository
R120 Plasma Workspace
Branch
arc_removed_devices
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 11642
Build 11660: arc lint + arc unit
thsurrel created this revision.Mar 15 2019, 9:35 PM
Restricted Application added a project: Plasma. · View Herald TranscriptMar 15 2019, 9:35 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
thsurrel requested review of this revision.Mar 15 2019, 9:35 PM

Anyone could have a look at this ? Thanks in advance.

anthonyfieroni added inline comments.
applets/devicenotifier/package/contents/ui/DeviceItem.qml
50

do we need ? true : false i think no?

applets/devicenotifier/package/contents/ui/devicenotifier.qml
226
clearMessage(udi)
thsurrel updated this revision to Diff 57736.May 7 2019, 7:29 PM

Fix comments.
Thank you for the review!

thsurrel marked 2 inline comments as done.May 7 2019, 7:30 PM
bruns requested changes to this revision.May 26 2019, 12:59 PM

This is just much to complicated, adding layer over layer does not help.

Also, the second problem is unrelated to the change you mention - the "hasMessage" property could only be true for one DeviceItem, before and after.

applets/devicenotifier/package/contents/ui/DeviceItem.qml
50–51

trigger is essentially always true

125

the item should not mess with the internals of the statusSource

applets/devicenotifier/package/contents/ui/FullRepresentation.qml
108

This is IMHO wrong, if the plasmoid shows an error message and I happen to close it by clicking elsewhere, I can no longer see the error message, although it still applies

181

This is an unrelated change, and fixes an error introduced in https://phabricator.kde.org/R120:d1a5507bd57aa74c18392354dcd43b65e15ee491

applets/devicenotifier/package/contents/ui/devicenotifier.qml
218

would be much simpler to use udi = data[sources[i]].udi

This revision now requires changes to proceed.May 26 2019, 12:59 PM