Review Request 120235: restore ResizeOrigin

[Marco Martin]

> On Sept. 17, 2014, 2:54 p.m., Vishesh Handa wrote:
> > The following tests are broken -
> > 
> > 1. dialog_positioning.qml
> > 2. dialog_positioning2.qml - The animation seems incorrect. Maybe that was always different and I did not notice?
> > 3. dialog_visualParentChange.qml

1 and 3 were casued by replacing syncToMainItemSize(), it's fixed now
2 is ok: the lack of animation is caused by the window flags, kwin doesn't animate Qt::Popup windows


> On Sept. 17, 2014, 2:54 p.m., Vishesh Handa wrote:
> > src/plasmaquick/dialog.cpp, line 470
> > <https://git.reviewboard.kde.org/r/120235/diff/2/?file=312805#file312805line470>
> >
> >     Why? It's being done at the end of the function. What was breaking?
> >     
> >     Also, considering that updateTheme internally uses the frameSvgItem's size, we want to call it after the size has been updated.
> >     
> >     Unless I'm missing something?

before the first call of updateTheme() there is no svg loaded, so margins will always be 0


> On Sept. 17, 2014, 2:54 p.m., Vishesh Handa wrote:
> > src/plasmaquick/dialog.cpp, line 919
> > <https://git.reviewboard.kde.org/r/120235/diff/2/?file=312805#file312805line919>
> >
> >     If you change the location doesn't the position also change? I imagine just syncing the borders won't be enough.

the default position may change, yes.
getting back to calling syncToMainItemSize


> On Sept. 17, 2014, 2:54 p.m., Vishesh Handa wrote:
> > src/plasmaquick/dialog.cpp, line 1069
> > <https://git.reviewboard.kde.org/r/120235/diff/2/?file=312805#file312805line1069>
> >
> >     Oh. I didn't realize we had cases when the mainItem could be 0.
> >     
> >     Could you please add an assert in syncToMainItemSize for checking the mainItem.

yes, if the qml part doesn't assign it, there will be none,
syncToMainItem already checks if it does exists


> On Sept. 17, 2014, 2:54 p.m., Vishesh Handa wrote:
> > src/plasmaquick/dialog.cpp, line 946
> > <https://git.reviewboard.kde.org/r/120235/diff/2/?file=312805#file312805line946>
> >
> >     Why set this variable at all? It's being changed again at the end of the function and not being used anywhere else.

slotMainItemSizeChanged won't be executed in this case


> On Sept. 17, 2014, 2:54 p.m., Vishesh Handa wrote:
> > src/plasmaquick/dialog.cpp, line 301
> > <https://git.reviewboard.kde.org/r/120235/diff/2/?file=312805#file312805line301>
> >
> >     Would this actually do anything?
> >     
> >     resizeOrigin = MainItem;
> >     // do stuff which changes the size and sends a resizeEvent
> >     resizeOrigin = Undefined;
> >     
> >     Then the resizeEvent would be called with the resizeOrigin as "Undefined". Hmm, I belive there was going to be an assert.

when the width of mainItem is actually changed in this function, causes slotMainItemSizeChanged to be called


> On Sept. 17, 2014, 2:54 p.m., Vishesh Handa wrote:
> > src/plasmaquick/dialog.cpp, line 771
> > <https://git.reviewboard.kde.org/r/120235/diff/2/?file=312805#file312805line771>
> >
> >     So the visual parent changes, but the position is never updated? Only the borders synced? Wouldn't that break changing the visual parent.
> >     
> >     I believe I wrote a test for test - dialog_visualParentChange.qml

yep, the test was broken, fixed now


- Marco


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


On Sept. 17, 2014, 2:32 p.m., Marco Martin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120235/
> -----------------------------------------------------------
> 
> (Updated Sept. 17, 2014, 2:32 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/6035372d/attachment-0001.html>