Review Request 120235: restore ResizeOrigin

[Vishesh Handa]

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


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


src/plasmaquick/dialog.cpp
<https://git.reviewboard.kde.org/r/120235/#comment46536>

    I'd updated the documentation of updateLayoutParameters to say that you must always call "syncToMainItemSize" before, this was done because the borders must be correct, which actually depends on the position.
    
    Are you sure calling syncBorders is enough? If it is then please update the documentation.



src/plasmaquick/dialog.cpp
<https://git.reviewboard.kde.org/r/120235/#comment46537>

    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.



src/plasmaquick/dialog.cpp
<https://git.reviewboard.kde.org/r/120235/#comment46538>

    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?



src/plasmaquick/dialog.cpp
<https://git.reviewboard.kde.org/r/120235/#comment46535>

    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



src/plasmaquick/dialog.cpp
<https://git.reviewboard.kde.org/r/120235/#comment46539>

    If you change the location doesn't the position also change? I imagine just syncing the borders won't be enough.



src/plasmaquick/dialog.cpp
<https://git.reviewboard.kde.org/r/120235/#comment46533>

    Why set this variable at all? It's being changed again at the end of the function and not being used anywhere else.



src/plasmaquick/dialog.cpp
<https://git.reviewboard.kde.org/r/120235/#comment46534>

    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.


- Vishesh Handa


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/5839ab32/attachment.html>