Improve the look of the Application Style KCM UI
ClosedPublic

Authored by GB_2 on Mar 10 2019, 10:42 AM.

Details

Summary

Apply the KDE HIG, merge the two tabs into one page and make the KCM look better.

Test Plan

Open the Application Style KCM.

Diff Detail

Repository
R119 Plasma Desktop
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 9466
Build 9484: arc lint + arc unit
GB_2 created this revision.Mar 10 2019, 10:42 AM
Restricted Application added a project: Plasma. · View Herald TranscriptMar 10 2019, 10:42 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
GB_2 requested review of this revision.Mar 10 2019, 10:42 AM
GB_2 updated this revision to Diff 53591.Mar 10 2019, 10:57 AM

Clean up style preview UI file

GB_2 updated this revision to Diff 53592.Mar 10 2019, 11:09 AM

Rename style preview checkbox from "Check box" to "Checkbox"

ndavis added a subscriber: ndavis.Mar 10 2019, 3:01 PM

Is that "Application style" combobox long enough to accommodate longer strings like "MS Windows 9x"?

GB_2 added a comment.Mar 10 2019, 3:03 PM

Is that "Application style" combobox long enough to accommodate longer strings like "MS Windows 9x"?

Yes, it adjusts itself to the width of the longest item automatically.

Can it have the same default width as the toolbar text location comboboxes?

GB_2 added a comment.EditedMar 10 2019, 3:07 PM

Can it have the same default width as the toolbar text location comboboxes?

Yeah, I can try that.

GB_2 updated this revision to Diff 53635.Mar 11 2019, 7:41 AM

Check if configure button should be enabled after clicking "Defaults" and rename "cbStyle" to "comboStyle"

GB_2 added a comment.Mar 11 2019, 7:42 AM
In D19651#428459, @GB_2 wrote:

Can it have the same default width as the toolbar text location comboboxes?

Yeah, I can try that.

For some reason I couldn't get it to work, I'll just leave it how it is now, I think it looks fine.

abetts added a subscriber: abetts.Mar 11 2019, 2:06 PM

Is there any way that we can make the preview window have more right and left margins? When presented like this, it seems like it is actually another module that users interact with.

GB_2 updated this revision to Diff 53747.Mar 12 2019, 5:41 PM

Improve preview

GB_2 edited the summary of this revision. (Show Details)Mar 12 2019, 5:43 PM
rooty added a subscriber: rooty.Mar 13 2019, 12:41 PM

Please consider abbreviating the labels:

  • dropping text in the last two dropdown menu items (No Text, Text Only, Beside Icons, Below Icons)
  • dropping the word location (it's implicit)
  • substituting the word text with the word label (it's more accurate)

One more concern: Why are there two seemingly unrelated radio buttons in the preview? Wouldn't just one button be enough?

+1 on the new clean layout and for getting rid of tabs

GB_2 added a comment.Mar 13 2019, 1:01 PM

Please consider abbreviating the labels:

  • dropping text in the last two dropdown menu items (No Text, Text Only, Beside Icons, Below Icons)
  • dropping the word location (it's implicit)
  • substituting the word text with the word label (it's more accurate)

    One more concern: Why are there two seemingly unrelated radio buttons in the preview? Wouldn't just one button be enough?

Ok, will do.
The first radio button should be activated, thanks for noticing that.

GB_2 updated this revision to Diff 53792.Mar 13 2019, 2:02 PM

Implement Rooty's suggestions and update preview

GB_2 edited the summary of this revision. (Show Details)Mar 13 2019, 2:03 PM
GB_2 updated this revision to Diff 53798.Mar 13 2019, 2:47 PM

Update preview

GB_2 edited the summary of this revision. (Show Details)Mar 13 2019, 2:48 PM
rooty accepted this revision as: rooty.Mar 13 2019, 2:50 PM
This revision is now accepted and ready to land.Mar 13 2019, 2:50 PM
ngraham requested changes to this revision.Mar 13 2019, 3:03 PM
ngraham added a subscriber: ngraham.
ngraham added inline comments.
kcms/style/kcmstyle.cpp
174

"the application style" isn't an example of "user interface elements" so the sentence does not make sense. This would be better:

"This module allows you to modify the visual appearance of applications' user interface elements"

This revision now requires changes to proceed.Mar 13 2019, 3:03 PM
GB_2 updated this revision to Diff 53803.Mar 13 2019, 3:09 PM

Update Quick Help text

GB_2 marked an inline comment as done.Mar 13 2019, 3:09 PM
ngraham accepted this revision.Mar 13 2019, 3:20 PM

Thanks! Code is sane too.

This revision is now accepted and ready to land.Mar 13 2019, 3:20 PM
This revision was automatically updated to reflect the committed changes.