D17333: Port digital clock settings to QQc2 and Kirigami

[Marco Martin]

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

REVISION SUMMARY
use QQC2 and FormLayout for alignment

also, qqc2 combobox doesn't suffer from 390801 anymore

BUG: 390801

TEST PLAN
tried to set all the options

REPOSITORY
R120 Plasma Workspace

BRANCH
phab/qqc2time

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

AFFECTED FILES
applets/digital-clock/package/contents/ui/configAppearance.qml

To: mart, #plasma, #vdg
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

[Marco Martin]

mart added a comment.


left old, right new
F6453040: Screenshot_20181203_164128.png <https://phabricator.kde.org/F6453040>

REPOSITORY
R120 Plasma Workspace

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

To: mart, #plasma, #vdg
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

[Nathaniel Graham]

ngraham added a comment.


The new one doesn't follow the HIG and has poor alignment. Combobox labels are supposed to be to the left of the comboboxes. And having a huge list of checkboxes not aligned to the rest of the controls doesn't look great. This is why we typically add a left label to the top checkbox to serve as a pseudo header for all of them. It's the only way to make the alignment work when you have both checkboxes and other controls in a form layout.

REPOSITORY
R120 Plasma Workspace

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

To: mart, #plasma, #vdg
Cc: ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

[Marco Martin]

mart added a comment.

Post by Nathaniel Graham
The new one doesn't follow the HIG and has poor alignment. Combobox labels are supposed to be to the left of the comboboxes. And having a huge list of checkboxes not aligned to the rest of the controls doesn't look great. This is why we typically add a left label to the top checkbox to serve as a pseudo header for all of them. It's the only way to make the alignment work when you have both checkboxes and other controls in a form layout.

hmm, that's as aligned as a formlayout (qml or qwidget) can do.
where are supposed to be the combobox labels? can you mock up?

REPOSITORY
R120 Plasma Workspace

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

To: mart, #plasma, #vdg
Cc: ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

[Marco Martin]

mart added a comment.


ah, like
F6453054: Screenshot_20181203_165012.png <https://phabricator.kde.org/F6453054>

that can be done only with hacks that will spontaneously break i'm afraid

REPOSITORY
R120 Plasma Workspace

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

To: mart, #plasma, #vdg
Cc: ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

[Nathaniel Graham]

ngraham added a comment.


Here are some examples (in production, from Dolphin's settings window) of how we integrate diverse controls into a form layout and preserve good alignment:

F6453053: Screenshot_20181203_085022.png <https://phabricator.kde.org/F6453053>
F6453057: Screenshot_20181203_085113.png <https://phabricator.kde.org/F6453057>
F6453056: Screenshot_20181203_085044.png <https://phabricator.kde.org/F6453056>
F6453066: Screenshot_20181203_085620.png <https://phabricator.kde.org/F6453066>

REPOSITORY
R120 Plasma Workspace

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

To: mart, #plasma, #vdg
Cc: ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

[Marco Martin]

mart added a comment.
well, in those the comboboxes are aligned with the checkbox and not the checkbox label, as it should be (and as i did)

wrt the excessive spacing between checkbox and checkbox label is a qqc2 issue which i fixed

REPOSITORY
R120 Plasma Workspace

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

To: mart, #plasma, #vdg
Cc: ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

[Marco Martin]

mart added a comment.


i can put information again in front of showdate, tough it conveys misleading and wrong info... i really, really hate this abuse of labels as not-really-sections as the label describes the control standing besides it, not any other

(btw, chiming in as the historyc memory of the usability project, and how the formlayout initially came to be, in the beginning labels of all checkboxes were supposed to be flipped, so all of them on the left (and on the right only the checkbox, without any label)
this was dropped because caused too many flame wars between developers (users were perfectly ok with it)

REPOSITORY
R120 Plasma Workspace

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

To: mart, #plasma, #vdg
Cc: ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

[Marco Martin]

mart updated this revision to Diff 46799.
mart added a comment.


restore Information: label

REPOSITORY
R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D17333?vs=46794&id=46799

BRANCH
phab/qqc2time

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

AFFECTED FILES
applets/digital-clock/package/contents/ui/configAppearance.qml

To: mart, #plasma, #vdg
Cc: ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

[Marco Martin]

mart added a comment.


F6453133: Screenshot_20181203_172322.png <https://phabricator.kde.org/F6453133>

REPOSITORY
R120 Plasma Workspace

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

To: mart, #plasma, #vdg
Cc: ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


Much better! I'd want some padding/whitespace between the checkboxes and the radio buttons, and maybe also between the two comboboxes too. But that's probably a judgment call. :)

REPOSITORY
R120 Plasma Workspace

BRANCH
phab/qqc2time

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

To: mart, #plasma, #vdg, ngraham
Cc: ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

[Marco Martin]

This revision was automatically updated to reflect the committed changes.
Closed by commit R120:39e812d57fd6: Port digital clock settings to QQc2 and Kirigami (authored by mart).

CHANGED PRIOR TO COMMIT
https://phabricator.kde.org/D17333?vs=46799&id=46803#toc

REPOSITORY
R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D17333?vs=46799&id=46803

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

AFFECTED FILES
applets/digital-clock/package/contents/ui/configAppearance.qml

To: mart, #plasma, #vdg, ngraham
Cc: ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart