[Folder View] Improve layout, formatting, and wording of Icons and Locations pages
ClosedPublic

Authored by ngraham on Oct 16 2018, 3:12 AM.

Details

Summary

This patch improves the layout, formatting, and wording of the Folder View widget's Icons and Locations pages to better follow the HIG (especially https://hig.kde.org/patterns/content/form.html). In the process, it adopts Kirigami.FormLayout.

Test Plan

Icons page, Desktop, before:

Icons page, Desktop, after:

Icons page, Panel widget, before:

Icons page, Panel widget, after:

Locations page, before:

Locations page, after:

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.
ngraham created this revision.Oct 16 2018, 3:12 AM
Restricted Application added a project: Plasma. · View Herald TranscriptOct 16 2018, 3:12 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
ngraham requested review of this revision.Oct 16 2018, 3:12 AM
ngraham edited the test plan for this revision. (Show Details)Oct 16 2018, 3:13 AM
This comment was removed by ngraham.
ngraham updated this revision to Diff 43712.Oct 16 2018, 3:18 AM

Improve another string

ngraham edited the test plan for this revision. (Show Details)Oct 16 2018, 3:18 AM

Note: I know the width: 500 is a horrible hack that is certainly the wrong way to do this, but I couldn't figure out how else to get the Labels' width to be long enough so that Layout.alignment: Qt.AlignRight worked. Layout.fillWidth: true had no effect. I look forward to learning the correct way to handle this situation. :)

That looks much better!
Any reason why the "Text lines" parameter in the Icon panel is not aligned with the rest of the form ?

ngraham planned changes to this revision.Oct 16 2018, 2:15 PM

Oops, found a few visual glitches. Will fix today.

ngraham updated this revision to Diff 43758.Oct 16 2018, 9:15 PM

Fix the Text Lines label and spinbox

ngraham edited the test plan for this revision. (Show Details)Oct 16 2018, 9:16 PM
ngraham updated this revision to Diff 43835.Oct 18 2018, 4:20 AM

No more ugly hacks: port the whole thing to use a Kirigami FormLayout

ngraham edited the test plan for this revision. (Show Details)Oct 18 2018, 4:21 AM
ngraham added a subscriber: mart.Oct 18 2018, 4:28 AM

Kirigami FormLayout is a really nice control to work with. I'm a big fan. This port was easy peasy and I can't wait to do more of these!

The double Kirigami.Separator items are to approximate what's wanted for https://bugs.kde.org/show_bug.cgi?id=399959, but this is probably an incorrect usage of them as-is. Would like to get @mart's opinion on the best way to add some whitespace to a Kirigami FormLayout.

ngraham edited the test plan for this revision. (Show Details)Oct 18 2018, 4:29 AM
ngraham edited the test plan for this revision. (Show Details)

Kirigami FormLayout is a really nice control to work with. I'm a big fan. This port was easy peasy and I can't wait to do more of these!

The double Kirigami.Separator items are to approximate what's wanted for https://bugs.kde.org/show_bug.cgi?id=399959, but this is probably an incorrect usage of them as-is. Would like to get @mart's opinion on the best way to add some whitespace to a Kirigami FormLayout.

Love it!

ngraham updated this revision to Diff 43899.Oct 19 2018, 4:19 AM

Make the ghetto spacers invisible

ngraham edited the test plan for this revision. (Show Details)Oct 19 2018, 4:22 AM

Instead of ghetto DIY spacers, maybe we can use purpose-built vertical spacers: D16330: Add a spacer item

ngraham updated this revision to Diff 43959.Oct 20 2018, 3:53 AM
  • Also do the Location page
  • Use Spacer items instead of hacks
ngraham retitled this revision from [Folder View] Improve layout and formatting of Icons page to [Folder View] Improve layout, formatting, and wording of Icons and Locations pages.Oct 20 2018, 4:00 AM
ngraham edited the summary of this revision. (Show Details)
ngraham edited the test plan for this revision. (Show Details)
ngraham added a dependency: D16330: Add a spacer item.
hein accepted this revision.Oct 22 2018, 5:08 AM
This revision is now accepted and ready to land.Oct 22 2018, 5:08 AM
mart requested changes to this revision.Oct 22 2018, 4:11 PM
mart added inline comments.
containments/desktop/package/contents/ui/ConfigIcons.qml
130–131

those should be just

Item {
Kirigami.FormData.isSection: true
}

then, we'll see how things in formlayout have to be fixed to reach the desired spacing/result

This revision now requires changes to proceed.Oct 22 2018, 4:11 PM
ngraham updated this revision to Diff 44089.Oct 22 2018, 11:20 PM

Implement @mart's suggestions

ngraham marked an inline comment as done.Oct 22 2018, 11:21 PM

@mart I've implemented your suggestions. To my eyes, the vertical spacing isn't enough. Compare the current version:

...With the previous one I had before:

But that's pretty minor and would need to be changed (if you agree) elsewhere.

hein accepted this revision.Nov 1 2018, 4:30 PM

Looks great, I'm happy to finally see this get cleaned up.

mart accepted this revision.Nov 1 2018, 4:34 PM
This revision is now accepted and ready to land.Nov 1 2018, 4:34 PM
ngraham edited the summary of this revision. (Show Details)Nov 1 2018, 5:17 PM
This revision was automatically updated to reflect the committed changes.