WIP: Add support for a theming in Kicker Dashboard
Needs ReviewPublic

Authored by ognarb on Sep 1 2019, 5:33 PM.

Details

Reviewers
None
Group Reviewers
Plasma
VDG
Summary
  • Replace hardcoded color with Kirigami.Theme.*
  • Replace search bar with Kirigami.Search

I tried to use PlasmaCore.Theme but this didn't work. I think a blurred background would look nicer but I didn't find how to do it.

Test Plan

Note the shutdown/logout button with a wrong color

Diff Detail

Repository
R119 Plasma Desktop
Branch
kicker-dask-theme-support
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 15957
Build 15975: arc lint + arc unit
ognarb created this revision.Sep 1 2019, 5:33 PM
Restricted Application added a project: Plasma. · View Herald TranscriptSep 1 2019, 5:33 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
ognarb requested review of this revision.Sep 1 2019, 5:33 PM
ognarb edited the test plan for this revision. (Show Details)Sep 1 2019, 5:34 PM
ognarb added reviewers: Plasma, VDG.
ognarb edited the summary of this revision. (Show Details)
ognarb edited the test plan for this revision. (Show Details)
felixernst added a subscriber: felixernst.EditedSep 2 2019, 12:39 PM

Current (?) one for comparison (from Kubuntu 18.04 since the one from Neon Unstable crashes for me)


I feel like there was a lot of thought put into the current design of the dashboard and the search field in particular:

  • It does fit the style of having different logical parts of the dashboard purely divided by white space (and not by borders, lines).
  • It makes very clear that typing will trigger the search while in your screenshot it looks like the search field doesn't have focus and typing might not trigger the search.
  • In addition the size of the elements of the dashboard should loosely correspond to their importance. The search is arguably the most important feature of the dashboard. The way you changed it it seems a bit too minor to me.

I agree that the search as it currently is is inconsistent with how we display search fields anywhere else but it does fit with the rest of the dashboard. The overall style of the dashboard is pretty unique imo (for better or worse). So imo keeping the current search would be fine for now.

Other than that +1 for the color change. I'm using Breeze and the dashboard with its white text on dark background always looked out of place to me.
I agree that a blurred background would most likely look very nice.

davidre added a subscriber: davidre.Sep 2 2019, 3:18 PM

The reason it's transparent is because the clear color of the buffer is set to the background color when it's changed [1][2].
This worked before probably because the Color was (0, 0, 0). For transparent windows we need to set it to Qt::transparent[3] and enable an alpha buffer[4].

diff --git a/applets/kicker/plugin/dashboardwindow.cpp b/applets/kicker/plugin/dashboardwindow.cpp
index 6665e8df7..cc6b70c32 100644
--- a/applets/kicker/plugin/dashboardwindow.cpp
+++ b/applets/kicker/plugin/dashboardwindow.cpp
@@ -32,6 +32,8 @@ DashboardWindow::DashboardWindow(QQuickItem *parent) : QQuickWindow(parent ? par
 , m_visualParentWindow(nullptr)
 {
     setClearBeforeRendering(true);
+    setDefaultAlphaBuffer(true);
+    setColor(Qt::transparent);
     setFlags(Qt::FramelessWindowHint);
 
     setIcon(QIcon::fromTheme(QStringLiteral("plasma")));
@@ -100,8 +102,6 @@ QColor DashboardWindow::backgroundColor() const
 void DashboardWindow::setBackgroundColor(const QColor &c)
 {
     if (color() != c) {
-        setColor(c);
-
         emit backgroundColorChanged();
     }
 }

[1] https://cgit.kde.org/plasma-desktop.git/tree/applets/kicker/plugin/dashboardwindow.cpp#n103
[2]https://doc.qt.io/qt-5/qquickwindow.html#color-prop
[3]https://doc.qt.io/qt-5/qquickwidget.html#details (see Limitations)
[4]https://doc.qt.io/qt-5/qquickwindow.html#setDefaultAlphaBuffer

filipf added a subscriber: filipf.Sep 4 2019, 11:18 AM

What I'm seeing is that this could result in poor visibility of action buttons.

They should use text color.

I would also think a bit more about using Kirigami.SearchField -> it's not used in Kickoff and it's not properly themable (the same way PlasmaComponents.TextField is).

It also looks too small.

ngraham added a subscriber: ngraham.Sep 4 2019, 2:05 PM

As a general concept, are we sure this makes sense? This is an overlay dashboard. It seems like it's supposed to always be dark. Making it respect the color scheme produces a result that, to be honest, I don't think works. The proposed light theme version has a background that's eyeball-searingly bright.

Is there a problem with keeping this a hardcoded dark-only UI?