BUG: 383040
KDecoration part of the patch:
https://phabricator.kde.org/D7246
graesslin |
BUG: 383040
KDecoration part of the patch:
https://phabricator.kde.org/D7246
Automatic diff as part of commit; lint not applicable. |
Automatic diff as part of commit; unit tests not applicable. |
kwin-5.11.3/decorations/decoratedclient.cpp | ||
---|---|---|
203 ↗ | (On Diff #23797) | I would prefer to have two dedicated methods to avoid a boolean trap. requestTooltip(false) is hardly descriptive without knowing the code. Perhaps requestShowToolTip(const QString &text) and requestHideToolTip() Shouldn't it be ToolTip (capital Tip)? Also pass argument as const QString & |
kwin-5.11.3/useractions.cpp | ||
1667 ↗ | (On Diff #23797) | Coding style, brace on its on line (for functions) |
kwin-5.11.3/decorations/decoratedclient.h | ||
---|---|---|
75 ↗ | (On Diff #23867) | What's the review id for the change in KDecoration? |
kwin-5.11.3/kcmkwin/kwindecoration/declarative-plugin/previewclient.cpp | ||
378–386 ↗ | (On Diff #23867) | we don't need that. Just remove the methods. |
kwin-5.11.3/useractions.cpp | ||
1669 ↗ | (On Diff #23867) | You cannot use QCursor in KWin. We have a dedicated cursor position class called "Cursor". |
1673 ↗ | (On Diff #23867) | coding style: { goes into new line |
kwin-5.11.3/workspace.h | ||
281–282 ↗ | (On Diff #23867) | I don't see a need for going through the Workspace. That can be done in DecoratedClientImpl directly. |
kwin-5.11.3/kcmkwin/kwindecoration/declarative-plugin/previewclient.cpp | ||
---|---|---|
378–386 ↗ | (On Diff #23867) | It fails to build without them: |
@graesslin thanks for review.
Can you look at new version of KDecoration part requested by @cfeck ?
Just ftr: I tried the complete changeset on Wayland and it works very nicely. Good work!