Differential D19289

[DeviceNotifier] Port settings to QQC2 and Kirigami
ClosedPublic
Actions

Authored by filipf on Feb 24 2019, 9:48 PM.

Details

Reviewers
ngraham
davidedmundson
Group Reviewers
VDG
Plasma
Maniphest Tasks
T10586: Modernize widget configuration settings
Commits
R120:6be6a8c003c6: [DeviceNotifier] Port settings to QQC2 and Kirigami
Summary

Settings are ported to QtQuickControls 2 and now use a Kirigami Form Layout.

Test Plan

Before:


After:

Diff Detail

Repository
R120 Plasma Workspace
Branch
modernize-devicenotifier-settings (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 8797
Build 8815: arc lint + arc unit
filipf created this revision.Feb 24 2019, 9:48 PM
Restricted Application added a project: Plasma. · View Herald TranscriptFeb 24 2019, 9:48 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
filipf requested review of this revision.Feb 24 2019, 9:48 PM
filipf edited the test plan for this revision. (Show Details)Feb 24 2019, 9:49 PM
filipf added reviewers: VDG, Plasma, ngraham.
davidedmundson accepted this revision.Feb 24 2019, 9:54 PM
This revision is now accepted and ready to land.Feb 24 2019, 9:54 PM
ngraham accepted this revision.Feb 24 2019, 10:02 PM

Nice work!

davidedmundson added a comment.Feb 24 2019, 10:16 PM

Out of curiosity, did you need the exclusive group.
In theory RadioButton should do it automatically when we share a parent, I'm curious if FormLayout breaks that.

filipf updated this revision to Diff 52493.Feb 24 2019, 10:29 PM

ButtonGroup is not needed

filipf added a comment.Feb 24 2019, 10:29 PM
In D19289#418964, @davidedmundson wrote:

Out of curiosity, did you need the exclusive group.
In theory RadioButton should do it automatically when we share a parent, I'm curious if FormLayout breaks that.

You're right, it wasn't necessary; I was following the original code too closely.

filipf added inline comments.Feb 24 2019, 10:32 PM
applets/devicenotifier/package/contents/ui/configGeneral.qml
23

Is PlasmaCore also needed here?

ngraham added inline comments.Feb 24 2019, 10:34 PM
applets/devicenotifier/package/contents/ui/configGeneral.qml
23

Looks like it's not used anywhere, so I'd say no.

filipf updated this revision to Diff 52497.Feb 24 2019, 10:47 PM

remove PlasmaCore as an import

filipf marked 2 inline comments as done.Feb 24 2019, 10:47 PM
Closed by commit R120:6be6a8c003c6: [DeviceNotifier] Port settings to QQC2 and Kirigami (authored by filipf). · Explain WhyFeb 25 2019, 10:29 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 52493

View Options

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

Loading...