Commit c9b08aa6 authored by Matt Perry's avatar Matt Perry Committed by GitHub

Fix a bug with how ScrollableGrid deals with padding. (#5963)

It wasn't taking padding into account when deciding which children were
visible. I modeled the solution off of the way ScrollableList handles
this.

Fixes https://github.com/flutter/flutter/issues/5522
parent 3f025e0d
......@@ -205,6 +205,9 @@ abstract class GridDelegate {
return getGridSpecification(constraints, childCount).gridSize;
}
/// Insets for the entire grid.
EdgeInsets get padding => EdgeInsets.zero;
// TODO(ianh): It's a bit dubious to be using the getSize function from the delegate to
// figure out the intrinsic dimensions. We really should either not support intrinsics,
// or we should expose intrinsic delegate callbacks and throw if they're not implemented.
......@@ -301,6 +304,7 @@ abstract class GridDelegateWithInOrderChildPlacement extends GridDelegate {
final double rowSpacing;
/// Insets for the entire grid.
@override
final EdgeInsets padding;
@override
......@@ -637,8 +641,18 @@ class RenderGrid extends RenderVirtualViewport<GridParentData> {
if (callback != null)
invokeLayoutCallback(callback);
final double gridTopPadding = _specification.padding.top;
final double gridLeftPadding = _specification.padding.left;
double gridTopPadding = 0.0;
double gridLeftPadding = 0.0;
switch (mainAxis) {
case Axis.vertical:
gridLeftPadding = _specification.padding.left;
break;
case Axis.horizontal:
gridTopPadding = _specification.padding.top;
break;
}
int childIndex = virtualChildBase;
RenderBox child = firstChild;
while (child != null) {
......
......@@ -83,7 +83,7 @@ class ScrollableGrid extends StatelessWidget {
Widget _buildViewport(BuildContext context, ScrollableState state, double scrollOffset) {
return new GridViewport(
startOffset: scrollOffset,
scrollOffset: scrollOffset,
delegate: delegate,
onExtentsChanged: state.handleExtentsChanged,
children: children
......@@ -126,7 +126,7 @@ class GridViewport extends VirtualViewportFromIterable {
///
/// The [delegate] argument must not be null.
GridViewport({
this.startOffset,
this.scrollOffset,
this.delegate,
this.onExtentsChanged,
this.children
......@@ -134,8 +134,15 @@ class GridViewport extends VirtualViewportFromIterable {
assert(delegate != null);
}
/// The [startOffset] without taking the [delegate]'s padding into account.
final double scrollOffset;
@override
final double startOffset;
double get startOffset {
if (delegate == null)
return scrollOffset;
return scrollOffset - delegate.padding.top;
}
/// The delegate that controls the layout of the children.
final GridDelegate delegate;
......
// Copyright 2016 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
import 'package:flutter_test/flutter_test.dart';
import 'package:flutter/widgets.dart';
import 'package:flutter/rendering.dart';
void main() {
// Tests https://github.com/flutter/flutter/issues/5522
testWidgets('ScrollableGrid displays correct children with nonzero padding', (WidgetTester tester) async {
GlobalKey<ScrollableState> scrollableKey = new GlobalKey<ScrollableState>();
final EdgeInsets padding = new EdgeInsets.fromLTRB(0.0, 100.0, 0.0, 0.0);
Widget testWidget = new Align(
child: new SizedBox(
height: 800.0,
width: 300.0, // forces the grid children to be 300..300
child: new ScrollableGrid(
scrollableKey: scrollableKey,
delegate: new FixedColumnCountGridDelegate(
columnCount: 1,
padding: padding
),
children: new List<Widget>.generate(10, (int index) {
return new Text('$index', key: new ValueKey<int>(index));
})
)
)
);
await tester.pumpWidget(testWidget);
// screen is 600px high, and has the following items:
// 100..400 = 0
// 400..700 = 1
await tester.pump();
expect(find.text('0'), findsOneWidget);
expect(find.text('1'), findsOneWidget);
expect(find.text('2'), findsNothing);
expect(find.text('3'), findsNothing);
await tester.scroll(find.text('1'), const Offset(0.0, -500.0));
await tester.pump();
// -100..300 = 1
// 300..600 = 2
// 600..600 = 3
expect(find.text('0'), findsNothing);
expect(find.text('1'), findsOneWidget);
expect(find.text('2'), findsOneWidget);
expect(find.text('3'), findsOneWidget);
expect(find.text('4'), findsNothing);
expect(find.text('5'), findsNothing);
await tester.scroll(find.text('1'), const Offset(0.0, 150.0));
await tester.pump();
// Child '0' is now back onscreen, but by less than `padding.top`.
// -250..050 = 0
// 050..450 = 1
// 450..750 = 2
expect(find.text('0'), findsOneWidget);
expect(find.text('1'), findsOneWidget);
expect(find.text('2'), findsOneWidget);
expect(find.text('3'), findsNothing);
expect(find.text('4'), findsNothing);
});
}
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