Review Request 120235: restore ResizeOrigin

[Marco Martin]

> On Sept. 17, 2014, 11:24 a.m., Vishesh Handa wrote:
> > src/plasmaquick/dialog.cpp, line 922
> > <https://git.reviewboard.kde.org/r/120235/diff/1/?file=312579#file312579line922>
> >
> >     I'm really not sure what is going to happen in this case -
> >     
> >     We set the resizeOrigin to Window, call updateLayoutParameters which changes the size of the window, which would call another resize event, which would recall this function ... This wouldn't actually be an infinite loops because at some point the new/old size would be the same and that would not create any more resizeEvents.
> >     
> >     It would be nice to have code paths which we can easily follow.

yes, calling d->updateLayoutParameters() is not correct, it should just resize the main item, i'll update the patch


> On Sept. 17, 2014, 11:24 a.m., Vishesh Handa wrote:
> > src/plasmaquick/dialog.cpp, line 913
> > <https://git.reviewboard.kde.org/r/120235/diff/1/?file=312579#file312579line913>
> >
> >     You would need to set this 'resizeOrigin' variable in 5 other places as well. Otherwise we will have strange issues of functions triggering each other.
> >     
> >     I think the places are -
> >     * updateMinimumWidth
> >     * updateMinimumHeight
> >     * updateMaximumWidth
> >     * updateMaximumHeight
> >     * updateLayoutParameters
> >     
> >     This is fundamentally why I'm against this double approach. It seems too error prone.

not sure about updateLayoutParameters, but for sure in updateMinimumWidth/maximum etc


> On Sept. 17, 2014, 11:24 a.m., Vishesh Handa wrote:
> > src/plasmaquick/dialog.cpp, line 551
> > <https://git.reviewboard.kde.org/r/120235/diff/1/?file=312579#file312579line551>
> >
> >     Hmm. reiszeOrigin != MainItem?
> >     
> >     Maybe we should assert if the resizeOrigin is ever undefined and this function is called?

+1 for assert


- Marco


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120235/#review66724
-----------------------------------------------------------


On Sept. 16, 2014, 3:52 p.m., Marco Martin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120235/
> -----------------------------------------------------------
> 
> (Updated Sept. 16, 2014, 3:52 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> -------
> 
> A dialog can be resize for two reasons: the mainItem size changes, or the dialog size changes.
> 
> the first can happen programmatically, caused by the Layout, or just by assigning the width.
> 
> the second can be caused either programmatically, assigning the size of the dialog or externally by the windowmanager, that is the only one theat in the end has the only final control of the window size
> 
> 
> Diffs
> -----
> 
>   src/plasmaquick/dialog.cpp 79a871b 
>   tests/dialog_minWidthHeightRepositioning.qml 37bd622 
> 
> Diff: https://git.reviewboard.kde.org/r/120235/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20140917/a2837aaf/attachment.html>