D13777: KMessageWidget : revert to using highlight colour for Information style (WIP)

[Nathaniel Graham]

ngraham requested changes to this revision.
ngraham added a comment.
This revision now requires changes to proceed.


  KWidgetsAddons is a Tier 1 framework, so it can't depend on any other KDE Frameworks, including KConfig, which is where the current theme's full palette is stored. Because of that, we're restricted to hardcoding the colors for the Positive, Warning, and Error styles rather than reading them from the current theme. My thinking was that since we're already forced to do this, it made more sense to just hardcode everything and preserve a consistent visual style until and unless we can somehow make it use the theme's colors.
  
  I'm not necessarily against using the palette's highlight color for the Information widget, but I'd strongly prefer a solution that allows the use of all colors from the active theme, not just one.
  
  We can't use the local widget's own palette, for reasons explained below in an inline comment.

INLINE COMMENTS

> kmessagewidget.cpp:262
> +    // use the current widget's palette (it could be different from the application palette)
> +    const QPalette palette = palette();
>  

Using the global application palette was deliberate. Try your change and then suspend a terminal window that has a dark background when you're using Breeze light. Local widgets may set a palette that's incompatible with some of the colors we're forced to hardcode due to `KWidgetsAddons` being a Tier 1 framework, or may set a palette that's incomplete and results in unreadable text. It's much safer to just use the app's own palette.

> kmessagewidget.cpp:272
>      case Information:
> -        bgBaseColor.setRgb(61, 174, 233); // Window: ForegroundActive
> +        // use the highlight colour for this type, as before
> +        bgBaseColor = palette.highlight().color();

"as before" not needed. Really, @cfeck is right and the entire comment isn't needed.

> kmessagewidget.cpp:282
>  
> -    const QPalette palette = QGuiApplication::palette();
>      const QColor windowColor = palette.window().color();

Same

REPOSITORY
  R236 KWidgetsAddons

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

To: rjvbb, ngraham, #frameworks
Cc: cfeck, kde-frameworks-devel, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180628/81b0c616/attachment.html>