Updated alignment, keylines, messures to new radio button layout and the new qml formlayout
ClosedPublic

Authored by fabianr on May 18 2018, 11:48 AM.

Diff Detail

Repository
R985 KDE Human Interface Guidelines
Branch
form
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 615
Build 627: arc lint + arc unit
fabianr requested review of this revision.May 18 2018, 11:48 AM
fabianr created this revision.

I notice that there's a lot of padding below the radio buttons in the new images compared to the old ones. It looks a little excessive to me. Is that deliberate, or a bug in whatever layout you're using to generate the screenshots?

source/layout/alignment.rst
29

This is somewhat confusingly worded, since the title is "Labels on the left" but the text says "it is recomended to place the lables to the right".

114–115

Same: we're overloading the word "label", which is confusing.

source/layout/metrics.rst
17

"whitespace" was fine, I think.

32

"resizeable" was fine IMHO, and "resize able" is just wrong. :)

55

I think "titlebar" is a technical term here, so it doesn't need the space in the middle.

73–75

"Kirigami and Plasma's"

Or even

"smallspacing and largespacing from Kirigami or Plasma"

74

"when ever" -> "whenever"

77

No need to capitalize "Multiples"

117

"mock ups" -> "mockups"

118

"multiple" -> "multiples"

"gridUnit, you wont" -> "gridUnit, and you won't"

127–128

"re sizable" -> "resizable"

128

"scroll able" -> "scrollable"

fabianr updated this revision to Diff 34468.May 19 2018, 9:45 AM
  • Fixed spelling mistakes
fabianr marked 11 inline comments as done.May 19 2018, 9:47 AM
ngraham added inline comments.May 19 2018, 3:04 PM
source/layout/alignment.rst
50

How about this?

"The labels that are to the left of their connected widgets should be right-aligned. This makes the whole group of form widgets appear to be center-aligned."

source/layout/metrics.rst
105

Let's not use the term "group box" here, as that could seem to imply that we're advocating the use of group boxes, which I believe we're trying to move away from.

fabianr updated this revision to Diff 34501.May 20 2018, 6:24 AM
  • Updated wording after phab feedback
fabianr marked 2 inline comments as done.May 20 2018, 5:30 PM
ngraham added inline comments.May 20 2018, 5:40 PM
source/layout/alignment.rst
29

"lables" -> "labels"

114–115

"on the top" -> "on top"

124

"multi line" -> "multi-line"

source/layout/metrics.rst
19

"whitespaces" -> "whitespace"

68

Maybe specifically recommend QFormLayout again here?

fabianr marked 3 inline comments as done.May 22 2018, 9:16 AM
fabianr added inline comments.
source/layout/metrics.rst
68

To me the content on the page is too much focused on form layout anyways. These should be more general guidelines for spacings and paddings. So I would rather not change that here, but in a next step add more general examples to the page and probably create a content pattern for forms.

fabianr updated this revision to Diff 34630.May 22 2018, 9:16 AM
  • Corrected typos. Reduced and adjusted whitespace in radio button images. Updated image for seperatos

Text looks good to me now. However I'm noticing that in the new images, there's an unequal amount of bedding on the two sides of the radio buttons and checkboxes: the label on the left is much closer than the label on the right.

fabianr planned changes to this revision.May 25 2018, 6:59 AM

I'll update the visuals as soon as I have D13086 on my system

fabianr updated this revision to Diff 35592.Jun 5 2018, 9:32 AM
  • Merge branch 'master' into form
  • Updated visuals after D13086
fabianr updated this revision to Diff 35669.Jun 6 2018, 9:25 AM
elvisangelaccio added inline comments.
source/layout/alignment.rst
110–113

Why are we removing this guideline?

117–122

I'd also add: "or if a label is quite long and cannot be reduced".

Sometimes a form layout would just look bad, even if there is plenty of space. For example see F5835555.

ngraham added inline comments.Jun 10 2018, 4:48 PM
source/layout/alignment.rst
110–113

I think because we're trying to move towards separating groups of controls using whitespace and horizontal lines instead of group boxes.

117–122

I worry that such a guideline could result in undesirable visual inconsistency on single page, with some using a label on the left, and others having a label on top. We're trying in general to move away from labels that are trying to be paragraphs. :)

source/layout/alignment.rst
110–113

But that's exactly what this sentence is suggesting: "use a 16px spacer to separate group of related options".

117–122

I'm not saying we should also add a screenshot. This is the place where we add "exceptions", right? Given that sometimes you cannot reduce the length of the label (see again the screenshot above), I think it's important we clarify what we should do in such cases.

ngraham added inline comments.Jun 10 2018, 10:19 PM
source/layout/alignment.rst
110–113

The guideline--sans group box recommendation--was moved to a different part of the document, onto line 106.

117–122

IMHO, it's almost always possible to reduce the length of the label. If it's not because the concept is rather complicated, then it would be better to express the information in another way, such as with a short label for the control itself, and an extended explanation label beneath it. Gwenview's Advanced Settings page does this, as an example.

This is looking good from my perspective, FWIW!

source/components/checkbox.rst
196

"check box" -> "checkbox"

source/components/radiobutton.rst
154

"that will" -> "which will"

source/layout/alignment.rst
124

"Keep track on label length" -> "Minimize label length"

131

"checkbox> see" -> "checkbox>, see"

source/layout/metrics.rst
22–23

"units as" -> "units such as"

23

"largeSpacing" -> "and largeSpacing"

68

"that will" -> "which will"

fabianr updated this revision to Diff 35990.Jun 11 2018, 7:13 AM
fabianr marked 10 inline comments as done.
  • Fixed typos and improved wording from phab feedback
source/layout/alignment.rst
110–113

As Nate said, the guidelines is not removed, but it is moved to a more appropriate page. Since this page is only about alignment, I moved it to "Metrics, placement & keylines" which has a lot of rules about spacing and paddings.

117–122

The HIG pages for check boxes https://hig.kde.org/components/checkbox.html has a rule, that could extend to cover these cases:
"Do not add line breaks. If necessary place an additional label below the check box."

Just two more little issues that were missed in the last round of revisions and then I think this is good to go!

source/components/checkbox.rst
196

You missed this one :)

source/components/radiobutton.rst
154

You missed this one :)

BTW, I'm not sure D13569 has any relationship to this. The UI there hasn't been updated according to these guidelines yet.

But the usage/layout of check boxes inside the KCollapsibleGroupBox is different from what we have defined in here.

But the usage/layout of check boxes inside the KCollapsibleGroupBox is different from what we have defined in here.

It is? In what way?

I'm noticing while implementing T9036 that 12px of whitespace space between sections feels too cramped, and the sections still kind of blended into one another. I tried with 18px and IMHO it looked vastly better:

If others agree, I might say that we should consider recommending 18px instead of 12px for spacers.

I agree, our controls are pretty big and need more breathing room.

ngraham added inline comments.Jun 19 2018, 2:29 AM
source/layout/metrics.rst
106

Per discussion below, let's increase this to 18px. A few more pixels makes a huge difference!

fabianr updated this revision to Diff 37192.Thu, Jul 5, 2:26 PM
fabianr marked an inline comment as done.
  • Fixed small typo and changes recomended spacer min height to 18px
fabianr marked an inline comment as done.Thu, Jul 5, 2:27 PM
fabianr added inline comments.
source/components/checkbox.rst
196

No change, since check box is used everywhere

ngraham accepted this revision.Fri, Jul 6, 11:37 AM

Thanks for your patience on this!

source/components/checkbox.rst
196

OK, let's take care of changing it to "checkbox" in another diff.

This revision is now accepted and ready to land.Fri, Jul 6, 11:37 AM
fabianr closed this revision.Fri, Jul 6, 11:51 AM