Discussion:
D17192: use a Kirigami Heading for perfect consistency
Marco Martin
2018-11-27 16:23:34 UTC
Permalink
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
Make the title of the submodules column a Kirigami Heading
so that looks the same and is perfectly aligned wuith the KCM title
to make everything look coherent and minimize the number of fonts
shown on the screen at once
depends from D17190 <https://phabricator.kde.org/D17190> and D17191 <https://phabricator.kde.org/D17191>

TEST PLAN
loaded a lot of kcms

REPOSITORY
R124 System Settings

BRANCH
phab/heading

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

AFFECTED FILES
sidebar/package/contents/ui/SubCategoryPage.qml

To: mart, #plasma, #vdg
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
Marco Martin
2018-11-27 16:25:49 UTC
Permalink
mart added a comment.


F6443914: image.png <https://phabricator.kde.org/F6443914>

REPOSITORY
R124 System Settings

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

To: mart, #plasma, #vdg
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
Nathaniel Graham
2018-11-27 16:29:44 UTC
Permalink
ngraham added a comment.


I worry that this distorts the UI by giving the back button the same visual weight as the open KCM's title. I'm not categorically against it because I do see the advantage of improving the alignment, but something about it just feels kind of off to me in its current form, like the visual hierarchy is now not quite right.

Perhaps we could just remove the bold effect but keep it the same size as the other entries in the subcategory list?

REPOSITORY
R124 System Settings

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

To: mart, #plasma, #vdg
Cc: ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
Marco Martin
2018-11-28 08:47:35 UTC
Permalink
mart added a comment.


anyways, this can wait/can be discussed, the ones instead i would like to get in soonish are D17190 <https://phabricator.kde.org/D17190> and D17191 <https://phabricator.kde.org/D17191>

REPOSITORY
R124 System Settings

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

To: mart, #plasma, #vdg
Cc: ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
Andres Betts
2018-11-28 15:08:53 UTC
Permalink
abetts added a comment.


+1

REPOSITORY
R124 System Settings

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

To: mart, #plasma, #vdg
Cc: abetts, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
Marco Martin
2018-11-28 15:40:06 UTC
Permalink
mart added a comment.


@abetts , @ngraham discuss :)

REPOSITORY
R124 System Settings

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

To: mart, #plasma, #vdg
Cc: abetts, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
Andres Betts
2018-11-28 15:42:08 UTC
Permalink
abetts added a comment.


I don't really have an issue with the font alignment. I think where I am conflicted is the part where we want this text to have too many purposes. The label is big so as to mean that it is a page title. However, when we place this on a phone, the header text is huge. So the label is trying to be a page title and a breadcrumb. I think that we should rethink the size, at least. The alignment is great.

REPOSITORY
R124 System Settings

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

To: mart, #plasma, #vdg
Cc: abetts, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
Nathaniel Graham
2018-11-28 16:30:25 UTC
Permalink
ngraham added a comment.


I agree with @abetts. The alignment changes are great, but I think it's a semantic error to use the title size and style for text that's essentially a glorified back button.

REPOSITORY
R124 System Settings

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

To: mart, #plasma, #vdg
Cc: abetts, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
Marco Martin
2018-11-29 15:34:59 UTC
Permalink
mart added a comment.


well, to me part of the problem is that a title became a back button... even tough smaller target to hit, i would have preferred to have a toolbutton followed by a proper title that is just a title

REPOSITORY
R124 System Settings

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

To: mart, #plasma, #vdg
Cc: fabianr, abetts, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
Andres Betts
2018-11-29 15:37:16 UTC
Permalink
abetts added a comment.
Post by Marco Martin
well, to me part of the problem is that a title became a back button... even tough smaller target to hit, i would have preferred to have a toolbutton followed by a proper title that is just a title
I agree. In my mind we had something like this
Post by Marco Martin
TITLE > SUBTITLE > SUBTITLE
Where the > were back buttons

REPOSITORY
R124 System Settings

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

To: mart, #plasma, #vdg
Cc: fabianr, abetts, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
Nathaniel Graham
2018-11-29 16:27:33 UTC
Permalink
ngraham added a comment.


It isn't a title, though. At best, the text in question is a list item, not a title.

I kinda like the fact that the whole item is a button. It even looks button-like. Makes for a nice easy click/touch target, and you know exactly where going back will take you.

REPOSITORY
R124 System Settings

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

To: mart, #plasma, #vdg
Cc: fabianr, abetts, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
Marco Martin
2018-12-03 09:04:27 UTC
Permalink
mart added a comment.
Post by Nathaniel Graham
It isn't a title, though. At best, the text in question is a list item, not a title.
It surely is, it's the title of that column and says, this column is stuff about "Fonts", "Workspace theme" etc

REPOSITORY
R124 System Settings

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

To: mart, #plasma, #vdg
Cc: fabianr, abetts, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
Nathaniel Graham
2018-12-07 00:34:21 UTC
Permalink
ngraham added a comment.


I see, you're right!

Finally got to test this out instead of just complaining about the pictures. :) I now have `KDeclarative`, `KCMUtils`, and `KWidgetsAddons` with the necessary changes, but when I compile System Settings with this, I see the following:

With certain KCMs, the sizes don't seem identical:
F6460363: Too big.png <https://phabricator.kde.org/F6460363>

But with other ones, they do:
F6460369: Same size.png <https://phabricator.kde.org/F6460369>

I have reservations about how well this larger size will work for wordy languages like German. We already have some strings elided in English:
F6460365: Elided text.png <https://phabricator.kde.org/F6460365>

With this patch, the back button's text just seems, I dunno, //too big.//

REPOSITORY
R124 System Settings

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

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