Add Kirigami ListSectionHeader component
ClosedPublic

Authored by GB_2 on Aug 9 2019, 3:28 PM.

Details

Summary

Adds a standard ListSectionHeader component to make ListView section headers look coherent.

Test Plan

Run the example with qmlscene:

Diff Detail

Repository
R169 Kirigami
Branch
add-listsection-component (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 14913
Build 14931: arc lint + arc unit
There are a very large number of changes, so older changes are hidden. Show Older Changes
Restricted Application added a project: Kirigami. · View Herald TranscriptAug 9 2019, 3:28 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
GB_2 requested review of this revision.Aug 9 2019, 3:28 PM
GB_2 updated this revision to Diff 63425.

Remove duplicate AbstractListItem

ngraham added a subscriber: ngraham.Aug 9 2019, 3:38 PM

Maybe call it a ListSectionHeader or just SectionHeader?

src/controls/ListSection.qml
3

You made this, so it's your copyright

29

coherent -> consistent

GB_2 added inline comments.Aug 9 2019, 3:39 PM
src/controls/ListSection.qml
29

I just copied it from the other list items :-)

GB_2 updated this revision to Diff 63426.Aug 9 2019, 3:46 PM

Update copyright

GB_2 added a comment.Aug 9 2019, 3:46 PM

Maybe call it a ListSectionHeader or just SectionHeader?

I think just the word section is more accurate, because that's what the property that it will be used in is called and header could mean only a heading label.

GB_2 marked 3 inline comments as done.Aug 9 2019, 3:46 PM
GB_2 added a reviewer: mart.
davidre added a subscriber: davidre.Aug 9 2019, 3:49 PM

Should this set sectionDelegate: true and not expose this to the outside?

GB_2 updated this revision to Diff 63428.Aug 9 2019, 4:02 PM

Set sectionDelegate to true

GB_2 updated this revision to Diff 63429.Aug 9 2019, 4:10 PM

Use correct background color

In D23049#509309, @GB_2 wrote:

Maybe call it a ListSectionHeader or just SectionHeader?

I think just the word section is more accurate, because that's what the property that it will be used in is called and header could mean only a heading label.

Hmm, that's not how I interpreted it. This component isn't a whole section, as its name implies; it's just the thing that goes above a section (i.e. a header)

GB_2 updated this revision to Diff 63430.Aug 9 2019, 4:16 PM

Rename to ListSectionHeader

GB_2 edited the test plan for this revision. (Show Details)Aug 9 2019, 4:16 PM
GB_2 retitled this revision from Add Kirigami ListSection component to Add Kirigami ListSectionHeader component.
GB_2 edited the summary of this revision. (Show Details)
ognarb added a subscriber: ognarb.Aug 9 2019, 4:21 PM

Something is probably wrong with the version numbers.

src/kirigamiplugin.cpp
131

Why 2.0?

src/qmldir
19

Why 2.5?

GB_2 added a comment.Aug 9 2019, 4:23 PM

Something is probably wrong with the version numbers.

Yeah, I don't know which I should choose and why, I just copied it from the other list items.

ognarb added a comment.Aug 9 2019, 4:27 PM

It's a new version so probably 2.9 everywhere

The numbers need to match. I think you want 2.10 (the current version appears to be 2.9, but this QML versioning stuff is pretty confusing to me).

GB_2 updated this revision to Diff 63432.Aug 9 2019, 4:34 PM

Use Kirigami 2.10

ngraham accepted this revision as: VDG, ngraham.Aug 9 2019, 4:36 PM

Thanks! We definitely need @mart's input on the right implementation of these details. But visually it looks great to me in System Settings (working on patches for the current users of these section headers), and but +1 on the concept of course.

This revision is now accepted and ready to land.Aug 9 2019, 4:36 PM
ngraham added 1 blocking reviewer(s): mart.Aug 9 2019, 4:37 PM
This revision now requires review to proceed.Aug 9 2019, 4:37 PM

Idea: add a RowLayout inside the contentItem and put the Heading in there, then expose a property called like "CustomItems" that allows the implementation to add additional items to that RowLayout. This would let us port Discover's sources page headers, which currently have custom controls in them:

GB_2 updated this revision to Diff 63442.Aug 9 2019, 6:15 PM

Add customItems list

GB_2 edited the test plan for this revision. (Show Details)Aug 9 2019, 6:15 PM

Idea: add a RowLayout inside the contentItem and put the Heading in there, then expose a property called like "CustomItems" that allows the implementation to add additional items to that RowLayout. This would let us port Discover's sources page headers, which currently have custom controls in them:

Done, I also updated the example :-)

GB_2 edited the test plan for this revision. (Show Details)Aug 9 2019, 6:16 PM
GB_2 edited the summary of this revision. (Show Details)Aug 9 2019, 6:19 PM
GB_2 updated this revision to Diff 63443.Aug 9 2019, 6:20 PM

Update customItems description

GB_2 edited the test plan for this revision. (Show Details)Aug 9 2019, 6:28 PM
GB_2 updated this revision to Diff 63444.Aug 9 2019, 6:36 PM

Add example code

mart requested changes to this revision.Aug 12 2019, 5:38 PM
This revision now requires changes to proceed.Aug 12 2019, 5:38 PM
mart added inline comments.Aug 13 2019, 9:28 AM
src/controls/ListSectionHeader.qml
92 ↗(On Diff #63444)

This is a no-go hack:
those customItems shouldn't be reparented, but rather the property should be an alias to the data of another layout, like:

property alias customItems: innerLayout.data
contentItem: RowLayout {
    id: rowLayout
    RowLayout {
        id: innerLayout
    }

or, i would prefer even more, to just ditch that extra property completely:

default property alias _contents: rowLayout.data
contentItem: RowLayout {

​ id: rowLayout
​ Heading {

....

so one just does

ListSectionHeader {
   Button {....}
}

and just works(tm)

GB_2 updated this revision to Diff 63653.Aug 13 2019, 10:31 AM

Ditch the extra property

GB_2 marked 3 inline comments as done.Aug 13 2019, 10:32 AM
GB_2 edited the test plan for this revision. (Show Details)

Hmm, if software making use of this needs to manually add a label, that seems to defeat part of the point of using a standard component.

GB_2 added a comment.Aug 13 2019, 1:47 PM

Hmm, if software making use of this needs to manually add a label, that seems to defeat part of the point of using a standard component.

The label in the example was just supposed to show that any controls can be added. Sorry for the confusion, I'll remove it.

GB_2 updated this revision to Diff 63662.Aug 13 2019, 2:09 PM

Improve example

GB_2 edited the summary of this revision. (Show Details)Aug 13 2019, 2:10 PM
GB_2 edited the test plan for this revision. (Show Details)
GB_2 added a comment.Aug 17 2019, 9:27 AM

@mart is this good now?

After living with this for a bit, I feel like maybe the following UI changes might be good to make the effect a bit more subtle:

  • Reduce the opacity of the gray background bit so the header doesn't feel like "a hole in the view"
  • Experiment with removing or reducing the strength of the horizontal line separators above and below the header text
GB_2 updated this revision to Diff 64217.Aug 21 2019, 2:43 PM

Improve look

GB_2 edited the summary of this revision. (Show Details)Aug 21 2019, 2:44 PM
GB_2 edited the test plan for this revision. (Show Details)
mart accepted this revision.Aug 30 2019, 12:36 PM
This revision is now accepted and ready to land.Aug 30 2019, 12:36 PM
This revision was automatically updated to reflect the committed changes.