[KDED KCM] Rewrite as KDeclarative ScrollViewKCM
ClosedPublic

Authored by broulik on Tue, Jan 7, 9:43 PM.

Details

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. A hint is also displayed when services got automatically started/stopped when applying settings as this reloads kded5.
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



Trying to start the device automounter which disables itself on load when automounting is disabled in the KCM

Starting or stopping a service shows a little animation

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
  • Some qt 5.14 adjustments (qregisteranonymoustype)
  • I get a binding loop on Kirigami.AbstractListItem.implicitHeight for my delegate for some reason

Diff Detail

Repository
R119 Plasma Desktop
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
broulik created this revision.Tue, Jan 7, 9:43 PM
Restricted Application added a project: Plasma. · View Herald TranscriptTue, Jan 7, 9:43 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
broulik requested review of this revision.Tue, Jan 7, 9:43 PM
ngraham added a subscriber: ngraham.Wed, Jan 8, 4:15 PM

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
broulik updated this revision to Diff 73085.Wed, Jan 8, 6:36 PM
  • Minor fixes
  • Fix Cmake

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:
  • 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.
broulik updated this revision to Diff 73089.Wed, Jan 8, 7:14 PM
  • Filter duplicates
mart accepted this revision as: mart.Fri, Jan 10, 12:31 PM
mart added a subscriber: mart.

Code wise is ok

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

This revision is now accepted and ready to land.Fri, Jan 10, 12:31 PM
broulik updated this revision to Diff 73369.Mon, Jan 13, 7:33 AM
broulik edited the summary of this revision. (Show Details)
broulik edited the test plan for this revision. (Show Details)
  • Remove enable/disable animation
  • Enabled becomes green
  • Now monitors kded running
  • Notifies when modules got automatically started/stopped when settings were saved
ngraham accepted this revision.Mon, Jan 13, 3:11 PM

Lovely.

This revision was automatically updated to reflect the committed changes.