Review Request: Aspect Ratio Modes in Plasma::Dialog

[Aaron Seigo]

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


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);
}


/trunk/KDE/kdelibs/plasma/dialog.cpp
<http://reviewboard.kde.org/r/2202/#comment2525>

    a switch statement might be more evident / natural? it would also give us compiler warnings if we forget a mode 



/trunk/KDE/kdelibs/plasma/dialog.cpp
<http://reviewboard.kde.org/r/2202/#comment2526>

    qRound?



/trunk/KDE/kdelibs/plasma/popupapplet.cpp
<http://reviewboard.kde.org/r/2202/#comment2527>

    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.


- Aaron


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
> 
>