D26506: [KDED KCM] Rewrite as KDeclarative ScrollViewKCM

[Kai Uwe Broulik]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:aa2119a835e9: [KDED KCM] Rewrite as KDeclarative 
ScrollViewKCM (authored by broulik).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D26506?vs=73369&id=73705#toc

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26506?vs=73369&id=73705

REVISION DETAIL
  https://phabricator.kde.org/D26506

AFFECTED FILES
  CMakeLists.txt
  kcms/kded/CMakeLists.txt
  kcms/kded/filterproxymodel.cpp
  kcms/kded/filterproxymodel.h
  kcms/kded/kcmkded.cpp
  kcms/kded/kcmkded.desktop
  kcms/kded/kcmkded.h
  kcms/kded/modulesmodel.cpp
  kcms/kded/modulesmodel.h
  kcms/kded/package/contents/ui/main.qml
  kcms/kded/package/metadata.desktop

To: broulik, #plasma, #vdg, mart, ngraham
Cc: mart, ngraham, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra

D26506: [KDED KCM] Rewrite as KDeclarative ScrollViewKCM

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.


  Lovely.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D26506

To: broulik, #plasma, #vdg, mart, ngraham
Cc: mart, ngraham, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra

D26506: [KDED KCM] Rewrite as KDeclarative ScrollViewKCM

[Kai Uwe Broulik]

broulik updated this revision to Diff 73369.
broulik edited the summary of this revision.
broulik edited the test plan for this revision.
broulik added a comment.


  - Remove enable/disable animation
  - Enabled becomes green
  - Now monitors kded running
  - Notifies when modules got automatically started/stopped when settings were 
saved

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26506?vs=73089&id=73369

REVISION DETAIL
  https://phabricator.kde.org/D26506

AFFECTED FILES
  CMakeLists.txt
  kcms/kded/CMakeLists.txt
  kcms/kded/filterproxymodel.cpp
  kcms/kded/filterproxymodel.h
  kcms/kded/kcmkded.cpp
  kcms/kded/kcmkded.desktop
  kcms/kded/kcmkded.h
  kcms/kded/modulesmodel.cpp
  kcms/kded/modulesmodel.h
  kcms/kded/package/contents/ui/main.qml
  kcms/kded/package/metadata.desktop

To: broulik, #plasma, #vdg, mart
Cc: mart, ngraham, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra

D26506: [KDED KCM] Rewrite as KDeclarative ScrollViewKCM

[Marco Martin]

mart accepted this revision as: mart.
mart added a comment.
This revision is now accepted and ready to land.


  Code wise is ok
  
  I think the checkbox for exabling/disabling the service is fine usability wise

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D26506

To: broulik, #plasma, #vdg, mart
Cc: mart, ngraham, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra

D26506: [KDED KCM] Rewrite as KDeclarative ScrollViewKCM

[Kai Uwe Broulik]

broulik updated this revision to Diff 73089.
broulik added a comment.


  - Filter duplicates

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26506?vs=73085&id=73089

REVISION DETAIL
  https://phabricator.kde.org/D26506

AFFECTED FILES
  CMakeLists.txt
  kcms/kded/CMakeLists.txt
  kcms/kded/filterproxymodel.cpp
  kcms/kded/filterproxymodel.h
  kcms/kded/kcmkded.cpp
  kcms/kded/kcmkded.desktop
  kcms/kded/kcmkded.h
  kcms/kded/modulesmodel.cpp
  kcms/kded/modulesmodel.h
  kcms/kded/package/contents/ui/main.qml
  kcms/kded/package/metadata.desktop

To: broulik, #plasma, #vdg
Cc: ngraham, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26506: [KDED KCM] Rewrite as KDeclarative ScrollViewKCM

[Nathaniel Graham]

ngraham added a comment.


  Thanks, much better, now I can test it out.
  
  Some initial thoughts:
  
  - Overall very nice! I like it.
  - Making the "Running" and "Not Running" labels colorize themselves and then 
transition to the normal color when toggles is flashy and cool, but I wonder if 
maybe they should just always use those colors.
  - Would it be possible to also fix the bug that causes every service to be 
listed twice when you're running plasma built from source in a non-system 
location? Otherwise I see this: F7871010: Screenshot_20200108_114640.png 

  - Using checkboxes for auto-load-on-startup is only communicated via a 
tooltip right now. I figured this out pretty easily, but having key information 
communicated only in a tooltip strikes me as inappropriate. On Touch, there's 
no way to see the tooltip.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D26506

To: broulik, #plasma, #vdg
Cc: ngraham, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26506: [KDED KCM] Rewrite as KDeclarative ScrollViewKCM

[Kai Uwe Broulik]

broulik updated this revision to Diff 73085.
broulik added a comment.


  - Minor fixes
  - Fix Cmake

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26506?vs=73017&id=73085

REVISION DETAIL
  https://phabricator.kde.org/D26506

AFFECTED FILES
  CMakeLists.txt
  kcms/kded/CMakeLists.txt
  kcms/kded/filterproxymodel.cpp
  kcms/kded/filterproxymodel.h
  kcms/kded/kcmkded.cpp
  kcms/kded/kcmkded.desktop
  kcms/kded/kcmkded.h
  kcms/kded/modulesmodel.cpp
  kcms/kded/modulesmodel.h
  kcms/kded/package/contents/ui/main.qml
  kcms/kded/package/metadata.desktop

To: broulik, #plasma, #vdg
Cc: ngraham, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26506: [KDED KCM] Rewrite as KDeclarative ScrollViewKCM

[Nathaniel Graham]

ngraham added a comment.


  Nice! It seems to have a CMake error though:
  
CMake Error at kcms/kded/CMakeLists.txt:10 (qt5_add_dbus_interface):
  qt5_add_dbus_interface Function invoked with incorrect arguments for
  function named: QT5_ADD_DBUS_INTERFACE

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D26506

To: broulik, #plasma, #vdg
Cc: ngraham, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26506: [KDED KCM] Rewrite as KDeclarative ScrollViewKCM

[Kai Uwe Broulik]

broulik created this revision.
broulik added reviewers: Plasma, VDG.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
broulik requested review of this revision.

REVISION SUMMARY
  This rewrites the "Background services" KCM in QML using `ScrollViewKCM`.
  The two separate list views are merged into a single one with the 
configurable services at the top, and the ones that are loaded on-demand and 
"only for your convenience" at the bottom.
  A search field is added searching through name and plugin ID. Since the 
sortable table headers are gone, a filter combo is provided instead to filter 
for all, running, or non running services.
  As an extra Schmankerl when starting a service that immediately disables 
itself again (which technically isn't an error that would be indicated as such) 
a hint is shown to the user so they're not left wondering why it doesn't start.
  Furthermore, the code is cleaned up a lot (quite eerie, adding a 2020 
Copyright to an existing 2002 one :), ported to json plugin data, and a proper 
`QAbstractListModel` added.

TEST PLAN
  F7868523: Screenshot_20200107_223707.png 

  F7868524: Screenshot_20200107_223814.png 

  Trying to start the device automounter which disables itself on load when 
automounting is disabled in the KCM
  F7868525: Screenshot_20200107_223859.png 

  Starting or stopping a service shows a little animation
  F7868531: Screenshot_20200107_224027.png 

  This is mostly for when you apply changes and kded reloads, it will start all 
autoloaded modules, even if user manually stopped them. Originally I wanted to 
show an inline message along the lines of "some services were started again 
when you saved your changes because..." but that turned out to be too 
brittle/unreliable.
  
  Issues remaining:
  
  - Somehow that Kirigami listdelegate feature of adding right padding to take 
into account the scrollbar doesn't work in ScrollViewKCM
  - Since it's using delegaterecycler, the animation will play as you scroll up 
and down since as far as the item is concerned, status does change.
  - Some qt 5.14 adjustments (qregisteranonymoustype)
  - I get a binding loop on `Kirigami.AbstractListItem.implicitHeight` for my 
delegate for some reason

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D26506

AFFECTED FILES
  kcms/kded/CMakeLists.txt
  kcms/kded/filterproxymodel.cpp
  kcms/kded/filterproxymodel.h
  kcms/kded/kcmkded.cpp
  kcms/kded/kcmkded.desktop
  kcms/kded/kcmkded.h
  kcms/kded/modulesmodel.cpp
  kcms/kded/modulesmodel.h
  kcms/kded/package/contents/ui/main.qml
  kcms/kded/package/metadata.desktop

To: broulik, #plasma, #vdg
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart