Wayland don't allow to update client position so we use PlasmaQuick/Dialog
PlasmaQuick/Dialog will talk to kwin throught plasma shell protocol to set dialog position
BUG: 385672
romangg | |
davidedmundson |
Plasma |
Wayland don't allow to update client position so we use PlasmaQuick/Dialog
PlasmaQuick/Dialog will talk to kwin throught plasma shell protocol to set dialog position
BUG: 385672
Lint Skipped |
Unit Tests Skipped |
Buildable 25226 | |
Build 25244: arc lint + arc unit |
kcm/output_identifier.cpp | ||
---|---|---|
62 | view leaks, no? I see it's not a problem in this patch. |
Technically this introduces a visual change.
Dialog draws a border and shadow by default which appears outside our red border from the QML.
I'm pretty sure you can disable it in Dialog, but frankly this looks way better so maybe it's a feature.
kcm/output_identifier.cpp | ||
---|---|---|
62 | Not part of your change, but we would leak view here. |
Conceptually makes sense. +1 form me on that. Good solution. Thanks for outlining it in the summary.
This patch can land with a fix for the view leak or without. I accept when my inline comments are fixed.
@davidedmundson: can you give a quick accept on the solution from PlasmaQuick pov?
kcm/output_identifier.cpp | ||
---|---|---|
61 | This part could use some empty lines. |
Using the protocol makes sense, using a class that does it already makes more sense than duplicating/
Especially as the other OSD is already a dialog.
From a plasma side, ship it
Thanks. Some minor remaining nitpicks you can change on push.
kcm/output_identifier.cpp | ||
---|---|---|
28 | Lexicographical order. Also add an empty line to differentiate include groups: #include <QStandardPaths> #include <QTimer> #include <QQuickItem> #include <QQuickView> #include <KDeclarative/kdeclarative/qmlobject.h> #include <PlasmaQuick/Dialog> | |
72 | You wanted to add a . but it became a " |
Fix import order (remove unused one)
Remove QTimer m_timer (field not used). Let me know if you want this one in a separate commit.
Yes, please put the m_timer removal in a separate patch. And (in this separate patch) if you removed the m_timer the QTimer include in the header file can be removed as well I assume. Thanks.