Review Request 111319: Move kmessagewidget to kwidgetsaddons
Kevin Ottens
ervin at kde.org
Mon Jul 1 15:08:53 UTC 2013
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111319/#review35367
-----------------------------------------------------------
tier1/kwidgetsaddons/src/kmessagewidget.cpp
<http://git.reviewboard.kde.org/r/111319/#comment25909>
Could be changed to "Close this message" I guess. "Close document" has always been weird in that context after all. :-)
tier1/kwidgetsaddons/src/kmessagewidget.cpp
<http://git.reviewboard.kde.org/r/111319/#comment25910>
Unneeded it's not pushed in a menu bar.
tier1/kwidgetsaddons/src/kmessagewidget.cpp
<http://git.reviewboard.kde.org/r/111319/#comment25912>
Not needed in that context either.
tier1/kwidgetsaddons/src/kmessagewidget.cpp
<http://git.reviewboard.kde.org/r/111319/#comment25911>
You can drop the comment about shortcut here IMO.
tier1/kwidgetsaddons/src/kmessagewidget.cpp
<http://git.reviewboard.kde.org/r/111319/#comment25913>
I think it's more readable if you keep the variable declaration as it was:
QColor bg0, bg1, bg2, border, fg;
It's kind of odd to have bg1 declared separately and then the other ones.
tier1/kwidgetsaddons/src/kmessagewidget.cpp
<http://git.reviewboard.kde.org/r/111319/#comment25914>
Please keep the graphicsEffectLevel() block. Just put it between #if 0 and #endif. Also add a comment so that we can find it later.
We're preparing a patch to Qt to have an equivalent through the style()
tier1/kwidgetsaddons/src/kmessagewidget.cpp
<http://git.reviewboard.kde.org/r/111319/#comment25915>
Same thing here, reintroduce the block between #if 0 / #endif and a TODO comment.
- Kevin Ottens
On June 29, 2013, 8:24 p.m., Romário Rios wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111319/
> -----------------------------------------------------------
>
> (Updated June 29, 2013, 8:24 p.m.)
>
>
> Review request for KDE Frameworks.
>
>
> Description
> -------
>
> Moves kmessagewidget to kwidgetsaddons framework and removes all KDE dependencies from it. I don't know how to get the Close button shortcuts the same way it was gotten in kglobalactions, but I don't know if it's necessary in the case of this widget specifically.
>
>
> Diffs
> -----
>
> kdeui/CMakeLists.txt ab43f17
> kdeui/tests/CMakeLists.txt f489b6b
> kdeui/tests/kmessagewidgettest.cpp eb151b9
> kdeui/widgets/kmessagewidget.h 9fbb176
> kdeui/widgets/kmessagewidget.cpp 7fb91e8
> tier1/kwidgetsaddons/src/CMakeLists.txt 5a37efa
> tier1/kwidgetsaddons/src/kmessagewidget.h PRE-CREATION
> tier1/kwidgetsaddons/src/kmessagewidget.cpp PRE-CREATION
> tier1/kwidgetsaddons/tests/CMakeLists.txt 9738c6c
> tier1/kwidgetsaddons/tests/kmessagewidgettest.cpp PRE-CREATION
>
> Diff: http://git.reviewboard.kde.org/r/111319/diff/
>
>
> Testing
> -------
>
> Compiles, test runs fine.
>
>
> Thanks,
>
> Romário Rios
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20130701/8f3c2916/attachment.html>
More information about the Kde-frameworks-devel
mailing list