Differential D2398

[Applet Configuration] Match category sidebar style with Breeze widget default style
ClosedPublic
Actions

Authored by broulik on Aug 10 2016, 9:36 PM.

Details

Reviewers
davidedmundson
andreaska
Group Reviewers
Plasma: Design
Plasma
Commits
R119:817b27f44d4a: [Applet Configuration] Match category sidebar style with Breeze widget…
Summary

This adjusts the category sidebar to look like the default Breeze widget style.
The width of the sidebar also adjusts (up to a certain maximum width) based on the labels within so it doesn't grow as wide anymore.
Moreover, keyboard navigation is greatly improved by indicating focus on the category sidebar and changing categories with arrow keys. Also, when tabbing through the dialog, the view is scrolled, if the item with focus would otherwise be inaccessible.

As Plasma doesn't seem to be supporting Qt high-dpi scaling anytime soon this uses units to get the sizing identical to what it looks like in the other applications.

Test Plan

You can change Breeze widget style to have a traditional frame but this option isn't the default and pretty hidden and I've lost all hope of getting a "proper" frame control anyway.

Only tested with DPR of 2 on high-dpi, sizing looks identical to eg. systme tray

While using a black and white icon in this context isn't correct (also urgh the triple scrollbars, I resized the window to the possible minimum though) the icon colorizes now like we do in other places

Diff Detail

Repository
R119 Plasma Desktop
Lint
Lint Skipped
Unit
Unit Tests Skipped
broulik updated this revision to Diff 5816.Aug 10 2016, 9:36 PM
broulik retitled this revision from to [Applet Configuration] Match category sidebar style with Breeze widget default style.
broulik updated this object.
broulik edited the test plan for this revision. (Show Details)
broulik added reviewers: Plasma, Plasma: Design.
broulik set the repository for this revision to R119 Plasma Desktop.
Restricted Application added a project: Plasma. · View Herald TranscriptAug 10 2016, 9:36 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
broulik added a comment.Aug 11 2016, 7:34 AM


Also on low-dpi looks identical to System Settings spacing-wise

colomar added a subscriber: colomar.Aug 11 2016, 9:23 AM

I'm glad that you're fixing this inconsistency!

Just for clarification since it can't be seen from the screenshots: In System Settings, the full highlight color is only used on the sidebar when it is currently focused. When the main window area is focused (initially when the sidebar is shown, or when interacting with the main content), the hover highlight visualization is used in the sidebar.

Is that the same in your patch?

andreaska accepted this revision.Aug 11 2016, 9:32 AM
andreaska added a reviewer: andreaska.
andreaska added a subscriber: andreaska.

+1 for the design improvement

This revision is now accepted and ready to land.Aug 11 2016, 9:32 AM
broulik added a comment.Aug 11 2016, 9:51 AM

In System Settings, the full highlight color is only used on the sidebar when it is currently focused

Yeah, noticed that, but focus handling in QML is just broken beyond repair so I didn't bother with that.

broulik updated this revision to Diff 5826.Aug 11 2016, 10:44 AM
broulik edited edge metadata.

Implement focus handling for category sidebar to match System Settings:

  • If it has focus the line uses a strong blue and arrow keys can be used to switch between pages
  • If it has no focus, the line is fainter
  • Clicking a category button gives it focus
broulik added a comment.Aug 11 2016, 10:44 AM

Fainter blue when it doesn't have focus

broulik updated this revision to Diff 5827.Aug 11 2016, 10:57 AM

More closely match focus behavior of Breeze widget style:
Only if the sidebar has focus and the item is current will it get intense blue, white text and selected icon state, otherwise it's a fainter blue with black text

broulik updated this revision to Diff 5829.Aug 11 2016, 11:35 AM

Disable activeFocusOnTab for the content ScrollView, when tabbing from the categories bar we should focus the first control in the page rather than the content ScrollView (it having focus isn't even visible because it doesn't have a frame)

broulik updated this revision to Diff 5832.Aug 11 2016, 1:25 PM
broulik updated this object.

When tabbing through the dialog, ensure the item with focus is visible, scrolling if neccessary

broulik updated this revision to Diff 5834.Aug 11 2016, 2:43 PM

Replicate fainter colors when window has no focus (QtQuick Controls don't do that unfortunately but the sidebar does that which is the most prominent part of the window, color-wise)

davidedmundson accepted this revision.Aug 11 2016, 3:01 PM
davidedmundson added a reviewer: davidedmundson.
davidedmundson added a subscriber: davidedmundson.

Seems good.

desktoppackage/contents/configuration/AppletConfiguration.qml
219 ↗(On Diff #5826)

If you make this this a ColumnLayout the layout's implicitWidth will be the size of the largest item, simplifiying line 167-176 to be a single line.

Your loop works, so only change it if you want to.

desktoppackage/contents/configuration/ConfigCategoryDelegate.qml
87 ↗(On Diff #5826)

get rid of the && Window.active

it does nothing.

colomar added a comment.Aug 11 2016, 3:19 PM

Looks great now, thank you for digging into the focus stuff!

broulik updated this revision to Diff 5838.Aug 11 2016, 3:19 PM
broulik edited edge metadata.

Let Layouts do the size calculations more

Closed by commit R119:817b27f44d4a: [Applet Configuration] Match category sidebar style with Breeze widget… (authored by broulik). · Explain WhyAug 11 2016, 3:24 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 5827

View Options

desktoppackage/contents/configuration/AppletConfiguration.qml

Loading...
View Options

desktoppackage/contents/configuration/ConfigCategoryDelegate.qml

Loading...