Review Request 124785: [Panelconfig] Reset the panel offset on alignment change

[David Edmundson]

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



desktoppackage/contents/configuration/panelconfiguration/Ruler.qml (line 48)
<https://git.reviewboard.kde.org/r/124785/#comment58317>

    PanelConfiguration.qml has this
    
    Connections {
            target: panel
            onOffsetChanged: ruler.offset = panel.offset
            onMinimumLengthChanged: ruler.minimumLength = panel.minimumLength
            onMaximumLengthChanged: ruler.maximumLength = panel.maximumLength
        }
    
    my thoughts are:
    
    1) with your change we're changing the ruler offset but not the panel offset and they'll get out of sync. Are you sure it's the right offset to change?
    
    2) All of that original code is really weird and should be done as bindings...then we can kill the Component.onCompleted in this class.
    (and yeah, I know it's not yours)


- David Edmundson


On Aug. 17, 2015, 9:31 a.m., David Kahles wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124785/
> -----------------------------------------------------------
> 
> (Updated Aug. 17, 2015, 9:31 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> -------
> 
> As offset and length have a different meaning in all alignments, the
> panel shifts on alignment change. This could result in wrong panel
> positions (e.g. panel shifted over a monitor border). The better approach
> would be the recalculation of all values, so that the panel stays at it's
> current position, but this would be error prone and complicated. As the
> panel alignment is rarely changed, it's not worth it. The more easy
> approach is just setting the panel offset to zero. This makes sure the
> panel has a valid position and size.
> 
> 
> Diffs
> -----
> 
>   desktoppackage/contents/configuration/panelconfiguration/Ruler.qml a31feb40598ba24a107f41ff3b3f823afaa89da6 
> 
> Diff: https://git.reviewboard.kde.org/r/124785/diff/
> 
> 
> Testing
> -------
> 
> When changing the alignment, the offset gets 0, so there are no invalid panel positions on alignment change
> 
> 
> Thanks,
> 
> David Kahles
> 
>

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