Scroller fix for Issue #561: Missing content-layout-offset updates in far-anchoring + overpanning modes #748
Conversation
… far-anchoring + overpanning modes.
|
|
||
| using (var setup = new TestSetupHelper("Scroller Tests")) | ||
| { | ||
| SetOutputDebugStringLevel("Verbose"); |
There was a problem hiding this comment.
Just for intermediate testing, or something you intend to check in?
| Log.Comment("scroller51.VerticalScrollPercent={0}", scroller51.VerticalScrollPercent); | ||
|
|
||
| // No layout offset is expected | ||
| double contentLayoutOffsetX = 0.0; |
There was a problem hiding this comment.
As pure out params you shouldn't need to initialize these
| (isForHorizontalDirection && scroller51.VerticalScrollPercent != 0.0) || | ||
| (!isForHorizontalDirection && scroller51.HorizontalScrollPercent != 0.0) || | ||
| (!isForHorizontalDirection && scroller51.VerticalScrollPercent != 100.0) || | ||
| (isForHorizontalDirection && contentLayoutOffsetX != 30.0) || |
There was a problem hiding this comment.
I don't really know what the logic is behind all these numbers. 30, 40, 10, 100, 240... they look like they're combinations of something but it's not obvious what.
| @@ -849,9 +850,10 @@ winrt::Size Scroller::ArrangeOverride(winrt::Size const& finalSize) | |||
| { | |||
| float unzoomedDelta = 0.0f; | |||
|
|
|||
| if (newUnzoomedExtentHeight > m_unzoomedExtentHeight) | |||
| if (newUnzoomedExtentHeight > m_unzoomedExtentHeight || // ExtentHeight grew | |||
There was a problem hiding this comment.
Identical logic except swapping Height for Width. Can you make a worker function to reduce that duplication?
| { | ||
| FrameworkElement content = scroller51.Content as FrameworkElement; | ||
|
|
||
| if (scroller51.HorizontalOffset > scroller51.ScrollableWidth + 20.0) |
There was a problem hiding this comment.
Where are these 20/30, 25/40 values coming from?
| @@ -259,6 +285,24 @@ private void MUXControlsTestHooks_LoggingMessage(object sender, MUXControlsTestH | |||
| } | |||
| } | |||
|
|
|||
| private void ScrollerTestHooks_ContentLayoutOffsetXChanged(Scroller sender, object args) | |||
| { | |||
| float contentLayoutOffsetX = 0.0f; | |||
There was a problem hiding this comment.
pure out value, doesn't need to be initialized
jevansaks
added
the
release note
|
🎉 Handy links: |
Fixes Issue #561.
The Scroller was unexpectedly shifting its Content position in these conditions:
The fix was to update the Scroller's content-layout-offset in these conditions.
(The typo fix for the RaiseViewportChanged call seen in #731 is also present in these changes).
The new Scroller interactive test covers the broken scenario. It checks the expected content-layout-offset.