Redesign QML applet configuration windows
ClosedPublic

Authored by filipf on Apr 29 2019, 9:10 PM.

Details

Summary

This patch updates the sidebar appearance of QML applet configuration windows by painting them with the view color and separating them from the rest of the content with Kirigami separators.

Margins around actual category content have also been tweaked to be uniform.

Test Plan

Screenshots are without window borders.

Breeze:

Breeze Light:

Breeze Dark:

Diff Detail

Repository
R119 Plasma Desktop
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 11371
Build 11389: arc lint + arc unit
filipf created this revision.Apr 29 2019, 9:10 PM
Restricted Application added a project: Plasma. · View Herald TranscriptApr 29 2019, 9:10 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
filipf requested review of this revision.Apr 29 2019, 9:10 PM
filipf updated this revision to Diff 57218.Apr 29 2019, 9:16 PM

remove unrelated changes

filipf edited the test plan for this revision. (Show Details)Apr 29 2019, 9:18 PM
filipf added reviewers: VDG, Plasma, ngraham.
filipf edited the test plan for this revision. (Show Details)Apr 29 2019, 9:21 PM

In general, this is a change I hugely support. This look for sidebars is already used in System Settings, Discover, and other QML-ish apps. It's attractive and clean, and provides adequate separation between dissimilar elements.

Looking at your Breeze Light and Breeze Dark screenshots makes me think that we really need to either make their titlebars have a subtly different color from the window, or else draw a line underneath the titlebars. The fact that the list just ends is really odd-looking. We already see the effect with System Settings:

(Than again that's all a task for T10201).

I agree, the sidebar looks a bit lost there. So do you mean to turn on the separator in KWin or add it ourselves like this?

I agree, the sidebar looks a bit lost there. So do you mean to turn on the separator in KWin or add it ourselves like this?

C'est magnifique.

In this case my recommendation would be for KWin to do it (it already can via that off-by-default feature), we just need to adjust the color so people don't hate it.

filipf updated this revision to Diff 57227.Apr 30 2019, 1:09 AM

have top separator fill the window's width

filipf edited the test plan for this revision. (Show Details)Apr 30 2019, 1:14 AM

Beautiful.

Now we just need to do it in Breeze too, and turn off those borders when using Breeze window decorations.

hein added a subscriber: hein.Apr 30 2019, 5:01 AM

Looks great to me. Subtle visual refreshs like this are very appreciated, especially when they improve consistency, too.

I was wondering, wouldn't it also make sense to change the layout to be Icon left of text to make it even more consistent with System Settings? Or will that eat too much horizontal space?

I was wondering, wouldn't it also make sense to change the layout to be Icon left of text to make it even more consistent with System Settings? Or will that eat too much horizontal space?

That would be even more consistent, although we should probably take into account the differences between the two cases ie. the number of categories.

When it comes to applet configuration, the most categories I've ever run into was 10 in Event Calendar, and you can still fit all of them vertically on a 1080p screen easily.

But lot of applets only have 2 categories. So yeah if we do row layouts we'd be using more horizontal space, however we've got a lot of vertical space so there's no need to make that trade-off IMO.


BTW implementation wise here's one reason why what I did here is not a good solution:

Yeah I think keeping the icons on top is nice here since it makes for large click areas. Putting the icon beside the text makes more sense when the number of categories is very large, which isn't normally the case for settings windows.

BTW implementation wise here's one reason why what I did here is not a good solution:

Looks like the rectangle needs to not be visible when there's only one category. Maybe we can get that information from globalConfigModel.

filipf updated this revision to Diff 57255.Apr 30 2019, 12:15 PM

Don't draw sidebar and its separator when there is only 1 category, also add left margin

Fixed. Not sure if the top separator should stay, but I guess it makes more sense if it does:

I'm so in love with this new style. it's just astonishingly good-looking.

Once this is in users' hands (in conjunction with no borders by default for Breeze), I predict that we get a flood of praise for it.

Implementation-wise I wonder if this is the correct place to locate the horizontal line below the titlebar. In this case it works, but we have other windows that currently need it too (e.g. System Settings, Discover, probably any Kirigami app). I wonder if it should be drawn by a framework instead, or the Breeze style, or even KWin. Or some combination of the two.

For more on the subject of horizontal lines under titlebars, see the discussion in T10201#183425.

mart added a subscriber: mart.May 6 2019, 9:04 AM

from the screenshots, i like it a lot,the code seems clean enough.

However, to continue with that i think some attempt should be done to make qwidget based apps follow this as well. (I think we should go on a campaign to remove unnecessary frames here too)

with qwidget is perhaps slightly harder to do, but should be totally possible

another issue is that (as kirigami apps) this may look a bit bad with the window borders which unfortunately are still default on

mart added a comment.May 6 2019, 9:06 AM

I was wondering, wouldn't it also make sense to change the layout to be Icon left of text to make it even more consistent with System Settings? Or will that eat too much horizontal space?

I think a config window is different enough to a dedicated config app to justify the difference and there is better to optimize the size

mart added a comment.May 6 2019, 10:32 AM

that's a veeery quick and dirty attempt: (so hacky that there is not really usable code i'm afraid)

apart the lack of right margin, that should be easily fixable, the worst parts seem to be the repositioning of bottom buttons, putting the help button where it is in the screenshot was really an ugly and unreliable hack.
also that remaining 2 pixels margins around the sidebar doesn't seem to be easily killable

if somebody wants to do more research, is very welcome

In D20908#461487, @mart wrote:

from the screenshots, i like it a lot,the code seems clean enough.

However, to continue with that i think some attempt should be done to make qwidget based apps follow this as well. (I think we should go on a campaign to remove unnecessary frames here too)

I absolutely agree 100% on both points.

with qwidget is perhaps slightly harder to do, but should be totally possible

Boom, awesome! How hard would it be to clean up the code to make that submittable?

another issue is that (as kirigami apps) this may look a bit bad with the window borders which unfortunately are still default on

That's in progress right now. :)

filipf updated this revision to Diff 57680.May 6 2019, 7:43 PM

Kirigami.Theme.viewBackgroundColor is deprecated so set the colorSet to Theme.View instead

mart added a comment.Jun 3 2019, 1:36 PM

This is the rough patch which gives a rough implementation of this look into qwidget config dialogs

The real hack which I consider a no-go (and no idea how to avoid) is this piece:

  • layout->addWidget(mButtonBox);

+ if (mPageWidget && mPageWidget->layout()) {
+ mButtonBox->setParent(mPageWidget);
+ static_cast<QGridLayout*>(mPageWidget->layout())->addWidget(mButtonBox, 4, 1);
+ } else {
+ layout->addWidget(mButtonBox);
+ }

it reparents the buttonbox inside the pagewidget itself to have the buttons laid out considering the sidebar.
This will break badly and possibily crash in many situations, so ideas for alternative implementations welcome :D

filipf updated this revision to Diff 60100.Jun 20 2019, 9:45 AM

rebase on master

filipf updated this revision to Diff 60101.Jun 20 2019, 9:47 AM

line 218: width -> implicitWidth

ngraham requested changes to this revision.Jun 20 2019, 12:32 PM

Hmm, something's not right anymore:

This revision now requires changes to proceed.Jun 20 2019, 12:32 PM
filipf updated this revision to Diff 60115.Jun 20 2019, 12:42 PM

fix wrong sidebar width

ngraham accepted this revision.Jun 20 2019, 1:16 PM

There we go, much better.

This is so nice. Let's not commit it until @mart's companion patch for Breeze has been submitted and accepted.

This revision is now accepted and ready to land.Jun 20 2019, 1:16 PM

Preview of a highlight style in line with what's being discussed in T11124:

From a technical POV is it possible to reuse the widgets/tasks SVG here?

mart added a comment.Jun 23 2019, 4:36 PM

Preview of a highlight style in line with what's being discussed in T11124:

From a technical POV is it possible to reuse the widgets/tasks SVG here?

just answered in real life.
short answer is.. better not :)

filipf updated this revision to Diff 60583.Jun 24 2019, 2:58 PM

Also implement the upcoming highlight style

filipf updated this revision to Diff 60584.Jun 24 2019, 3:02 PM

highlight effect: remove unused ID and bettery opacity values

The companion patches have landed; this can go in now!

ngraham retitled this revision from RFC: Redesign QML applet configuration windows to Redesign QML applet configuration windows.Jul 25 2019, 4:49 PM
ngraham edited the summary of this revision. (Show Details)
filipf updated this revision to Diff 62557.Jul 25 2019, 5:02 PM

do not update the highlight effect (yet)

filipf updated this revision to Diff 62558.Jul 25 2019, 5:04 PM

don't remove the other file :)

ngraham accepted this revision.Jul 25 2019, 5:15 PM
filipf edited the summary of this revision. (Show Details)Jul 25 2019, 5:20 PM
This revision was automatically updated to reflect the committed changes.