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
Lint Skipped |
Unit Tests Skipped |
kwin-5.11.3/decorations/decoratedclient.cpp | ||
---|---|---|
203 | 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 | Coding style, brace on its on line (for functions) |
kwin-5.11.3/decorations/decoratedclient.h | ||
---|---|---|
75 | What's the review id for the change in KDecoration? | |
kwin-5.11.3/kcmkwin/kwindecoration/declarative-plugin/previewclient.cpp | ||
378–386 | we don't need that. Just remove the methods. | |
kwin-5.11.3/useractions.cpp | ||
1669 | You cannot use QCursor in KWin. We have a dedicated cursor position class called "Cursor". | |
1673 | coding style: { goes into new line | |
kwin-5.11.3/workspace.h | ||
281–282 | 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 | 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!