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