Unverified Commit c169136e authored by Hans Muller's avatar Hans Muller Committed by GitHub

Revert "Fixing list tile size in the presence of a large subtitle" (#17611)

parent 1095eafe
...@@ -443,11 +443,11 @@ class ListTile extends StatelessWidget { ...@@ -443,11 +443,11 @@ class ListTile extends StatelessWidget {
); );
} }
const EdgeInsets _defaultContentPadding = const EdgeInsets.symmetric(horizontal: 16.0); const EdgeInsets _kDefaultContentPadding = const EdgeInsets.symmetric(horizontal: 16.0);
final TextDirection textDirection = Directionality.of(context); final TextDirection textDirection = Directionality.of(context);
final EdgeInsets resolvedContentPadding = contentPadding?.resolve(textDirection) final EdgeInsets resolvedContentPadding = contentPadding?.resolve(textDirection)
?? tileTheme?.contentPadding?.resolve(textDirection) ?? tileTheme?.contentPadding?.resolve(textDirection)
?? _defaultContentPadding; ?? _kDefaultContentPadding;
return new InkWell( return new InkWell(
onTap: enabled ? onTap : null, onTap: enabled ? onTap : null,
...@@ -529,11 +529,8 @@ class _RenderListTile extends RenderBox { ...@@ -529,11 +529,8 @@ class _RenderListTile extends RenderBox {
_isThreeLine = isThreeLine, _isThreeLine = isThreeLine,
_textDirection = textDirection; _textDirection = textDirection;
static const double _minLeadingWidth = 40.0; static const double _kMinLeadingWidth = 40.0;
// The horizontal gap between the titles and the leading/trailing widgets static const double _kTitleGap = 16.0; // between the titles and the leading/trailing widgets
static const double _horizontalTitleGap = 16.0;
// The minimum padding on the top and bottom of the title and subtitle widgets.
static const double _minVerticalPadding = 4.0;
final Map<_ListTileSlot, RenderBox> slotToChild = <_ListTileSlot, RenderBox>{}; final Map<_ListTileSlot, RenderBox> slotToChild = <_ListTileSlot, RenderBox>{};
final Map<RenderBox, _ListTileSlot> childToSlot = <RenderBox, _ListTileSlot>{}; final Map<RenderBox, _ListTileSlot> childToSlot = <RenderBox, _ListTileSlot>{};
...@@ -667,7 +664,7 @@ class _RenderListTile extends RenderBox { ...@@ -667,7 +664,7 @@ class _RenderListTile extends RenderBox {
@override @override
double computeMinIntrinsicWidth(double height) { double computeMinIntrinsicWidth(double height) {
final double leadingWidth = leading != null final double leadingWidth = leading != null
? math.max(leading.getMinIntrinsicWidth(height), _minLeadingWidth) + _horizontalTitleGap ? math.max(leading.getMinIntrinsicWidth(height), _kMinLeadingWidth) + _kTitleGap
: 0.0; : 0.0;
return leadingWidth return leadingWidth
+ math.max(_minWidth(title, height), _minWidth(subtitle, height)) + math.max(_minWidth(title, height), _minWidth(subtitle, height))
...@@ -677,7 +674,7 @@ class _RenderListTile extends RenderBox { ...@@ -677,7 +674,7 @@ class _RenderListTile extends RenderBox {
@override @override
double computeMaxIntrinsicWidth(double height) { double computeMaxIntrinsicWidth(double height) {
final double leadingWidth = leading != null final double leadingWidth = leading != null
? math.max(leading.getMaxIntrinsicWidth(height), _minLeadingWidth) + _horizontalTitleGap ? math.max(leading.getMaxIntrinsicWidth(height), _kMinLeadingWidth) + _kTitleGap
: 0.0; : 0.0;
return leadingWidth return leadingWidth
+ math.max(_maxWidth(title, height), _maxWidth(subtitle, height)) + math.max(_maxWidth(title, height), _maxWidth(subtitle, height))
...@@ -749,10 +746,10 @@ class _RenderListTile extends RenderBox { ...@@ -749,10 +746,10 @@ class _RenderListTile extends RenderBox {
final Size trailingSize = _layoutBox(trailing, looseConstraints); final Size trailingSize = _layoutBox(trailing, looseConstraints);
final double titleStart = hasLeading final double titleStart = hasLeading
? math.max(_minLeadingWidth, leadingSize.width) + _horizontalTitleGap ? math.max(_kMinLeadingWidth, leadingSize.width) + _kTitleGap
: 0.0; : 0.0;
final BoxConstraints textConstraints = looseConstraints.tighten( final BoxConstraints textConstraints = looseConstraints.tighten(
width: tileWidth - titleStart - (hasTrailing ? trailingSize.width + _horizontalTitleGap : 0.0), width: tileWidth - titleStart - (hasTrailing ? trailingSize.width + _kTitleGap : 0.0),
); );
final Size titleSize = _layoutBox(title, textConstraints); final Size titleSize = _layoutBox(title, textConstraints);
final Size subtitleSize = _layoutBox(subtitle, textConstraints); final Size subtitleSize = _layoutBox(subtitle, textConstraints);
...@@ -773,7 +770,7 @@ class _RenderListTile extends RenderBox { ...@@ -773,7 +770,7 @@ class _RenderListTile extends RenderBox {
double titleY; double titleY;
double subtitleY; double subtitleY;
if (!hasSubtitle) { if (!hasSubtitle) {
tileHeight = math.max(_defaultTileHeight, titleSize.height + 2.0 * _minVerticalPadding); tileHeight = math.max(_defaultTileHeight, titleSize.height);
titleY = (tileHeight - titleSize.height) / 2.0; titleY = (tileHeight - titleSize.height) / 2.0;
} else { } else {
titleY = titleBaseline - _boxBaseline(title); titleY = titleBaseline - _boxBaseline(title);
...@@ -790,13 +787,11 @@ class _RenderListTile extends RenderBox { ...@@ -790,13 +787,11 @@ class _RenderListTile extends RenderBox {
} }
// If the title or subtitle overflow tileHeight then punt: title // If the title or subtitle overflow tileHeight then punt: title
// and subtitle are arranged in a column, tileHeight = column height plus // and subtitle are arranged in a column, tileHeight = column height.
// _minVerticalPadding on top and bottom. if (titleY < 0.0 || subtitleY > tileHeight) {
if (titleY < _minVerticalPadding || tileHeight = titleSize.height + subtitleSize.height;
(subtitleY + subtitleSize.height + _minVerticalPadding) > tileHeight) { titleY = 0.0;
tileHeight = titleSize.height + subtitleSize.height + 2.0 * _minVerticalPadding; subtitleY = titleSize.height;
titleY = _minVerticalPadding;
subtitleY = titleSize.height + _minVerticalPadding;
} }
} }
...@@ -807,7 +802,7 @@ class _RenderListTile extends RenderBox { ...@@ -807,7 +802,7 @@ class _RenderListTile extends RenderBox {
case TextDirection.rtl: { case TextDirection.rtl: {
if (hasLeading) if (hasLeading)
_positionBox(leading, new Offset(tileWidth - leadingSize.width, leadingY)); _positionBox(leading, new Offset(tileWidth - leadingSize.width, leadingY));
final double titleX = hasTrailing ? trailingSize.width + _horizontalTitleGap : 0.0; final double titleX = hasTrailing ? trailingSize.width + _kTitleGap : 0.0;
_positionBox(title, new Offset(titleX, titleY)); _positionBox(title, new Offset(titleX, titleY));
if (hasSubtitle) if (hasSubtitle)
_positionBox(subtitle, new Offset(titleX, subtitleY)); _positionBox(subtitle, new Offset(titleX, subtitleY));
......
...@@ -56,9 +56,8 @@ void main() { ...@@ -56,9 +56,8 @@ void main() {
const double leftPadding = 10.0; const double leftPadding = 10.0;
const double rightPadding = 20.0; const double rightPadding = 20.0;
Widget buildFrame({ bool dense: false, bool isTwoLine: false, bool isThreeLine: false, double textScaleFactor: 1.0, double subtitleScaleFactor }) { Widget buildFrame({ bool dense: false, bool isTwoLine: false, bool isThreeLine: false, double textScaleFactor: 1.0 }) {
hasSubtitle = isTwoLine || isThreeLine; hasSubtitle = isTwoLine || isThreeLine;
subtitleScaleFactor ??= textScaleFactor;
return new MaterialApp( return new MaterialApp(
home: new MediaQuery( home: new MediaQuery(
data: new MediaQueryData( data: new MediaQueryData(
...@@ -70,7 +69,7 @@ void main() { ...@@ -70,7 +69,7 @@ void main() {
child: new ListTile( child: new ListTile(
leading: new Container(key: leadingKey, width: 24.0, height: 24.0), leading: new Container(key: leadingKey, width: 24.0, height: 24.0),
title: const Text('title'), title: const Text('title'),
subtitle: hasSubtitle ? new Text('subtitle', textScaleFactor: subtitleScaleFactor) : null, subtitle: hasSubtitle ? const Text('subtitle') : null,
trailing: new Container(key: trailingKey, width: 24.0, height: 24.0), trailing: new Container(key: trailingKey, width: 24.0, height: 24.0),
dense: dense, dense: dense,
isThreeLine: isThreeLine, isThreeLine: isThreeLine,
...@@ -92,7 +91,6 @@ void main() { ...@@ -92,7 +91,6 @@ void main() {
double left(String text) => tester.getTopLeft(find.text(text)).dx; double left(String text) => tester.getTopLeft(find.text(text)).dx;
double top(String text) => tester.getTopLeft(find.text(text)).dy; double top(String text) => tester.getTopLeft(find.text(text)).dy;
double bottom(String text) => tester.getBottomLeft(find.text(text)).dy; double bottom(String text) => tester.getBottomLeft(find.text(text)).dy;
double height(String text) => tester.getRect(find.text(text)).height;
double leftKey(Key key) => tester.getTopLeft(find.byKey(key)).dx; double leftKey(Key key) => tester.getTopLeft(find.byKey(key)).dx;
double rightKey(Key key) => tester.getTopRight(find.byKey(key)).dx; double rightKey(Key key) => tester.getTopRight(find.byKey(key)).dx;
...@@ -100,7 +98,7 @@ void main() { ...@@ -100,7 +98,7 @@ void main() {
double heightKey(Key key) => tester.getSize(find.byKey(key)).height; double heightKey(Key key) => tester.getSize(find.byKey(key)).height;
// ListTiles are contained by a SafeArea defined like this: // ListTiles are contained by a SafeArea defined like this:
// SafeArea(top: false, bottom: false, minimum: contentPadding) // SafeArea(top: false, bottom: false, minimim: contentPadding)
// The default contentPadding is 16.0 on the left and right. // The default contentPadding is 16.0 on the left and right.
void testHorizontalGeometry() { void testHorizontalGeometry() {
expect(leftKey(leadingKey), math.max(16.0, leftPadding)); expect(leftKey(leadingKey), math.max(16.0, leftPadding));
...@@ -113,15 +111,9 @@ void main() { ...@@ -113,15 +111,9 @@ void main() {
} }
void testVerticalGeometry(double expectedHeight) { void testVerticalGeometry(double expectedHeight) {
final Rect tileRect = tester.getRect(find.byType(ListTile)); expect(tester.getSize(find.byType(ListTile)), new Size(800.0, expectedHeight));
expect(tileRect.size, new Size(800.0, expectedHeight)); if (hasSubtitle)
expect(top('title'), greaterThanOrEqualTo(tileRect.top));
if (hasSubtitle) {
expect(top('subtitle'), greaterThanOrEqualTo(bottom('title'))); expect(top('subtitle'), greaterThanOrEqualTo(bottom('title')));
expect(bottom('subtitle'), lessThan(tileRect.bottom));
} else {
expect(top('title'), equals(tileRect.top + (tileRect.height - height('title')) / 2.0));
}
expect(heightKey(trailingKey), 24.0); expect(heightKey(trailingKey), 24.0);
} }
...@@ -158,40 +150,35 @@ void main() { ...@@ -158,40 +150,35 @@ void main() {
await tester.pumpWidget(buildFrame(textScaleFactor: 4.0)); await tester.pumpWidget(buildFrame(textScaleFactor: 4.0));
testChildren(); testChildren();
testHorizontalGeometry(); testHorizontalGeometry();
testVerticalGeometry(72.0); testVerticalGeometry(64.0);
await tester.pumpWidget(buildFrame(dense: true, textScaleFactor: 4.0)); await tester.pumpWidget(buildFrame(dense: true, textScaleFactor: 4.0));
testChildren(); testChildren();
testHorizontalGeometry(); testHorizontalGeometry();
testVerticalGeometry(72.0); testVerticalGeometry(64.0);
await tester.pumpWidget(buildFrame(isTwoLine: true, textScaleFactor: 4.0)); await tester.pumpWidget(buildFrame(isTwoLine: true, textScaleFactor: 4.0));
testChildren(); testChildren();
testHorizontalGeometry(); testHorizontalGeometry();
testVerticalGeometry(128.0); testVerticalGeometry(120.0);
// Make sure that the height of a large subtitle is taken into account.
await tester.pumpWidget(buildFrame(isTwoLine: true, textScaleFactor: 0.5, subtitleScaleFactor: 4.0));
testChildren();
testHorizontalGeometry();
testVerticalGeometry(72.0);
await tester.pumpWidget(buildFrame(isTwoLine: true, dense: true, textScaleFactor: 4.0)); await tester.pumpWidget(buildFrame(isTwoLine: true, dense: true, textScaleFactor: 4.0));
testChildren(); testChildren();
testHorizontalGeometry(); testHorizontalGeometry();
testVerticalGeometry(128.0); testVerticalGeometry(120.0);
await tester.pumpWidget(buildFrame(isThreeLine: true, textScaleFactor: 4.0)); await tester.pumpWidget(buildFrame(isThreeLine: true, textScaleFactor: 4.0));
testChildren(); testChildren();
testHorizontalGeometry(); testHorizontalGeometry();
testVerticalGeometry(128.0); testVerticalGeometry(120.0);
await tester.pumpWidget(buildFrame(isThreeLine: true, dense: true, textScaleFactor: 4.0)); await tester.pumpWidget(buildFrame(isThreeLine: true, dense: true, textScaleFactor: 4.0));
testChildren(); testChildren();
testHorizontalGeometry(); testHorizontalGeometry();
testVerticalGeometry(128.0); testVerticalGeometry(120.0);
}); });
testWidgets('ListTile geometry (RTL)', (WidgetTester tester) async { testWidgets('ListTile geometry (RTL)', (WidgetTester tester) async {
const double leftPadding = 10.0; const double leftPadding = 10.0;
const double rightPadding = 20.0; const double rightPadding = 20.0;
...@@ -490,6 +477,7 @@ void main() { ...@@ -490,6 +477,7 @@ void main() {
expect(right('L'), 790.0); // 800 - contentPadding.start expect(right('L'), 790.0); // 800 - contentPadding.start
}); });
testWidgets('ListTileTheme wide leading Widget', (WidgetTester tester) async { testWidgets('ListTileTheme wide leading Widget', (WidgetTester tester) async {
const Key leadingKey = const ValueKey<String>('L'); const Key leadingKey = const ValueKey<String>('L');
...@@ -556,5 +544,7 @@ void main() { ...@@ -556,5 +544,7 @@ void main() {
expect(tester.getBottomLeft(find.byKey(leadingKey)), const Offset(800.0 - 56.0, 52.0)); expect(tester.getBottomLeft(find.byKey(leadingKey)), const Offset(800.0 - 56.0, 52.0));
expect(right('title'), 800.0 - 72.0); expect(right('title'), 800.0 - 72.0);
expect(right('subtitle'), 800.0 - 72.0); expect(right('subtitle'), 800.0 - 72.0);
}); });
} }
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