D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

ngraham added a comment.


  Submitted D12831: Don't automatically set the default icons for each style 
 to correct the issues.

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg, cfeck
Cc: kde-frameworks-devel, dhaumann, broulik, anemeth, abetts, cfeck, mart, 
fabianr, elvisangelaccio, jnoack, michaelh, ngraham, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Dominik Haumann]

dhaumann added a comment.


  I think the setIcon() is also buggy, since if you call setIcon() and then 
setMessageType(), the icon is lost. So yes, I would be in favor of a better 
solution.
  
  If you revert the setIcon() part, do the screenshot examples all loose their 
icon? Or is it still set explicitly in the respective application code?

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg, cfeck
Cc: kde-frameworks-devel, dhaumann, broulik, anemeth, abetts, cfeck, mart, 
fabianr, elvisangelaccio, jnoack, michaelh, ngraham, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

ngraham added a comment.


  Sure, I'll be happy to fix those issues.
  
  The additional `setIcon()` call is to match the behavior of the Kirigami 
version, which always has an icon by default. I can revert that if we're okay 
with having it be a little bit inconsistent with the Kirigami version.

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg, cfeck
Cc: kde-frameworks-devel, dhaumann, broulik, anemeth, abetts, cfeck, mart, 
fabianr, elvisangelaccio, jnoack, michaelh, ngraham, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Dominik Haumann]

dhaumann added a comment.


  Somehow my other comments were lost, here we go:
  
  - could you also update the screenshot in the doxygen documentation?
  - setIcon() is behavior incompatible, and in fact, the referenced bugs did 
not complain about icons. So why the change? In my opinion this is not good 
enough in case of Kate/KWrite.
  - setIcon(): Now the API documentation in the header file is wrong, since it 
says by default no icon is set.
  
  @ngraham Could we have another revision?

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg, cfeck
Cc: kde-frameworks-devel, dhaumann, broulik, anemeth, abetts, cfeck, mart, 
fabianr, elvisangelaccio, jnoack, michaelh, ngraham, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Dominik Haumann]

dhaumann added a comment.
Restricted Application edited subscribers, added: kde-frameworks-devel; 
removed: Frameworks.


  Can we revert the setIcon() part? It changes application behavior, an in the 
case of Kate/KWrite, this is note wanted, see my comments.

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg, cfeck
Cc: kde-frameworks-devel, dhaumann, broulik, anemeth, abetts, cfeck, mart, 
fabianr, elvisangelaccio, jnoack, michaelh, ngraham, bruns, #frameworks

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

ngraham closed this revision.

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg, cfeck
Cc: broulik, anemeth, abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, 
#frameworks, michaelh, ngraham, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Eike Hein]

hein accepted this revision.
hein added a comment.


  Looks good to me, thanks for working on this.

REPOSITORY
  R236 KWidgetsAddons

BRANCH
  arcpatch-D12508

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg, cfeck
Cc: broulik, anemeth, abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, 
#frameworks, michaelh, ngraham, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Andres Betts]

abetts added a comment.


  +1 on this!

REPOSITORY
  R236 KWidgetsAddons

BRANCH
  arcpatch-D12508

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg, cfeck
Cc: broulik, anemeth, abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, 
#frameworks, michaelh, ngraham, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

ngraham added a comment.


  Thanks! I'm waiting for at least @hein before landing it.

REPOSITORY
  R236 KWidgetsAddons

BRANCH
  arcpatch-D12508

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg, cfeck
Cc: broulik, anemeth, abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, 
#frameworks, michaelh, ngraham, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Christoph Feck]

cfeck accepted this revision.
cfeck added a comment.


  My "ship it" still stands, especially after you resolved the transparency 
issue :)

REPOSITORY
  R236 KWidgetsAddons

BRANCH
  arcpatch-D12508

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg, cfeck
Cc: broulik, anemeth, abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, 
#frameworks, michaelh, ngraham, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

ngraham edited the test plan for this revision.

REPOSITORY
  R236 KWidgetsAddons

BRANCH
  arcpatch-D12508

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg, cfeck
Cc: broulik, anemeth, abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, 
#frameworks, michaelh, ngraham, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

ngraham removed a dependency: D12523: Fix overlapping warning widget.

REPOSITORY
  R236 KWidgetsAddons

BRANCH
  arcpatch-D12508

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg, cfeck
Cc: broulik, anemeth, abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, 
#frameworks, michaelh, ngraham, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

ngraham updated this revision to Diff 33786.
ngraham added a comment.
This revision is now accepted and ready to land.


  Pre-generate the final background color to resolve extra border issue

REPOSITORY
  R236 KWidgetsAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12508?vs=2=33786

BRANCH
  arcpatch-D12508

REVISION DETAIL
  https://phabricator.kde.org/D12508

AFFECTED FILES
  src/kmessagewidget.cpp
  src/kmessagewidget.h

To: ngraham, hein, #plasma, #vdg, cfeck
Cc: broulik, anemeth, abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, 
#frameworks, michaelh, ngraham, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

ngraham planned changes to this revision.

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg, cfeck
Cc: broulik, anemeth, abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, 
#frameworks, michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

ngraham edited the test plan for this revision.

REPOSITORY
  R236 KWidgetsAddons

BRANCH
  arcpatch-D12508

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg, cfeck
Cc: broulik, anemeth, abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, 
#frameworks, michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

ngraham edited the test plan for this revision.

REPOSITORY
  R236 KWidgetsAddons

BRANCH
  arcpatch-D12508

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg, cfeck
Cc: broulik, anemeth, abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, 
#frameworks, michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

ngraham updated this revision to Diff 2.
ngraham added a comment.
This revision is now accepted and ready to land.


  Match inlineMessage with border thickness and corner roundness

REPOSITORY
  R236 KWidgetsAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12508?vs=33265=2

BRANCH
  arcpatch-D12508

REVISION DETAIL
  https://phabricator.kde.org/D12508

AFFECTED FILES
  src/kmessagewidget.cpp

To: ngraham, hein, #plasma, #vdg, cfeck
Cc: broulik, anemeth, abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, 
#frameworks, michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Elvis Angelaccio]

elvisangelaccio resigned from this revision.
elvisangelaccio added a comment.


  I'll just leave my +1 to the visual change.

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg, cfeck
Cc: broulik, anemeth, abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, 
#frameworks, michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

ngraham planned changes to this revision.

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg, elvisangelaccio, cfeck
Cc: broulik, anemeth, abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, 
#frameworks, michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

ngraham edited the summary of this revision.

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg, elvisangelaccio, cfeck
Cc: broulik, anemeth, abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, 
#frameworks, michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

ngraham updated this revision to Diff 33265.
ngraham added a comment.


  Further improvements; always draw the theme background color under the 
KMessageWidget, which handles the case of the theme and active window defining 
different colors.
  
  One remaining issue: the KMessageWidget now has a 1px border around it that 
may be the wrong color.

REPOSITORY
  R236 KWidgetsAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12508?vs=33254=33265

BRANCH
  arcpatch-D12508

REVISION DETAIL
  https://phabricator.kde.org/D12508

AFFECTED FILES
  src/kmessagewidget.cpp

To: ngraham, hein, #plasma, #vdg, elvisangelaccio, cfeck
Cc: broulik, anemeth, abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, 
#frameworks, michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

ngraham updated this revision to Diff 33254.
ngraham added a comment.


  set autoFillBackground, which fixes most client use cases except for when the 
parent widget's palette's background color matches the theme's text color (as 
with a dark Konsole theme on Breeze light)

REPOSITORY
  R236 KWidgetsAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12508?vs=33089=33254

BRANCH
  arcpatch-D12508

REVISION DETAIL
  https://phabricator.kde.org/D12508

AFFECTED FILES
  src/kmessagewidget.cpp

To: ngraham, hein, #plasma, #vdg, elvisangelaccio, cfeck
Cc: broulik, anemeth, abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, 
#frameworks, michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

ngraham added a comment.


  I'm working on setting the background color within the KMessageWidget itself, 
which should make these implementation issues in individual apps non-blockers.

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg, elvisangelaccio, cfeck
Cc: broulik, anemeth, abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, 
#frameworks, michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

ngraham added a comment.


  In D12508#255158 , @jnoack wrote:
  
  > Imo KMessageWidget should not rely on being placed where there's enough 
contrast. The transparency just doesn't work in the case of Konsole.
  
  
  Hmm, I see your point. Let me see what I can do with it...

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg, elvisangelaccio, cfeck
Cc: broulik, anemeth, abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, 
#frameworks, michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Joshua Noack]

jnoack added a comment.


  Imo KMessageWidget should not rely on being placed where there's enough 
contrast. The transparency just doesn't work in the case of Konsole.
  Either Konsole needs to move the widget outside the terminal painting area 
which looks complicated to achieve or the message widget should provide some API
  which apps can use in cases where the background has not enough contrast.
  
  In D12508#253854 , @cfeck wrote:
  
  > Regarding the Konsole isse, it may not be the only application expecting a 
window background. Maybe some autoFillBackground helps, see 
http://doc.qt.io/qt-5/qwidget.html#autoFillBackground-prop
  
  
  I just tried setting autoFillBackground: The background is not animated, so 
it breaks the animation of the message widget. Also it is painted black. I 
guess those things could be fixed within the KMessageWidget.
  
  The question is what makes more sense, fixing each application where the 
message widget looks broken or fixing the message widget to look not broken. 樂

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg, elvisangelaccio, cfeck
Cc: broulik, anemeth, abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, 
#frameworks, michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Christoph Feck]

cfeck added a comment.


  Regarding the Konsole isse, it may not be the only application expecting a 
window background. Maybe some autofillBackground helps.

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg, elvisangelaccio, cfeck
Cc: broulik, anemeth, abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, 
#frameworks, michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Christoph Feck]

cfeck accepted this revision.

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg, elvisangelaccio, cfeck
Cc: broulik, anemeth, abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, 
#frameworks, michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

ngraham edited the summary of this revision.
ngraham added a dependency: D12523: Fix overlapping warning widget.

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg, elvisangelaccio, cfeck
Cc: broulik, anemeth, abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, 
#frameworks, michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Kai Uwe Broulik]

broulik added a comment.


  Sure! It has to work without Breeze and on other platforms anyway but for a 
unified look we could add some special Breeze code on top maybe :)

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg, elvisangelaccio, cfeck
Cc: broulik, anemeth, abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, 
#frameworks, michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

ngraham added a comment.


  In D12508#253782 , @broulik wrote:
  
  > Can we maybe do something in Breeze widget style to override the colors 
with theme colors? Breeze does some special behavior for other widgets as well 
like `KTitleWidget`. Might be worth looking into?
  
  
  An interesting idea. Worth mentioning that nothing's being lost here, though; 
the colors were already hardcoded. Making them respect the theme would be 
lovely, but could be done as a separate patch after (if?) this goes in.

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg, elvisangelaccio, cfeck
Cc: broulik, anemeth, abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, 
#frameworks, michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Kai Uwe Broulik]

broulik added a comment.


  Can we maybe do something in Breeze widget style to override the colors with 
theme colors? Breeze does some special behavior for other widgets as well like 
`KTitleWidget`. Might be worth looking into?

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg, elvisangelaccio, cfeck
Cc: broulik, anemeth, abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, 
#frameworks, michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

ngraham marked an inline comment as done.

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg, elvisangelaccio, cfeck
Cc: anemeth, abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, 
#frameworks, michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

ngraham added a subscriber: anemeth.

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg, elvisangelaccio, cfeck
Cc: anemeth, abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, 
#frameworks, michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

ngraham added a comment.


  In D12508#253724 , @cfeck wrote:
  
  > The gradient was a remnant from the Oxygen days. Glad that it's gone.
  >
  > ~~Could you please restore the comments that state where the colors were 
taken from?~~
  >
  > EDIT: Ah, you added a separate comment above...
  
  
  I did, but I decided to take your advice and add more anyway for maximum 
clarity.

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg, elvisangelaccio, cfeck
Cc: abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, #frameworks, 
michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

ngraham updated this revision to Diff 33089.
ngraham added a comment.


  Address @cfeck's review comments

REPOSITORY
  R236 KWidgetsAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12508?vs=33063=33089

BRANCH
  arcpatch-D12508

REVISION DETAIL
  https://phabricator.kde.org/D12508

AFFECTED FILES
  src/kmessagewidget.cpp

To: ngraham, hein, #plasma, #vdg, elvisangelaccio, cfeck
Cc: abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, #frameworks, 
michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Andres Betts]

abetts added a comment.


  +1, looking good. Keep up the work!

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg, elvisangelaccio, cfeck
Cc: abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, #frameworks, 
michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Christoph Feck]

cfeck requested changes to this revision.
cfeck added a comment.
This revision now requires changes to proceed.


  The gradient was a remnant from the Oxygen days. Glad that it's gone.
  
  Could you please restore the comments that state where the colors were taken 
from?

INLINE COMMENTS

> kmessagewidget.cpp:286
> +// Adjust the background color to match the Kirigami InlineMessage
> +bg.setAlpha(51); // == 20% opacity (255 * 0.2 = 51)
>  

If you use setAlphaF(0.2) you don't need to explain the value.

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg, elvisangelaccio, cfeck
Cc: cfeck, mart, fabianr, elvisangelaccio, jnoack, #frameworks, michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Marco Martin]

mart added inline comments.

INLINE COMMENTS

> elvisangelaccio wrote in CMakeLists.txt:20
> Unfortunately we can't do this, kwidgetsaddons is tier1 and it cannot depend 
> on another framework.
> 
> We'll need to duplicate the relevant code from kconfigwidgets here.

if one doesn't fear horrible and beoken code... the color scheme could be read 
by QSettings... tough not sure if the hack is worth it

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg, elvisangelaccio
Cc: mart, fabianr, elvisangelaccio, jnoack, #frameworks, michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

ngraham edited the summary of this revision.

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg, elvisangelaccio
Cc: fabianr, elvisangelaccio, jnoack, #frameworks, michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

ngraham marked an inline comment as done.

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg, elvisangelaccio
Cc: fabianr, elvisangelaccio, jnoack, #frameworks, michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

ngraham added a comment.


  What a shame; this means that it won't inherit background colors from the 
active theme. I guess that's why it was implemented this way before. I've 
reverted that part and added a comment into the code so future folks aren't 
tripped up as I was.
  
  Still more red than green in this diff though!

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg, elvisangelaccio
Cc: fabianr, elvisangelaccio, jnoack, #frameworks, michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

ngraham updated this revision to Diff 33063.
ngraham added a comment.


  Remove KConfigWdgets dependency and revert to hardcoded background colors

REPOSITORY
  R236 KWidgetsAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12508?vs=33028=33063

BRANCH
  arcpatch-D12508

REVISION DETAIL
  https://phabricator.kde.org/D12508

AFFECTED FILES
  src/kmessagewidget.cpp

To: ngraham, hein, #plasma, #vdg, elvisangelaccio
Cc: fabianr, elvisangelaccio, jnoack, #frameworks, michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Elvis Angelaccio]

elvisangelaccio requested changes to this revision.
elvisangelaccio added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> CMakeLists.txt:20
>  
> +find_package(KF5ConfigWidgets ${KF5_VERSION} REQUIRED)
>  

Unfortunately we can't do this, kwidgetsaddons is tier1 and it cannot depend on 
another framework.

We'll need to duplicate the relevant code from kconfigwidgets here.

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg, elvisangelaccio
Cc: elvisangelaccio, jnoack, #frameworks, michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

ngraham added a subscriber: jnoack.

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg
Cc: jnoack, #frameworks, michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

ngraham edited the test plan for this revision.

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg
Cc: #frameworks, michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

ngraham edited the summary of this revision.
ngraham edited the test plan for this revision.

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D12508

To: ngraham, hein, #plasma, #vdg
Cc: #frameworks, michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

ngraham created this revision.
ngraham added reviewers: hein, Plasma, VDG.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.
ngraham requested review of this revision.

REVISION SUMMARY
  This patch makes the venerable and wonderful `KMessageWidget` match the slick 
style of Kirigami's new `inlineMessage` control. As a by-product, it correctly 
takes colors from the user's theme and fixes a few usability bugs.
  
  BUG: 385299
  BUG: 381255

TEST PLAN
  Spectacle, Breeze light, information:
  
  Spectable, Breeze light, positive:
  
  Dolphin, Breeze dark: warning:
  
  Konsole, light, warning:
  
  Oops, doesn't work! We'll need to fix Konsole to put a proper background 
behind it rather than overlapping the content.
  
  Style comparison between KMessageWidget and inlineMessage:

REPOSITORY
  R236 KWidgetsAddons

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D12508

AFFECTED FILES
  CMakeLists.txt
  src/CMakeLists.txt
  src/kmessagewidget.cpp

To: ngraham, hein, #plasma, #vdg
Cc: #frameworks, michaelh, bruns