[kde-edu]: Re: Review Request: Configuration plugin for the coordinate lines.
Torsten Rahn
rahn at kde.org
Wed Jan 5 18:44:05 CET 2011
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/6281/#review9533
-----------------------------------------------------------
Looks good. You still need to fix the issues mentioned below :-)
/trunk/KDE/kdeedu/marble/src/plugins/render/graticule/GraticulePlugin.h
<http://svn.reviewboard.kde.org/r/6281/#comment10540>
No need to have the color dialog as a member.
/trunk/KDE/kdeedu/marble/src/plugins/render/graticule/GraticulePlugin.cpp
<http://svn.reviewboard.kde.org/r/6281/#comment10541>
no need to have the color dialogs created as members.
/trunk/KDE/kdeedu/marble/src/plugins/render/graticule/GraticulePlugin.cpp
<http://svn.reviewboard.kde.org/r/6281/#comment10535>
No :-)
You don't need the QColorDialog as a member. This doesn't make sense. Also don't assign the color that gets returned as a member before you know it's valid.
Just use:
QColor c;
c = QColorDialog::getColor( m_gridColor, 0, "Please choose the color for the coordinate grid" );
if ( c.isValid() ) {
[...]
}
/trunk/KDE/kdeedu/marble/src/plugins/render/graticule/GraticulePlugin.cpp
<http://svn.reviewboard.kde.org/r/6281/#comment10536>
Like this you create an "empty" default palette and change a single color of that one.
But that's not what you want.
You want to pick the palette that is already used by the button and change a single color:
QPalette gridPalette = ui_configWidget->gridPushButton->palette();
/trunk/KDE/kdeedu/marble/src/plugins/render/graticule/GraticulePlugin.cpp
<http://svn.reviewboard.kde.org/r/6281/#comment10537>
Please adjust the same way as for the gridGetColor method!
/trunk/KDE/kdeedu/marble/src/plugins/render/graticule/GraticulePlugin.cpp
<http://svn.reviewboard.kde.org/r/6281/#comment10538>
Please adjust the same way as for the gridGetColor method!
/trunk/KDE/kdeedu/marble/src/plugins/render/graticule/GraticulePlugin.cpp
<http://svn.reviewboard.kde.org/r/6281/#comment10539>
Since the changes of the about dialog are already in trunk it doesn't make sense to commit them again :-)
- Torsten
On 2011-01-05 17:13:59, Cezar Mocan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/6281/
> -----------------------------------------------------------
>
> (Updated 2011-01-05 17:13:59)
>
>
> Review request for KDE-Edu, marble and Torsten Rahn.
>
>
> Summary
> -------
>
> Configuration plugin for the coordinate lines.
>
>
> Diffs
> -----
>
> /trunk/KDE/kdeedu/marble/src/plugins/render/graticule/CMakeLists.txt 1201473
> /trunk/KDE/kdeedu/marble/src/plugins/render/graticule/GraticuleConfigWidget.ui PRE-CREATION
> /trunk/KDE/kdeedu/marble/src/plugins/render/graticule/GraticulePlugin.h 1201473
> /trunk/KDE/kdeedu/marble/src/plugins/render/graticule/GraticulePlugin.cpp 1201473
>
> Diff: http://svn.reviewboard.kde.org/r/6281/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Cezar
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-edu/attachments/20110105/1ac7b004/attachment.htm
More information about the kde-edu
mailing list