oxygen-demo : add KMessage preview
ClosedPublic

Authored by rjvbb on Jul 4 2018, 11:46 AM.

Details

Summary

A recent change to the KMessageWidget look has caught my interest and made me start to experiment with ways to make them integrate better, in particular concerning the use of colour (see D13777 and recent posts on the frameworks-devel ML).

This patch adds a preview of the 4 different KMessageWidget types to what I think is the most appropriate existing widget, plus the magic required for detecting and reacting to colour theme changes.

This change makes testing a new look across all installed colour themes and widget styles much easier. Idem for assessing just how well the current design really works, of course.

Test Plan

Works as intended.

Possible improvements:

  • add a dedicated frame/tab that could show other, comparable widgets (but which?)
  • add a signal to ColorSchemeChooser (but is that better than handling the QEvent it already triggers?)

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 601
Build 613: arc lint + arc unit
rjvbb created this revision.Jul 4 2018, 11:46 AM
Restricted Application added a project: Plasma. · View Herald TranscriptJul 4 2018, 11:46 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
rjvbb requested review of this revision.Jul 4 2018, 11:46 AM
broulik added a subscriber: broulik.Jul 4 2018, 1:00 PM

+1

kstyle/demo/oxygenframedemowidget.cpp
91

What do you need this for?

rjvbb added inline comments.Jul 4 2018, 1:18 PM
kstyle/demo/oxygenframedemowidget.cpp
91

That's to catch the event sent to the qApp instance when the theme is changed (to be exact: after KColorSchemeManager sets or changes the KDE_COLOR_SCHEME_PATH property.

Without this, the messages don't adapt to reflect the new colours (even the current implementation mixes the window background into its background colour).

broulik added inline comments.Jul 4 2018, 1:28 PM
kstyle/demo/oxygenframedemowidget.cpp
91

I think you're working around a bug here. KColorSchemeManager does

qApp->setPalette(KColorScheme::createApplicationPalette(KSharedConfig::openConfig(action->data().toString())));

so the KMessageWidget should get a PaletteChange event and adjust, if not, it's a bug there.

1/ As far as I know, the widget style has no way to style these widgets. Correct ? As such I wonder if oxygen-demo is the right place for showing this. Might better have a showcase in kwidgetsaddon.
(but in fact we also discussed that ultimately oxygen-demo should go "elsewhere", like e.g. frameworks-integration/kstyle).

2/ putting these in the Tab of the "Frame" page does not feel like the right place to me. (there seem to be no logic behind it) I would give it its own page; If you want to keep them in the "Frame" page (why not. They do look like frame), then they should be kept outside of the Tab frame. Possibly in a separate vlayout.
Also, please add an extra "addStretch" below the last messagebox so that they stack towards the top rather than being evenly distributed in the layout.

1/ As far as I know, the widget style has no way to style these widgets. Correct ?

Yes. I would have preferred if Breeze applied the new flat style to the widget instead of changing the look in the class itself.

As such I wonder if oxygen-demo is the right place for showing this. Might better have a showcase in kwidgetsaddon.

There is, actually.

1/ As far as I know, the widget style has no way to style these widgets. Correct ?

Yes. I would have preferred if Breeze applied the new flat style to the widget instead of changing the look in the class itself.

That requires either some hacking in breeze, or declaring a custom primitive (as done for kcapacitybar).
You would still need to implement a "default" rendering in the widget, for styles that do not support the custom element. (again as in kcapacitybar)..

As such I wonder if oxygen-demo is the right place for showing this. Might better have a showcase in kwidgetsaddon.

There is, actually.

Concerning the paletteChange business, as far as I understand, as soon as you set a custom palette to a widget, you do not recieve the paletteChange events from QApp any more (I think. To be confirmed).

D13884 fixes that. It doesn't set a custom palette but a custom style sheet on the QFrame (where you apparently can't style the hyperlink color for labels inside :/)

rjvbb updated this revision to Diff 37155.Jul 4 2018, 5:59 PM

Updated as (hopefully) requested.

I've left the property change event filter, for builds against older than 5.48.0 (or whatever version D13884 will appear in).

There is one side-effect that will exist with older frameworks and not with D13884 in place: messages are recreated so will reappear if you've closed them. The easy way to prevent that would be to disable the close button, should I do that or do we not care about this? I can't say I'll lay awake about the detail...

rjvbb set the repository for this revision to R113 Oxygen Theme.Jul 4 2018, 6:00 PM
rjvbb added a comment.Jul 4 2018, 6:08 PM

A couple of snapshot (with my current KMessageWidget facelift; styles and themes switched "on-the-fly"):

Oxygen with the Oxygen theme:

Breeze with Breeze-Dark:

QtCurve with my Mac OS X Graphite theme:

rjvbb marked 3 inline comments as done.Jul 11 2018, 7:32 AM

Ping?

A couple of snapshot (with my current KMessageWidget facelift; styles and themes switched "on-the-fly"):

Oxygen with the Oxygen theme:

Breeze with Breeze-Dark:

QtCurve with my Mac OS X Graphite theme:

Hi Rene,
First, I have stepped down from maintaining breeze, and oxygen, so you do not have to wait for my approval for pushing code any more.
Now, concerning the screenshot you sent, I guess I would have prefered to have the message boxes located in a vboxlayout sitting right of the three other frames, rather than below, and changing orientation (top-to-bottom, etc.), together with the other three frames when selecting the corresponding direction in the combobx above.
But that is just a matter of taste.

Regards,

Hugo

GB_2 added a subscriber: GB_2.Sep 28 2019, 7:59 AM

Looks fine IMO.

This revision was not accepted when it landed; it landed in state Needs Review.Oct 4 2019, 9:40 AM
This revision was automatically updated to reflect the committed changes.
lbeltrame reopened this revision.Oct 6 2019, 12:59 PM
lbeltrame added a subscriber: lbeltrame.

This breaks when building the Oxygen style for Qt4 (which is still a supported build option AFAIK):

[   73s] /home/abuild/rpmbuild/BUILD/oxygen-5.6.90git/kstyle/demo/oxygenframedemowidget.cpp:33:10: fatal error: kwidgetsaddons_version.h: No such file or directory
[   73s]    33 | #include <kwidgetsaddons_version.h>
[   73s]       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~
[   73s] compilation terminated.
lbeltrame requested changes to this revision.Oct 6 2019, 1:00 PM
This revision now requires changes to proceed.Oct 6 2019, 1:00 PM
This revision was not accepted when it landed; it landed in state Needs Revision.Oct 6 2019, 2:25 PM
This revision was automatically updated to reflect the committed changes.