Skip to content

F
Front-End
Unverified Commit 56039b4b authored by Michael Goderbauer's avatar Michael Goderbauer Committed by GitHub
Browse files
Options

Make getOffsetToReveal work with nested Viewports (#26663) 

Fixes #20893, the most-upvoted bug for framework.

A detailed analysis explaining the cause of the bug has been posted at #20893 (comment).
parent 7e22b5f2
Hide whitespace changes
Inline Side-by-side
Showing with 143 additions and 29 deletions
packages/flutter/lib/src/rendering/viewport.dart
View file @ 56039b4b
......@@ -57,9 +57,9 @@ abstract class RenderAbstractViewport extends RenderObject {
/// edge of the viewport as possible. If `alignment` is 0.5, the child must be
/// positioned as close to the center of the viewport as possible.
///
/// The target might not be a direct child of this viewport but it must be a
/// descendant of the viewport and there must not be any other
/// [RenderAbstractViewport] objects between the target and this object.
/// The `target` might not be a direct child of this viewport but it must be a
/// descendant of the viewport. Other viewports in between this viewport and
/// the `target` will not be adjusted.
///
/// This method assumes that the content of the viewport moves linearly, i.e.
/// when the offset of the viewport is changed by x then `target` also moves
......@@ -585,26 +585,39 @@ abstract class RenderViewportBase<ParentDataClass extends ContainerParentDataMix
@override
RevealedOffset getOffsetToReveal(RenderObject target, double alignment, {Rect rect}) {
double leadingScrollOffset;
double leadingScrollOffset = 0.0;
double targetMainAxisExtent;
RenderObject descendant;
rect ??= target.paintBounds;
if (target is RenderBox) {
final RenderBox targetBox = target;
// The pivot will be the topmost child before we hit a RenderSliver.
RenderBox pivot = targetBox;
while (pivot.parent is RenderBox)
pivot = pivot.parent;
// Starting at `target` and walking towards the root:
// - `child` will be the last object before we reach this viewport, and
// - `pivot` will be the last RenderBox before we reach this viewport.
RenderObject child = target;
RenderBox pivot;
bool onlySlivers = target is RenderSliver; // ... between viewport and `target` (`target` included).
while (child.parent != this) {
assert(child.parent != null, '$target must be a descendant of $this');
if (child is RenderBox) {
pivot = child;
}
if (child.parent is RenderSliver) {
final RenderSliver parent = child.parent;
leadingScrollOffset += parent.childScrollOffset(child);
} else {
onlySlivers = false;
leadingScrollOffset = 0.0;
}
child = child.parent;
}
if (pivot != null) {
assert(pivot.parent != null);
assert(pivot.parent != this);
assert(pivot != this);
assert(pivot.parent is RenderSliver); // TODO(abarth): Support other kinds of render objects besides slivers.
final RenderSliver pivotParent = pivot.parent;
final Matrix4 transform = targetBox.getTransformTo(pivot);
final Matrix4 transform = target.getTransformTo(pivot);
final Rect bounds = MatrixUtils.transformRect(transform, rect);
final GrowthDirection growthDirection = pivotParent.constraints.growthDirection;
......@@ -619,15 +632,15 @@ abstract class RenderViewportBase<ParentDataClass extends ContainerParentDataMix
offset = bounds.top;
break;
}
leadingScrollOffset = pivot.size.height - offset;
leadingScrollOffset += pivot.size.height - offset;
targetMainAxisExtent = bounds.height;
break;
case AxisDirection.right:
leadingScrollOffset = bounds.left;
leadingScrollOffset += bounds.left;
targetMainAxisExtent = bounds.width;
break;
case AxisDirection.down:
leadingScrollOffset = bounds.top;
leadingScrollOffset += bounds.top;
targetMainAxisExtent = bounds.height;
break;
case AxisDirection.left:
......@@ -640,28 +653,17 @@ abstract class RenderViewportBase<ParentDataClass extends ContainerParentDataMix
offset = bounds.left;
break;
}
leadingScrollOffset = pivot.size.width - offset;
leadingScrollOffset += pivot.size.width - offset;
targetMainAxisExtent = bounds.width;
break;
}
descendant = pivot;
} else if (target is RenderSliver) {
} else if (onlySlivers) {
final RenderSliver targetSliver = target;
leadingScrollOffset = 0.0;
targetMainAxisExtent = targetSliver.geometry.scrollExtent;
descendant = targetSliver;
} else {
return RevealedOffset(offset: offset.pixels, rect: rect);
}
// The child will be the topmost object before we get to the viewport.
RenderObject child = descendant;
while (child.parent is RenderSliver) {
final RenderSliver parent = child.parent;
leadingScrollOffset += parent.childScrollOffset(child);
child = parent;
}
assert(child.parent == this);
assert(child is RenderSliver);
final RenderSliver sliver = child;
......
packages/flutter/test/rendering/viewport_test.dart
View file @ 56039b4b
......@@ -642,6 +642,118 @@ void main() {
});
});
testWidgets('Nested Viewports showOnScreen with allowImplicitScrolling=false for inner viewport', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/20893.
List<Widget> slivers;
final ScrollController controllerX = ScrollController(initialScrollOffset: 0.0);
final ScrollController controllerY = ScrollController(initialScrollOffset: 0.0);
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: Center(
child: Container(
height: 200.0,
width: 200.0,
child: ListView(
controller: controllerY,
children: <Widget>[
Container(
height: 150.0,
),
Container(
height: 100.0,
child: ListView(
physics: const PageScrollPhysics(), // Turns off `allowImplicitScrolling`
scrollDirection: Axis.horizontal,
controller: controllerX,
children: slivers = <Widget>[
Container(
width: 150.0,
),
Container(
width: 150.0,
),
],
),
),
Container(
height: 150.0,
),
],
),
),
),
),
);
tester.renderObject(find.byWidget(slivers[1])).showOnScreen();
await tester.pumpAndSettle();
expect(controllerX.offset, 0.0);
expect(controllerY.offset, 50.0);
});
testWidgets('Nested Viewports showOnScreen on Sliver with allowImplicitScrolling=false for inner viewport', (WidgetTester tester) async {
Widget sliver;
final ScrollController controllerX = ScrollController(initialScrollOffset: 0.0);
final ScrollController controllerY = ScrollController(initialScrollOffset: 0.0);
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: Center(
child: Container(
height: 200.0,
width: 200.0,
child: ListView(
controller: controllerY,
children: <Widget>[
Container(
height: 150.0,
),
Container(
height: 100.0,
child: CustomScrollView(
physics: const PageScrollPhysics(), // Turns off `allowImplicitScrolling`
scrollDirection: Axis.horizontal,
controller: controllerX,
slivers: <Widget>[
SliverPadding(
padding: const EdgeInsets.all(25.0),
sliver: SliverToBoxAdapter(
child: Container(
width: 100.0,
),
),
),
SliverPadding(
padding: const EdgeInsets.all(25.0),
sliver: sliver = SliverToBoxAdapter(
child: Container(
width: 100.0,
),
),
),
],
),
),
Container(
height: 150.0,
),
],
),
),
),
),
);
tester.renderObject(find.byWidget(sliver)).showOnScreen();
await tester.pumpAndSettle();
expect(controllerX.offset, 0.0);
expect(controllerY.offset, 25.0);
});
testWidgets('Viewport showOnScreen with objects larger than viewport', (WidgetTester tester) async {
List<Widget> children;
ScrollController controller;
......
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

HIAST - GitLab