Review Request: Aspect Ratio Modes in Plasma::Dialog

[Giulio Camuffo]

> On 2009-11-18 01:25:47, Aaron Seigo wrote:
> > good idea; just a few tweaks needed.
> > 
> > "But if the applet changes its ratio mode while the dialog is being displayed it won't be get."
> > 
> > the way to accomplish this is to modify the implementation of Applet::setAspectRatioMode. add something like:
> > 
> > PopupApplet *popup = qobject_cast<PopupApplet *>(this);
> > if (popup && popup->d->dialogPtr) {
> >     popup->d->dialogPtr.data()->setAspectRatioMode(mode);
> > }

thanks for the tip :)


> On 2009-11-18 01:25:47, Aaron Seigo wrote:
> > /trunk/KDE/kdelibs/plasma/dialog.cpp, line 253
> > <http://reviewboard.kde.org/r/2202/diff/1/?file=14663#file14663line253>
> >
> >     a switch statement might be more evident / natural? it would also give us compiler warnings if we forget a mode

it could be. But the FixedSize case is not managed by that piece of code, so maybe there will be an unneded qarning.


> On 2009-11-18 01:25:47, Aaron Seigo wrote:
> > /trunk/KDE/kdelibs/plasma/dialog.cpp, lines 256-262
> > <http://reviewboard.kde.org/r/2202/diff/1/?file=14663#file14663line256>
> >
> >     qRound?

yea :). i'm not familiar at all with all that qMax, qMin, qRound, etc.


> On 2009-11-18 01:25:47, Aaron Seigo wrote:
> > /trunk/KDE/kdelibs/plasma/popupapplet.cpp, line 349
> > <http://reviewboard.kde.org/r/2202/diff/1/?file=14664#file14664line349>
> >
> >     this changes the behaviour; it should only set it to square if an icon widget has been created. this will set it to be square no matter what, which is incorrect.

yes, my fault. i moved that line because it needs to be called only once, and that function is called many times. I'll move it back inside the "if (icon) {" branch


- Giulio


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


On 2009-11-17 21:53:22, Giulio Camuffo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2202/
> -----------------------------------------------------------
> 
> (Updated 2009-11-17 21:53:22)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> This patch adds the aspect ratio modes to Plasma::Dialog too. It adds the methods setAspectRatioMode(Plasma::AspectRatioMode) and aspectRatioMode(). 
> Then I modified PopupApplet to let its dialog assume the aspectRatioMode of the applet when it is expanded.
> 
> This solves an issue with the Comic applet that uses KeepAspectRatio not to deform the comic. Without this, when shown in the dialog, there is no control about that.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdelibs/plasma/dialog.h 1050673 
>   /trunk/KDE/kdelibs/plasma/dialog.cpp 1050673 
>   /trunk/KDE/kdelibs/plasma/popupapplet.cpp 1050673 
> 
> Diff: http://reviewboard.kde.org/r/2202/diff
> 
> 
> Testing
> -------
> 
> Tried with all the aspect ratio modes. I'm actually a bit unsure about the ConstrainedSquare. It hasn't so much sense in a dialog and currently it behaves like the Square one. And the PopupApplet can't know when the mode gets changed so the mode of the dialog is changed in popupConstraintsEvent and in internalTogglePopup. But if the applet changes its ratio mode while the dialog is being displayed it won't be get.
> 
> 
> Thanks,
> 
> Giulio
> 
>