From kde-core-devel  Fri Apr 30 09:31:43 2010
From: "David Faure" <faure () kde ! org>
Date: Fri, 30 Apr 2010 09:31:43 +0000
To: kde-core-devel
Subject: Re: Review Request: KPixmapRegionSelectorDialog fix: Enforce maximum
Message-Id: <20100430093143.10622.37276 () localhost>
X-MARC-Message: https://marc.info/?l=kde-core-devel&m=127261988026609


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3413/#review5320
-----------------------------------------------------------

Ship it!


Looks fine to me, including the private slot.

- David


On 2010-03-27 20:00:22, Darío Andrés wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3413/
> -----------------------------------------------------------
> 
> (Updated 2010-03-27 20:00:22)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> - Remove some code duplication
> - Enforce the maximum width/height on the KPixmapRegionSelectorWidget, so the dialog doesn't overflow the screen size
> 
> I had also to fix some code on KPixmapRegionSelectorWidget, otherwise it would create a false selection (when there were none) after the rotation.
> 
> Additions: centering the dialog in the screen after the rotation could be useful (specially for some rectangle images)
> 
> The name of the functions and/or the apidox could be improved...
> 
> 
> This addresses bug 227659.
>     https://bugs.kde.org/show_bug.cgi?id=227659
> 
> 
> Diffs
> -----
> 
>   svn://anonsvn.kde.org/home/kde/trunk/KDE/kdelibs/kdeui/dialogs/kpixmapregionselectordialog.h 1107102 
>   svn://anonsvn.kde.org/home/kde/trunk/KDE/kdelibs/kdeui/dialogs/kpixmapregionselectordialog.cpp 1107102 
>   svn://anonsvn.kde.org/home/kde/trunk/KDE/kdelibs/kdeui/widgets/kpixmapregionselectorwidget.h 1107102 
>   svn://anonsvn.kde.org/home/kde/trunk/KDE/kdelibs/kdeui/widgets/kpixmapregionselectorwidget.cpp 1107102 
> 
> Diff: http://reviewboard.kde.org/r/3413/diff
> 
> 
> Testing
> -------
> 
> The dialog works OK, the resize is done properly and the dialog size remains the same.
> 
> I'm not really sure if my approach of "Slots inside a private class" is OK.... Could someone check it ?
> 
> 
> Thanks,
> 
> Darío
> 
>