[containments/desktop] Port desktop settings pages to QQC2+Kirigami FormLayout and modernize UI
ClosedPublic

Authored by GB_2 on Apr 28 2019, 2:02 PM.

Details

Summary
  • Port to QQC2+Kirigami FormLayout (except for the mimetype table, there is no QQC2 equivalent)
  • Improve some strings
  • Follow the KDE HIG
  • Improve look of buttons and other controls



Test Plan

Configure a Folder View widget and go through the three settings pages.

Diff Detail

Repository
R119 Plasma Desktop
Branch
containments-desktop-port-desktop-settings-pages-to-qqc2-plus-kirigami-formlayout-and-modernize-ui (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 11315
Build 11333: arc lint + arc unit
GB_2 created this revision.Apr 28 2019, 2:02 PM
Restricted Application added a project: Plasma. · View Herald TranscriptApr 28 2019, 2:02 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
GB_2 requested review of this revision.Apr 28 2019, 2:02 PM
GB_2 updated this revision to Diff 57133.Apr 28 2019, 2:11 PM

Fix some capitalizations, "Hide Files Matching" -> "Hide matching"

davidedmundson accepted this revision.Apr 28 2019, 4:59 PM
davidedmundson added a subscriber: davidedmundson.

One comment, please include when committing

containments/desktop/package/contents/ui/ConfigFilter.qml
33–34

These lines are quite wrong to begin with, but resizing childenRect inside a columnLayout now becomes doubly wrong. It's a clear binding loop as children are resized to the layout.

If you don't have a tonne of warnings being printed, it probably means these properties get override by the instantiator of this Item.

Please kill them.

This revision is now accepted and ready to land.Apr 28 2019, 4:59 PM
GB_2 updated this revision to Diff 57144.Apr 28 2019, 6:37 PM

Don't set root size to childrenRect size

GB_2 marked an inline comment as done.Apr 28 2019, 6:37 PM
filipf accepted this revision.Apr 28 2019, 7:01 PM
filipf added a subscriber: filipf.

Works fine and looks good to me. This is probably a style issue, but it should be noted that these buttons shouldn't stay highlighted after they're pressed:

containments/desktop/package/contents/ui/ConfigLocation.qml
202

Personally I don't think we need this indent as the enabled property already gives a clue what the text field is related to, but it's not particularly important one way or another.

ngraham accepted this revision.Apr 28 2019, 10:39 PM
ngraham added a subscriber: ngraham.

Very nice work.

GB_2 added a comment.Apr 29 2019, 11:22 AM

Works fine and looks good to me. This is probably a style issue, but it should be noted that these buttons shouldn't stay highlighted after they're pressed:

Can't do much about that and it's like this for other buttons as well. It goes away after you click something else though, I think it isn't that bad.

containments/desktop/package/contents/ui/ConfigLocation.qml
202

This is the correct way and like this in other places such as the Fonts KCM too.

GB_2 marked 2 inline comments as done.Apr 29 2019, 11:22 AM
This revision was automatically updated to reflect the committed changes.
broulik added inline comments.
containments/desktop/package/contents/ui/ConfigFilter.qml
211

I think using ToolTip attached property is discouraged as that cannot inherit the default times from the desktop style.
Instead use

ToolTip {
    ...
}