Reland fixes sliver list child layout offset calculation (#53187)
This commit is contained in:
parent
b684041bb8
commit
13fa5734f8
@ -928,13 +928,16 @@ class SliverLogicalParentData extends ParentData {
|
||||
///
|
||||
/// The number of pixels from from the zero scroll offset of the parent sliver
|
||||
/// (the line at which its [SliverConstraints.scrollOffset] is zero) to the
|
||||
/// side of the child closest to that offset.
|
||||
/// side of the child closest to that offset. A [layoutOffset] can be null
|
||||
/// when it cannot be determined. The value will be set after layout.
|
||||
///
|
||||
/// In a typical list, this does not change as the parent is scrolled.
|
||||
double layoutOffset = 0.0;
|
||||
///
|
||||
/// Defaults to null.
|
||||
double layoutOffset;
|
||||
|
||||
@override
|
||||
String toString() => 'layoutOffset=${layoutOffset.toStringAsFixed(1)}';
|
||||
String toString() => 'layoutOffset=${layoutOffset == null ? 'None': layoutOffset.toStringAsFixed(1)}';
|
||||
}
|
||||
|
||||
/// Parent data for slivers that have multiple children and that position their
|
||||
|
@ -91,8 +91,28 @@ class RenderSliverList extends RenderSliverMultiBoxAdaptor {
|
||||
// it's possible for a child to get removed without notice.
|
||||
RenderBox leadingChildWithLayout, trailingChildWithLayout;
|
||||
|
||||
// Find the last child that is at or before the scrollOffset.
|
||||
RenderBox earliestUsefulChild = firstChild;
|
||||
|
||||
// A firstChild with null layout offset is likely a result of children
|
||||
// reordering.
|
||||
//
|
||||
// We rely on firstChild to have accurate layout offset. In the case of null
|
||||
// layout offset, we have to find the first child that has valid layout
|
||||
// offset.
|
||||
if (childScrollOffset(firstChild) == null) {
|
||||
int leadingChildrenWithoutLayoutOffset = 0;
|
||||
while (childScrollOffset(earliestUsefulChild) == null) {
|
||||
earliestUsefulChild = childAfter(firstChild);
|
||||
leadingChildrenWithoutLayoutOffset += 1;
|
||||
}
|
||||
// We should be able to destroy children with null layout offset safely,
|
||||
// because they are likely outside of viewport
|
||||
collectGarbage(leadingChildrenWithoutLayoutOffset, 0);
|
||||
assert(firstChild != null);
|
||||
}
|
||||
|
||||
// Find the last child that is at or before the scrollOffset.
|
||||
earliestUsefulChild = firstChild;
|
||||
for (double earliestScrollOffset = childScrollOffset(earliestUsefulChild);
|
||||
earliestScrollOffset > scrollOffset;
|
||||
earliestScrollOffset = childScrollOffset(earliestUsefulChild)) {
|
||||
@ -140,12 +160,15 @@ class RenderSliverList extends RenderSliverMultiBoxAdaptor {
|
||||
correction += paintExtentOf(firstChild);
|
||||
earliestUsefulChild = insertAndLayoutLeadingChild(childConstraints, parentUsesSize: true);
|
||||
}
|
||||
geometry = SliverGeometry(
|
||||
scrollOffsetCorrection: correction - earliestScrollOffset,
|
||||
);
|
||||
final SliverMultiBoxAdaptorParentData childParentData = firstChild.parentData as SliverMultiBoxAdaptorParentData;
|
||||
childParentData.layoutOffset = 0.0;
|
||||
return;
|
||||
earliestUsefulChild = firstChild;
|
||||
if ((correction - earliestScrollOffset).abs() > precisionErrorTolerance) {
|
||||
geometry = SliverGeometry(
|
||||
scrollOffsetCorrection: correction - earliestScrollOffset,
|
||||
);
|
||||
final SliverMultiBoxAdaptorParentData childParentData = firstChild.parentData as SliverMultiBoxAdaptorParentData;
|
||||
childParentData.layoutOffset = 0.0;
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
final SliverMultiBoxAdaptorParentData childParentData = earliestUsefulChild.parentData as SliverMultiBoxAdaptorParentData;
|
||||
|
@ -578,7 +578,6 @@ abstract class RenderSliverMultiBoxAdaptor extends RenderSliver
|
||||
assert(child != null);
|
||||
assert(child.parent == this);
|
||||
final SliverMultiBoxAdaptorParentData childParentData = child.parentData as SliverMultiBoxAdaptorParentData;
|
||||
assert(childParentData.layoutOffset != null);
|
||||
return childParentData.layoutOffset;
|
||||
}
|
||||
|
||||
|
@ -1069,6 +1069,7 @@ class SliverMultiBoxAdaptorElement extends RenderObjectElement implements Render
|
||||
assert(_currentlyUpdatingChildIndex == null);
|
||||
try {
|
||||
final SplayTreeMap<int, Element> newChildren = SplayTreeMap<int, Element>();
|
||||
final Map<int, double> indexToLayoutOffset = HashMap<int, double>();
|
||||
|
||||
void processElement(int index) {
|
||||
_currentlyUpdatingChildIndex = index;
|
||||
@ -1080,17 +1081,31 @@ class SliverMultiBoxAdaptorElement extends RenderObjectElement implements Render
|
||||
if (newChild != null) {
|
||||
_childElements[index] = newChild;
|
||||
final SliverMultiBoxAdaptorParentData parentData = newChild.renderObject.parentData as SliverMultiBoxAdaptorParentData;
|
||||
if (index == 0) {
|
||||
parentData.layoutOffset = 0.0;
|
||||
} else if (indexToLayoutOffset.containsKey(index)) {
|
||||
parentData.layoutOffset = indexToLayoutOffset[index];
|
||||
}
|
||||
if (!parentData.keptAlive)
|
||||
_currentBeforeChild = newChild.renderObject as RenderBox;
|
||||
} else {
|
||||
_childElements.remove(index);
|
||||
}
|
||||
}
|
||||
|
||||
for (final int index in _childElements.keys.toList()) {
|
||||
final Key key = _childElements[index].widget.key;
|
||||
final int newIndex = key == null ? null : widget.delegate.findIndexByKey(key);
|
||||
final SliverMultiBoxAdaptorParentData childParentData =
|
||||
_childElements[index].renderObject?.parentData as SliverMultiBoxAdaptorParentData;
|
||||
|
||||
if (childParentData != null && childParentData.layoutOffset != null)
|
||||
indexToLayoutOffset[index] = childParentData.layoutOffset;
|
||||
|
||||
if (newIndex != null && newIndex != index) {
|
||||
// The layout offset of the child being moved is no longer accurate.
|
||||
if (childParentData != null)
|
||||
childParentData.layoutOffset = null;
|
||||
|
||||
newChildren[newIndex] = _childElements[index];
|
||||
// We need to make sure the original index gets processed.
|
||||
newChildren.putIfAbsent(index, () => null);
|
||||
@ -1309,7 +1324,8 @@ class SliverMultiBoxAdaptorElement extends RenderObjectElement implements Render
|
||||
break;
|
||||
}
|
||||
|
||||
return parentData.layoutOffset < renderObject.constraints.scrollOffset + renderObject.constraints.remainingPaintExtent &&
|
||||
return parentData.layoutOffset != null &&
|
||||
parentData.layoutOffset < renderObject.constraints.scrollOffset + renderObject.constraints.remainingPaintExtent &&
|
||||
parentData.layoutOffset + itemExtent > renderObject.constraints.scrollOffset;
|
||||
}).forEach(visitor);
|
||||
}
|
||||
|
@ -340,18 +340,20 @@ void main() {
|
||||
final SliverMultiBoxAdaptorParentData candidate = SliverMultiBoxAdaptorParentData();
|
||||
expect(candidate.keepAlive, isFalse);
|
||||
expect(candidate.index, isNull);
|
||||
expect(candidate.toString(), 'index=null; layoutOffset=0.0');
|
||||
expect(candidate.toString(), 'index=null; layoutOffset=None');
|
||||
candidate.keepAlive = null;
|
||||
expect(candidate.toString(), 'index=null; layoutOffset=0.0');
|
||||
expect(candidate.toString(), 'index=null; layoutOffset=None');
|
||||
candidate.keepAlive = true;
|
||||
expect(candidate.toString(), 'index=null; keepAlive; layoutOffset=0.0');
|
||||
expect(candidate.toString(), 'index=null; keepAlive; layoutOffset=None');
|
||||
candidate.keepAlive = false;
|
||||
expect(candidate.toString(), 'index=null; layoutOffset=0.0');
|
||||
expect(candidate.toString(), 'index=null; layoutOffset=None');
|
||||
candidate.index = 0;
|
||||
expect(candidate.toString(), 'index=0; layoutOffset=0.0');
|
||||
expect(candidate.toString(), 'index=0; layoutOffset=None');
|
||||
candidate.index = 1;
|
||||
expect(candidate.toString(), 'index=1; layoutOffset=0.0');
|
||||
expect(candidate.toString(), 'index=1; layoutOffset=None');
|
||||
candidate.index = -1;
|
||||
expect(candidate.toString(), 'index=-1; layoutOffset=0.0');
|
||||
expect(candidate.toString(), 'index=-1; layoutOffset=None');
|
||||
candidate.layoutOffset = 100.0;
|
||||
expect(candidate.toString(), 'index=-1; layoutOffset=100.0');
|
||||
});
|
||||
}
|
||||
|
@ -163,6 +163,118 @@ void main() {
|
||||
expect(find.text('Tile 1'), findsOneWidget);
|
||||
expect(find.text('Tile 2'), findsOneWidget);
|
||||
});
|
||||
|
||||
testWidgets('SliverList should recalculate inaccurate layout offset case 1', (WidgetTester tester) async {
|
||||
// Regression test for https://github.com/flutter/flutter/issues/42142.
|
||||
final List<int> items = List<int>.generate(20, (int i) => i);
|
||||
final ScrollController controller = ScrollController();
|
||||
await tester.pumpWidget(
|
||||
_buildSliverList(
|
||||
items: List<int>.from(items),
|
||||
controller: controller,
|
||||
itemHeight: 50,
|
||||
viewportHeight: 200,
|
||||
)
|
||||
);
|
||||
await tester.pumpAndSettle();
|
||||
|
||||
await tester.drag(find.text('Tile 2'), const Offset(0.0, -1000.0));
|
||||
await tester.pumpAndSettle();
|
||||
|
||||
// Viewport should be scrolled to the end of list.
|
||||
expect(controller.offset, 800.0);
|
||||
expect(find.text('Tile 15'), findsNothing);
|
||||
expect(find.text('Tile 16'), findsOneWidget);
|
||||
expect(find.text('Tile 17'), findsOneWidget);
|
||||
expect(find.text('Tile 18'), findsOneWidget);
|
||||
expect(find.text('Tile 19'), findsOneWidget);
|
||||
|
||||
// Prepends item to the list.
|
||||
items.insert(0, -1);
|
||||
await tester.pumpWidget(
|
||||
_buildSliverList(
|
||||
items: List<int>.from(items),
|
||||
controller: controller,
|
||||
itemHeight: 50,
|
||||
viewportHeight: 200,
|
||||
)
|
||||
);
|
||||
await tester.pump();
|
||||
// We need second pump to ensure the scheduled animation gets run.
|
||||
await tester.pumpAndSettle();
|
||||
// Scroll offset should stay the same, and the items in viewport should be
|
||||
// shifted by one.
|
||||
expect(controller.offset, 800.0);
|
||||
expect(find.text('Tile 14'), findsNothing);
|
||||
expect(find.text('Tile 15'), findsOneWidget);
|
||||
expect(find.text('Tile 16'), findsOneWidget);
|
||||
expect(find.text('Tile 17'), findsOneWidget);
|
||||
expect(find.text('Tile 18'), findsOneWidget);
|
||||
expect(find.text('Tile 19'), findsNothing);
|
||||
|
||||
// Drags back to beginning and newly added item is visible.
|
||||
await tester.drag(find.text('Tile 16'), const Offset(0.0, 1000.0));
|
||||
await tester.pumpAndSettle();
|
||||
expect(controller.offset, 0.0);
|
||||
expect(find.text('Tile -1'), findsOneWidget);
|
||||
expect(find.text('Tile 0'), findsOneWidget);
|
||||
expect(find.text('Tile 1'), findsOneWidget);
|
||||
expect(find.text('Tile 2'), findsOneWidget);
|
||||
expect(find.text('Tile 3'), findsNothing);
|
||||
|
||||
});
|
||||
|
||||
testWidgets('SliverList should recalculate inaccurate layout offset case 2', (WidgetTester tester) async {
|
||||
// Regression test for https://github.com/flutter/flutter/issues/42142.
|
||||
final List<int> items = List<int>.generate(20, (int i) => i);
|
||||
final ScrollController controller = ScrollController();
|
||||
await tester.pumpWidget(
|
||||
_buildSliverList(
|
||||
items: List<int>.from(items),
|
||||
controller: controller,
|
||||
itemHeight: 50,
|
||||
viewportHeight: 200,
|
||||
)
|
||||
);
|
||||
await tester.pumpAndSettle();
|
||||
|
||||
await tester.drag(find.text('Tile 2'), const Offset(0.0, -1000.0));
|
||||
await tester.pumpAndSettle();
|
||||
|
||||
// Viewport should be scrolled to the end of list.
|
||||
expect(controller.offset, 800.0);
|
||||
expect(find.text('Tile 15'), findsNothing);
|
||||
expect(find.text('Tile 16'), findsOneWidget);
|
||||
expect(find.text('Tile 17'), findsOneWidget);
|
||||
expect(find.text('Tile 18'), findsOneWidget);
|
||||
expect(find.text('Tile 19'), findsOneWidget);
|
||||
|
||||
// Reorders item to the front. This should make item 19 to be first child
|
||||
// with layout offset = null.
|
||||
final int swap = items[19];
|
||||
items[19] = items[3];
|
||||
items[3] = swap;
|
||||
|
||||
await tester.pumpWidget(
|
||||
_buildSliverList(
|
||||
items: List<int>.from(items),
|
||||
controller: controller,
|
||||
itemHeight: 50,
|
||||
viewportHeight: 200,
|
||||
)
|
||||
);
|
||||
await tester.pump();
|
||||
// We need second pump to ensure the scheduled animation gets run.
|
||||
await tester.pumpAndSettle();
|
||||
// Scroll offset should stay the same
|
||||
expect(controller.offset, 800.0);
|
||||
expect(find.text('Tile 14'), findsNothing);
|
||||
expect(find.text('Tile 15'), findsNothing);
|
||||
expect(find.text('Tile 16'), findsOneWidget);
|
||||
expect(find.text('Tile 17'), findsOneWidget);
|
||||
expect(find.text('Tile 18'), findsOneWidget);
|
||||
expect(find.text('Tile 3'), findsOneWidget);
|
||||
});
|
||||
}
|
||||
|
||||
Widget _buildSliverListRenderWidgetChild(List<String> items) {
|
||||
@ -216,6 +328,11 @@ Widget _buildSliverList({
|
||||
child: Text('Tile ${items[i]}'),
|
||||
);
|
||||
},
|
||||
findChildIndexCallback: (Key key) {
|
||||
final ValueKey<int> valueKey = key as ValueKey<int>;
|
||||
final int index = items.indexOf(valueKey.value);
|
||||
return index == -1 ? null : index;
|
||||
},
|
||||
childCount: items.length,
|
||||
),
|
||||
),
|
||||
|
@ -50,7 +50,8 @@ Future<void> testSliverFixedExtentList(WidgetTester tester, List<String> items)
|
||||
findChildIndexCallback: (Key key) {
|
||||
final ValueKey<String> valueKey = key as ValueKey<String>;
|
||||
final String data = valueKey.value;
|
||||
return items.indexOf(data);
|
||||
final int index = items.indexOf(data);
|
||||
return index == -1 ? null : index;
|
||||
},
|
||||
),
|
||||
),
|
||||
|
Loading…
x
Reference in New Issue
Block a user