[applets/devicenotifier] Port to ExpandableListItem
AbandonedPublic

Authored by ngraham on Mar 26 2020, 9:21 PM.

Details

Summary

Depends on D28033

Test Plan

The only major UI change is the loss of the progress bar. It is replaced with a textual
display of free space and total space in the subtitle, which is more consistent with the
appearance of other ExpandableListItem-using applets, and IMO is more useful anyway.

Diff Detail

Repository
R120 Plasma Workspace
Branch
port-to-ExpandableListItem (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 26399
Build 26417: arc lint + arc unit
ngraham created this revision.Mar 26 2020, 9:21 PM
Restricted Application added a project: Plasma. · View Herald TranscriptMar 26 2020, 9:21 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
ngraham requested review of this revision.Mar 26 2020, 9:21 PM
ngraham edited the summary of this revision. (Show Details)Mar 26 2020, 9:21 PM
ngraham updated this revision to Diff 80113.Apr 14 2020, 2:22 PM

Rebase correctly

broulik added a subscriber: dfaure.Apr 30 2020, 3:07 PM
broulik added inline comments.
applets/devicenotifier/package/contents/ui/DeviceItem.qml
32–38

There's an expandedDevice property in devicenotifier.qml which is supposed to decide which device is expanded. You either want to wire up this properly with the new delegate or nuke it.
For example in DeviceItem do

Connections {
    target: devicenotifier
    onExpandedDeviceChanged: {
        if (devicenotifier.expandedDevice === udi) {
            deviceItem.expand();
        } else {
            deviceItem.collapse();
        }
    }
}

However, it resets the expandedDevice right away for whatever reason. But this should maybe give you an idea. Alternatively, since expanded state is now in the delegate, you could just emit a signal when a source is added to have the device expand.

34

This is all over the place for me. All the free space jobs in the dataengine get the root size back. Not sure if a local issue or KIO? @dfaure

dfaure added inline comments.Apr 30 2020, 11:10 PM
applets/devicenotifier/package/contents/ui/DeviceItem.qml
34

Please give me a proper bug report in KIO terms :-)
What class do you use, what do you get as a result?

I don't see a bug in a TODO comment in some QML code ;)

ngraham updated this revision to Diff 82037.May 5 2020, 10:41 PM

Fix old TODOs; add a few new hopefully-less-bad FIXMEs

ngraham updated this revision to Diff 82038.May 5 2020, 10:41 PM

Remove debug logging that snuck in

ngraham updated this revision to Diff 82107.May 6 2020, 2:34 PM

Fix 'Unable to assign [undefined] to QString' warning FIXME

ngraham updated this revision to Diff 82108.May 6 2020, 2:35 PM

Remove TODO comment that was already done

ngraham updated this revision to Diff 82111.May 6 2020, 2:58 PM

Namespace qqc2

ngraham edited the summary of this revision. (Show Details)May 6 2020, 3:06 PM
ngraham updated this revision to Diff 82262.May 8 2020, 1:08 PM

Fix a FIXME

ngraham updated this revision to Diff 82264.May 8 2020, 1:21 PM

Remove more FIXMEs (the first one was already broken with the current implementation), and for the second one, there's no better way to implement this, though I may remove the logic entirely

ngraham retitled this revision from [WIP] [applets/devicenotifier] Port to ExpandableListItem to [applets/devicenotifier] Port to ExpandableListItem.May 8 2020, 1:24 PM
ngraham edited the summary of this revision. (Show Details)
ngraham marked 3 inline comments as done.May 8 2020, 1:27 PM
ngraham updated this revision to Diff 82653.May 12 2020, 12:45 PM

Remove a FIXME (it's already a problem in the existing version so it's not an artifact of this port

I'd like to land this early in the 5.20 cycle so we have lots of time for testing, if possible.