D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

This revision was automatically updated to reflect the committed changes. Closed by commit R236:835c64d1b693: [KMessageWidget] Draw it with QPainter instead of using stylesheet (authored by davidre). REPOSITORY R236 KWidgetsAddons CHANGES SINCE LAST UPDATE

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

davidre added a comment. So I think this is ready to be merged and I would do so this weekend REPOSITORY R236 KWidgetsAddons BRANCH no-css (branched from master) REVISION DETAIL https://phabricator.kde.org/D27035 To: davidre, #frameworks, ngraham, cfeck Cc: apol, cfeck, dhaumann,

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

apol added a comment. +1 code looks much better! REPOSITORY R236 KWidgetsAddons BRANCH no-css (branched from master) REVISION DETAIL https://phabricator.kde.org/D27035 To: davidre, #frameworks, ngraham, cfeck Cc: apol, cfeck, dhaumann, ngraham, kde-frameworks-devel, LeGast00n,

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

cfeck accepted this revision. cfeck added a comment. Merci :) REPOSITORY R236 KWidgetsAddons BRANCH no-css (branched from master) REVISION DETAIL https://phabricator.kde.org/D27035 To: davidre, #frameworks, ngraham, cfeck Cc: cfeck, dhaumann, ngraham, kde-frameworks-devel, LeGast00n,

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

davidre updated this revision to Diff 76018. davidre marked 4 inline comments as done. davidre added a comment. Code style and typos REPOSITORY R236 KWidgetsAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27035?vs=75717=76018 BRANCH no-css (branched from master)

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

cfeck added inline comments. INLINE COMMENTS > kmessagewidget.cpp:155 > }; > - > +// Add bordersize to the margin so it starts from the inner border and > doesn't look to cramped > +q->layout()->setContentsMargins(q->layout()->contentsMargins() + > borderSize); too >

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

ngraham accepted this revision. This revision is now accepted and ready to land. REPOSITORY R236 KWidgetsAddons BRANCH no-css (branched from master) REVISION DETAIL https://phabricator.kde.org/D27035 To: davidre, #frameworks, ngraham Cc: cfeck, dhaumann, ngraham, kde-frameworks-devel,

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

davidre updated this revision to Diff 75717. davidre added a comment. remove debug remains REPOSITORY R236 KWidgetsAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27035?vs=75716=75717 BRANCH no-css (branched from master) REVISION DETAIL

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

davidre updated this revision to Diff 75716. davidre added a comment. Explicitly set TextColor and palettes of the labels and react to parent changes . For example necessary for konsole that uses stylesheet which break palette propagation and reparents the widget after creation.

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

ngraham added a comment. In D27035#604158 , @davidre wrote: > Something like this: > F8059669: Screenshot_20200131_163146.PNG That would be fine, yes. In the current code I took the approach

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

davidre added a comment. Ping, how should I proceed? REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D27035 To: davidre, #frameworks, ngraham Cc: cfeck, dhaumann, ngraham, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, bruns

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

davidre added a comment. Something like this: F8059669: Screenshot_20200131_163146.PNG REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D27035 To: davidre, #frameworks, ngraham Cc: cfeck, dhaumann, ngraham,

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

davidre added a comment. In D27035#604026 , @ngraham wrote: > Uh-oh, now it looks like this in Konsole: F8056116: Screenshot_20200131_064332.png > > Compare to the current appearance: F8056130:

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

ngraham requested changes to this revision. ngraham added a comment. This revision now requires changes to proceed. Uh-oh, now it looks like this in Konsole: F8056116: Screenshot_20200131_064332.png Compare to the current appearance: F8056130:

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

davidre added a comment. In D27035#603619 , @dhaumann wrote: > Better :) with corners I mean the 1-3 pixels left due to the rounding corners. These pixels were once also drawn as background although they are outside of the frame. It may be a

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

davidre updated this revision to Diff 74766. davidre marked an inline comment as done. davidre added a comment. Use parentWidget() REPOSITORY R236 KWidgetsAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27035?vs=74714=74766 BRANCH no-css (branched from master) REVISION

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

davidre added inline comments. INLINE COMMENTS > kmessagewidget.cpp:317 > +constexpr float alpha = 0.2; > +const auto parentWidget = qobject_cast(parent()); > +const QColor parentWindowColor = (parentWidget ? parentWidget->palette() > : qApp->palette()).color(QPalette::Window); I

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

davidre added inline comments. INLINE COMMENTS > kmessagewidget.cpp:39 > + > +constexpr int borderSize = 2; > + I don't think so: > internal linkage > [...] > variables, variable templates (since C++14), functions, or function templates > declared static; > non-volatile non-template (since

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

cfeck added inline comments. INLINE COMMENTS > kmessagewidget.cpp:39 > + > +constexpr int borderSize = 2; > + Does it need to be `static` to avoid an external symbol? REPOSITORY R236 KWidgetsAddons BRANCH no-css (branched from master) REVISION DETAIL https://phabricator.kde.org/D27035

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

dhaumann added a comment. Better :) with corners I mean the 1-3 pixels left due to the rounding corners. These pixels were once also drawn as background although they are outside of the frame. It may be a minor detail, but imho such details are important. But indeed, the screenshots look

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

davidre added a comment. Kate with this patch and Oxygen style: F8030118: Screenshot_20200130_181731.PNG The misplacement I spotted earlier is specific to QtCurve and also happens with the current KMessageWidget. REPOSITORY R236 KWidgetsAddons

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

davidre updated this revision to Diff 74714. davidre added a comment. Draw opaque and mix the colors manually again. REPOSITORY R236 KWidgetsAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27035?vs=74697=74714 BRANCH no-css (branched from master) REVISION DETAIL

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

davidre added a comment. In D27035#603460 , @dhaumann wrote: > Does that also work in Kate for floating messages like when the search wraps? What I want to know is whether the corners behind the green frame are transparent in this case, or

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

davidre added a comment. In D27035#603460 , @dhaumann wrote: > Does that also work in Kate for floating messages like when the search wraps? What I want to know is whether the corners behind the green frame are transparent in this case, or

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

dhaumann added a comment. Can you provide screenshots of more styles? :-) REPOSITORY R236 KWidgetsAddons BRANCH no-css (branched from master) REVISION DETAIL https://phabricator.kde.org/D27035 To: davidre, #frameworks, ngraham Cc: dhaumann, ngraham, kde-frameworks-devel, LeGast00n,

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

dhaumann added a comment. Does that also work in Kate for floating messages like when the search wraps? What I want to know is whether the corners behind the green frame are transparent in this case, or whether the corners are painted solid. If I remember correctly, these kind of bugs were

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

davidre added a dependent revision: D27038: [KMessageWidget] Allow the style to change our palette. REPOSITORY R236 KWidgetsAddons BRANCH no-css (branched from master) REVISION DETAIL https://phabricator.kde.org/D27035 To: davidre, #frameworks, ngraham Cc: ngraham, kde-frameworks-devel,

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

davidre added a comment. In D27035#603375 , @davidre wrote: > The border radius is slightly different because I copied what Kirigami does for the inner rect but I can also go back to radius 4 Nevermind that was because of above mistake

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

davidre edited the test plan for this revision. REPOSITORY R236 KWidgetsAddons BRANCH no-css (branched from master) REVISION DETAIL https://phabricator.kde.org/D27035 To: davidre, #frameworks, ngraham Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

davidre updated this revision to Diff 74697. davidre added a comment. Tiny mistakes REPOSITORY R236 KWidgetsAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27035?vs=74693=74697 BRANCH no-css (branched from master) REVISION DETAIL https://phabricator.kde.org/D27035

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

davidre added a comment. The border radius is slightly different because I copied what Kirigami does for the inner rect but I can also go back to radius 4 REPOSITORY R236 KWidgetsAddons BRANCH no-css (branched from master) REVISION DETAIL https://phabricator.kde.org/D27035 To:

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

ngraham accepted this revision. ngraham added a comment. This revision is now accepted and ready to land. Very nice. Much simpler and more comprehensible now. REPOSITORY R236 KWidgetsAddons BRANCH no-css (branched from master) REVISION DETAIL https://phabricator.kde.org/D27035 To:

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

davidre edited the test plan for this revision. REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D27035 To: davidre, #frameworks Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

davidre created this revision. davidre added a reviewer: Frameworks. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. davidre requested review of this revision. REVISION SUMMARY While being a prepatory step to KMessageWidget respecting the current color