> On Aug. 22, 2015, 11:42 p.m., David Edmundson wrote: > > desktoppackage/contents/configuration/panelconfiguration/Ruler.qml, line 48 > > <https://git.reviewboard.kde.org/r/124785/diff/1/?file=395466#file395466line48> > > > > 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)
1) Yes I am. Ruler.qml, line 30 has this: property alias offset: offsetHandle.value and line 75 (in the new Ruler.qml) has this: SliderHandle { id: offsetHandle graphicElementName: "offsetslider" onValueChanged: panel.offset = value property int position: (dialogRoot.vertical) ? y : x } So when I change `offset`, this means that panel.offset is changed too. 2) Yes thats right, but I think we should merge this now and do the refactoring later, because it works for now. Or do you consider this as a merge blocker? - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124785/#review84194 ----------------------------------------------------------- On Aug. 17, 2015, 11: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, 11: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 > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel