Unverified Commit 8bb61c38 authored by Dan Field's avatar Dan Field Committed by GitHub

Reland ensure visible fix for nested viewports (#67773)

* Revert "Revert "Improve the behavior of Scrollable.ensureVisible when Scrollable nested (#65226)" (#66918)"

This reverts commit e8812c40.

* Fix for page views

* comment
parent 43999247
...@@ -331,6 +331,29 @@ class _PagePosition extends ScrollPositionWithSingleContext implements PageMetri ...@@ -331,6 +331,29 @@ class _PagePosition extends ScrollPositionWithSingleContext implements PageMetri
final int initialPage; final int initialPage;
double _pageToUseOnStartup; double _pageToUseOnStartup;
@override
Future<void> ensureVisible(
RenderObject object, {
double alignment = 0.0,
Duration duration = Duration.zero,
Curve curve = Curves.ease,
ScrollPositionAlignmentPolicy alignmentPolicy = ScrollPositionAlignmentPolicy.explicit,
RenderObject? targetRenderObject,
}) {
// Since the _PagePosition is intended to cover the available space within
// its viewport, stop trying to move the target render object to the center
// - otherwise, could end up changing which page is visible and moving the
// targetRenderObject out of the viewport.
return super.ensureVisible(
object,
alignment: alignment,
duration: duration,
curve: curve,
alignmentPolicy: alignmentPolicy,
targetRenderObject: null,
);
}
@override @override
double get viewportFraction => _viewportFraction; double get viewportFraction => _viewportFraction;
double _viewportFraction; double _viewportFraction;
......
...@@ -647,6 +647,12 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics { ...@@ -647,6 +647,12 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics {
/// Animates the position such that the given object is as visible as possible /// Animates the position such that the given object is as visible as possible
/// by just scrolling this position. /// by just scrolling this position.
/// ///
/// The optional `targetRenderObject` parameter is used to determine which area
/// of that object should be as visible as possible. If `targetRenderObject`
/// is null, the entire [RenderObject] (as defined by its
/// [RenderObject.paintBounds]) will be as visible as possible. If
/// `targetRenderObject` is provided, it must be a descendant of the object.
///
/// See also: /// See also:
/// ///
/// * [ScrollPositionAlignmentPolicy] for the way in which `alignment` is /// * [ScrollPositionAlignmentPolicy] for the way in which `alignment` is
...@@ -657,25 +663,34 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics { ...@@ -657,25 +663,34 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics {
Duration duration = Duration.zero, Duration duration = Duration.zero,
Curve curve = Curves.ease, Curve curve = Curves.ease,
ScrollPositionAlignmentPolicy alignmentPolicy = ScrollPositionAlignmentPolicy.explicit, ScrollPositionAlignmentPolicy alignmentPolicy = ScrollPositionAlignmentPolicy.explicit,
RenderObject? targetRenderObject,
}) { }) {
assert(alignmentPolicy != null); assert(alignmentPolicy != null);
assert(object.attached); assert(object.attached);
final RenderAbstractViewport viewport = RenderAbstractViewport.of(object)!; final RenderAbstractViewport viewport = RenderAbstractViewport.of(object)!;
assert(viewport != null); assert(viewport != null);
Rect? targetRect;
if (targetRenderObject != null && targetRenderObject != object) {
targetRect = MatrixUtils.transformRect(
targetRenderObject.getTransformTo(object),
object.paintBounds.intersect(targetRenderObject.paintBounds)
);
}
double target; double target;
switch (alignmentPolicy) { switch (alignmentPolicy) {
case ScrollPositionAlignmentPolicy.explicit: case ScrollPositionAlignmentPolicy.explicit:
target = viewport.getOffsetToReveal(object, alignment).offset.clamp(minScrollExtent, maxScrollExtent); target = viewport.getOffsetToReveal(object, alignment, rect: targetRect).offset.clamp(minScrollExtent, maxScrollExtent);
break; break;
case ScrollPositionAlignmentPolicy.keepVisibleAtEnd: case ScrollPositionAlignmentPolicy.keepVisibleAtEnd:
target = viewport.getOffsetToReveal(object, 1.0).offset.clamp(minScrollExtent, maxScrollExtent); target = viewport.getOffsetToReveal(object, 1.0, rect: targetRect).offset.clamp(minScrollExtent, maxScrollExtent);
if (target < pixels) { if (target < pixels) {
target = pixels; target = pixels;
} }
break; break;
case ScrollPositionAlignmentPolicy.keepVisibleAtStart: case ScrollPositionAlignmentPolicy.keepVisibleAtStart:
target = viewport.getOffsetToReveal(object, 0.0).offset.clamp(minScrollExtent, maxScrollExtent); target = viewport.getOffsetToReveal(object, 0.0, rect: targetRect).offset.clamp(minScrollExtent, maxScrollExtent);
if (target > pixels) { if (target > pixels) {
target = pixels; target = pixels;
} }
......
...@@ -307,6 +307,13 @@ class Scrollable extends StatefulWidget { ...@@ -307,6 +307,13 @@ class Scrollable extends StatefulWidget {
}) { }) {
final List<Future<void>> futures = <Future<void>>[]; final List<Future<void>> futures = <Future<void>>[];
// The `targetRenderObject` is used to record the first target renderObject.
// If there are multiple scrollable widgets nested, we should let
// the `targetRenderObject` as visible as possible to improve the user experience.
// Otherwise, let the outer renderObject as visible as possible maybe cause
// the `targetRenderObject` invisible.
// Also see https://github.com/flutter/flutter/issues/65100
RenderObject? targetRenderObject;
ScrollableState? scrollable = Scrollable.of(context); ScrollableState? scrollable = Scrollable.of(context);
while (scrollable != null) { while (scrollable != null) {
futures.add(scrollable.position.ensureVisible( futures.add(scrollable.position.ensureVisible(
...@@ -315,7 +322,10 @@ class Scrollable extends StatefulWidget { ...@@ -315,7 +322,10 @@ class Scrollable extends StatefulWidget {
duration: duration, duration: duration,
curve: curve, curve: curve,
alignmentPolicy: alignmentPolicy, alignmentPolicy: alignmentPolicy,
targetRenderObject: targetRenderObject,
)); ));
targetRenderObject = targetRenderObject ?? context.findRenderObject();
context = scrollable.context; context = scrollable.context;
scrollable = Scrollable.of(context); scrollable = Scrollable.of(context);
} }
......
...@@ -224,6 +224,83 @@ void main() { ...@@ -224,6 +224,83 @@ void main() {
await tester.pump(); await tester.pump();
expect(tester.getTopLeft(findKey(0)).dy, moreOrLessEquals(500.0, epsilon: 0.1)); expect(tester.getTopLeft(findKey(0)).dy, moreOrLessEquals(500.0, epsilon: 0.1));
}); });
testWidgets('Nested SingleChildScrollView ensureVisible behavior test', (WidgetTester tester) async {
// Regressing test for https://github.com/flutter/flutter/issues/65100
Finder findKey(String coordinate) => find.byKey(ValueKey<String>(coordinate));
BuildContext findContext(String coordinate) => tester.element(findKey(coordinate));
final List<Row> rows = List<Row>.generate(
7,
(int y) => Row(
children: List<Container>.generate(
7,
(int x) => Container(key: ValueKey<String>('$x, $y'), width: 200.0, height: 200.0,),
),
),
);
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: Center(
child: SizedBox(
width: 600.0,
height: 400.0,
child: SingleChildScrollView(
scrollDirection: Axis.horizontal,
child: SingleChildScrollView(
scrollDirection: Axis.vertical,
child: Column(
children: rows,
),
),
),
),
),
),
);
// Items: 7 * 7 Container(width: 200.0, height: 200.0)
// viewport: Size(width: 600.0, height: 400.0)
//
// 0 600
// +----------------------+
// |0,0 |1,0 |2,0 |
// | | | |
// +----------------------+
// |0,1 |1,1 |2,1 |
// | | | |
// 400 +----------------------+
Scrollable.ensureVisible(findContext('0, 0'));
await tester.pump();
expect(tester.getTopLeft(findKey('0, 0')), const Offset(100.0, 100.0));
Scrollable.ensureVisible(findContext('3, 0'));
await tester.pump();
expect(tester.getTopLeft(findKey('3, 0')), const Offset(100.0, 100.0));
Scrollable.ensureVisible(findContext('3, 0'), alignment: 0.5);
await tester.pump();
expect(tester.getTopLeft(findKey('3, 0')), const Offset(300.0, 100.0));
Scrollable.ensureVisible(findContext('6, 0'));
await tester.pump();
expect(tester.getTopLeft(findKey('6, 0')), const Offset(500.0, 100.0));
Scrollable.ensureVisible(findContext('0, 2'));
await tester.pump();
expect(tester.getTopLeft(findKey('0, 2')), const Offset(100.0, 100.0));
Scrollable.ensureVisible(findContext('3, 2'));
await tester.pump();
expect(tester.getTopLeft(findKey('3, 2')), const Offset(100.0, 100.0));
// It should be at the center of the screen.
Scrollable.ensureVisible(findContext('3, 2'), alignment: 0.5);
await tester.pump();
expect(tester.getTopLeft(findKey('3, 2')), const Offset(300.0, 200.0));
});
}); });
group('ListView', () { group('ListView', () {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment