Unverified Commit 3509a1df authored by Kate Lovett's avatar Kate Lovett Committed by GitHub

Remove assertions on getOffsetToReveal (#135634)

parent 55c7a2ef
...@@ -1125,11 +1125,8 @@ class RenderListWheelViewport ...@@ -1125,11 +1125,8 @@ class RenderListWheelViewport
RenderObject target, RenderObject target,
double alignment, { double alignment, {
Rect? rect, Rect? rect,
Axis? axis, Axis? axis, // Unused, only Axis.vertical supported by this viewport.
}) { }) {
// One dimensional viewport has only one axis, it should match if it has
// been provided.
assert(axis == null || axis == Axis.vertical);
// `target` is only fully revealed when in the selected/center position. Therefore, // `target` is only fully revealed when in the selected/center position. Therefore,
// this method always returns the offset that shows `target` in the center position, // this method always returns the offset that shows `target` in the center position,
// which is the same offset for all `alignment` values. // which is the same offset for all `alignment` values.
......
...@@ -111,9 +111,14 @@ abstract interface class RenderAbstractViewport extends RenderObject { ...@@ -111,9 +111,14 @@ abstract interface class RenderAbstractViewport extends RenderObject {
/// The optional [Axis] is used by /// The optional [Axis] is used by
/// [RenderTwoDimensionalViewport.getOffsetToReveal] to /// [RenderTwoDimensionalViewport.getOffsetToReveal] to
/// determine which of the two axes to compute an offset for. One dimensional /// determine which of the two axes to compute an offset for. One dimensional
/// subclasses like [RenderViewportBase] and [RenderListWheelViewport] will /// subclasses like [RenderViewportBase] and [RenderListWheelViewport]
/// assert in debug builds if the `axis` value is provided and does not match /// will ignore the `axis` value if provided, since there is only one [Axis].
/// the single [Axis] that viewport is configured for. ///
/// If the `axis` is omitted when called on [RenderTwoDimensionalViewport],
/// the [RenderTwoDimensionalViewport.mainAxis] is used. To reveal an object
/// properly in both axes, this method should be called for each [Axis] as the
/// returned [RevealedOffset.offset] only represents the offset of one of the
/// the two [ScrollPosition]s.
/// ///
/// See also: /// See also:
/// ///
...@@ -821,10 +826,9 @@ abstract class RenderViewportBase<ParentDataClass extends ContainerParentDataMix ...@@ -821,10 +826,9 @@ abstract class RenderViewportBase<ParentDataClass extends ContainerParentDataMix
Rect? rect, Rect? rect,
Axis? axis, Axis? axis,
}) { }) {
// One dimensional viewport has only one axis, it should match if it has // One dimensional viewport has only one axis, override if it was
// been provided. // provided/may be mismatched.
axis ??= this.axis; axis = this.axis;
assert(axis == this.axis);
// Steps to convert `rect` (from a RenderBox coordinate system) to its // Steps to convert `rect` (from a RenderBox coordinate system) to its
// scroll offset within this viewport (not in the exact order): // scroll offset within this viewport (not in the exact order):
......
...@@ -598,10 +598,9 @@ class _RenderSingleChildViewport extends RenderBox with RenderObjectWithChildMix ...@@ -598,10 +598,9 @@ class _RenderSingleChildViewport extends RenderBox with RenderObjectWithChildMix
Rect? rect, Rect? rect,
Axis? axis, Axis? axis,
}) { }) {
// One dimensional viewport has only one axis, it should match if it has // One dimensional viewport has only one axis, override if it was
// been provided. // provided/may be mismatched.
axis ??= this.axis; axis = this.axis;
assert(axis == this.axis);
rect ??= target.paintBounds; rect ??= target.paintBounds;
if (target is! RenderBox) { if (target is! RenderBox) {
......
...@@ -924,11 +924,10 @@ abstract class RenderTwoDimensionalViewport extends RenderBox implements RenderA ...@@ -924,11 +924,10 @@ abstract class RenderTwoDimensionalViewport extends RenderBox implements RenderA
Rect? rect, Rect? rect,
Axis? axis, Axis? axis,
}) { }) {
// We must know which axis we are revealing for, since RevealedOffset // If an axis has not been specified, use the mainAxis.
// refers to only one of two scroll positions. axis ??= mainAxis;
assert(axis != null);
final (double offset, AxisDirection axisDirection) = switch (axis!) { final (double offset, AxisDirection axisDirection) = switch (axis) {
Axis.vertical => (verticalOffset.pixels, verticalAxisDirection), Axis.vertical => (verticalOffset.pixels, verticalAxisDirection),
Axis.horizontal => (horizontalOffset.pixels, horizontalAxisDirection), Axis.horizontal => (horizontalOffset.pixels, horizontalAxisDirection),
}; };
......
...@@ -1586,6 +1586,14 @@ void main() { ...@@ -1586,6 +1586,14 @@ void main() {
final double revealOffset = viewport.getOffsetToReveal(target, 0.0).offset; final double revealOffset = viewport.getOffsetToReveal(target, 0.0).offset;
expect(revealOffset, -(300.0 + padding.horizontal) * 5 + 34.0 * 2); expect(revealOffset, -(300.0 + padding.horizontal) * 5 + 34.0 * 2);
}); });
testWidgets('will not assert on mismatched axis', (WidgetTester tester) async {
await tester.pumpWidget(buildList(axis: Axis.vertical, reverse: true, reverseGrowth: true));
final RenderAbstractViewport viewport = tester.allRenderObjects.whereType<RenderAbstractViewport>().first;
final RenderObject target = tester.renderObject(find.text('Tile 0', skipOffstage: false));
viewport.getOffsetToReveal(target, 0.0, axis: Axis.horizontal);
});
}); });
testWidgets('RenderViewportBase.showOnScreen reports the correct targetRect', (WidgetTester tester) async { testWidgets('RenderViewportBase.showOnScreen reports the correct targetRect', (WidgetTester tester) async {
......
...@@ -1598,6 +1598,37 @@ void main() { ...@@ -1598,6 +1598,37 @@ void main() {
expect(revealed.rect, const Rect.fromLTWH(165.0, 265.0, 10.0, 10.0)); expect(revealed.rect, const Rect.fromLTWH(165.0, 265.0, 10.0, 10.0));
}); });
testWidgets('will not assert on getOffsetToReveal Axis', (WidgetTester tester) async {
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: Center(
child: SizedBox(
height: 500.0,
width: 300.0,
child: ListWheelScrollView(
controller: ScrollController(initialScrollOffset: 300.0),
itemExtent: 100.0,
children: List<Widget>.generate(10, (int i) {
return Center(
child: SizedBox(
height: 50.0,
width: 50.0,
child: Text('Item $i'),
),
);
}),
),
),
),
),
);
final RenderListWheelViewport viewport = tester.allRenderObjects.whereType<RenderListWheelViewport>().first;
final RenderObject target = tester.renderObject(find.text('Item 5'));
viewport.getOffsetToReveal(target, 0.0, axis: Axis.horizontal);
});
testWidgets('ListWheelScrollView showOnScreen', (WidgetTester tester) async { testWidgets('ListWheelScrollView showOnScreen', (WidgetTester tester) async {
List<Widget> outerChildren; List<Widget> outerChildren;
final List<Widget> innerChildren = List<Widget>.generate(10, (int index) => Container()); final List<Widget> innerChildren = List<Widget>.generate(10, (int index) => Container());
......
...@@ -486,6 +486,40 @@ void main() { ...@@ -486,6 +486,40 @@ void main() {
expect(semanticsClip.size.width, length); expect(semanticsClip.size.width, length);
}); });
testWidgetsWithLeakTracking('SingleChildScrollView getOffsetToReveal - will not assert on axis mismatch', (WidgetTester tester) async {
final ScrollController controller = ScrollController(initialScrollOffset: 300.0);
addTearDown(controller.dispose);
List<Widget> children;
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: Center(
child: SizedBox(
height: 200.0,
width: 300.0,
child: SingleChildScrollView(
controller: controller,
child: Column(
children: children = List<Widget>.generate(20, (int i) {
return SizedBox(
height: 100.0,
width: 300.0,
child: Text('Tile $i'),
);
}),
),
),
),
),
),
);
final RenderAbstractViewport viewport = tester.allRenderObjects.whereType<RenderAbstractViewport>().first;
final RenderObject target = tester.renderObject(find.byWidget(children[5]));
viewport.getOffsetToReveal(target, 0.0, axis: Axis.horizontal);
});
testWidgetsWithLeakTracking('SingleChildScrollView getOffsetToReveal - down', (WidgetTester tester) async { testWidgetsWithLeakTracking('SingleChildScrollView getOffsetToReveal - down', (WidgetTester tester) async {
final ScrollController controller = ScrollController(initialScrollOffset: 300.0); final ScrollController controller = ScrollController(initialScrollOffset: 300.0);
addTearDown(controller.dispose); addTearDown(controller.dispose);
......
...@@ -2366,11 +2366,53 @@ void main() { ...@@ -2366,11 +2366,53 @@ void main() {
); );
}, variant: TargetPlatformVariant.all()); }, variant: TargetPlatformVariant.all());
group('TwoDimensionalViewport showOnScreen & showInViewport', () { group('showOnScreen & showInViewport', () {
Finder findKey(ChildVicinity vicinity) { Finder findKey(ChildVicinity vicinity) {
return find.byKey(ValueKey<ChildVicinity>(vicinity)); return find.byKey(ValueKey<ChildVicinity>(vicinity));
} }
testWidgets('getOffsetToReveal', (WidgetTester tester) async {
await tester.pumpWidget(simpleBuilderTest(useCacheExtent: true));
RenderAbstractViewport viewport = tester.allRenderObjects.whereType<RenderAbstractViewport>().first;
final RevealedOffset verticalOffset = viewport.getOffsetToReveal(
tester.renderObject(findKey(const ChildVicinity(xIndex: 5, yIndex: 5))),
1.0,
axis: Axis.vertical,
);
final RevealedOffset horizontalOffset = viewport.getOffsetToReveal(
tester.renderObject(findKey(const ChildVicinity(xIndex: 5, yIndex: 5))),
1.0,
axis: Axis.horizontal,
);
expect(verticalOffset.offset, 600.0);
expect(verticalOffset.rect, const Rect.fromLTRB(1000.0, 400.0, 1200.0, 600.0));
expect(horizontalOffset.offset, 400.0);
expect(horizontalOffset.rect, const Rect.fromLTRB(600.0, 1000.0, 800.0, 1200.0));
// default is to use mainAxis when axis is not provided, mainAxis
// defaults to Axis.vertical.
RevealedOffset defaultOffset = viewport.getOffsetToReveal(
tester.renderObject(findKey(const ChildVicinity(xIndex: 5, yIndex: 5))),
1.0,
);
expect(defaultOffset.offset, verticalOffset.offset);
expect(defaultOffset.rect, verticalOffset.rect);
// mainAxis as Axis.horizontal
await tester.pumpWidget(simpleBuilderTest(
useCacheExtent: true,
mainAxis: Axis.horizontal,
));
viewport = tester.allRenderObjects.whereType<RenderAbstractViewport>().first;
defaultOffset = viewport.getOffsetToReveal(
tester.renderObject(findKey(const ChildVicinity(xIndex: 5, yIndex: 5))),
1.0,
);
expect(defaultOffset.offset, horizontalOffset.offset);
expect(defaultOffset.rect, horizontalOffset.rect);
});
testWidgets('Axis.vertical', (WidgetTester tester) async { testWidgets('Axis.vertical', (WidgetTester tester) async {
await tester.pumpWidget(simpleBuilderTest(useCacheExtent: true)); await tester.pumpWidget(simpleBuilderTest(useCacheExtent: true));
// Child visible at origin // Child visible at origin
......
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