Skip to content

Commit 360e42c

Browse files
nate-thegratenate-thegrate
authored
Factor out Container objects (#153619)
This pull request follows up on [a PR from 4 months ago](flutter/flutter#147432) that aimed to reduce the number of `Container` objects in the framework. I feel like now's a good time to wrap it up! (especially since I've gained a grasp of how "rebase" vs. "merge commit" can [affect test results](https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#using-git) �) <br> resolves #147431
1 parent 5d5e633 commit 360e42c

21 files changed

+420
-347
lines changed

‎packages/flutter/lib/src/material/expansion_panel.dart‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -389,8 +389,8 @@ class _ExpansionPanelListState extends State<ExpansionPanelList> {
389389
_isChildExpanded(index),
390390
);
391391

392-
Widget expandIconContainer = Container(
393-
margin: const EdgeInsetsDirectional.only(end: 8.0),
392+
Widget expandIconPadded = Padding(
393+
padding: const EdgeInsetsDirectional.only(end: 8.0),
394394
child: ExpandIcon(
395395
color: widget.expandIconColor,
396396
disabledColor: child.canTapOnHeader ? widget.expandIconColor : null,
@@ -406,10 +406,10 @@ class _ExpansionPanelListState extends State<ExpansionPanelList> {
406406

407407
if (!child.canTapOnHeader) {
408408
final MaterialLocalizations localizations = MaterialLocalizations.of(context);
409-
expandIconContainer = Semantics(
409+
expandIconPadded = Semantics(
410410
label: _isChildExpanded(index)? localizations.expandedIconTapHint : localizations.collapsedIconTapHint,
411411
container: true,
412-
child: expandIconContainer,
412+
child: expandIconPadded,
413413
);
414414
}
415415
Widget header = Row(
@@ -425,7 +425,7 @@ class _ExpansionPanelListState extends State<ExpansionPanelList> {
425425
),
426426
),
427427
),
428-
expandIconContainer,
428+
expandIconPadded,
429429
],
430430
);
431431
if (child.canTapOnHeader) {
@@ -446,7 +446,7 @@ class _ExpansionPanelListState extends State<ExpansionPanelList> {
446446
children: <Widget>[
447447
header,
448448
AnimatedCrossFade(
449-
firstChild: Container(height: 0.0),
449+
firstChild: const LimitedBox(maxWidth: 0.0, child: SizedBox(width: double.infinity, height: 0)),
450450
secondChild: child.body,
451451
firstCurve: const Interval(0.0, 0.6, curve: Curves.fastOutSlowIn),
452452
secondCurve: const Interval(0.4, 1.0, curve: Curves.fastOutSlowIn),

‎packages/flutter/lib/src/material/flexible_space_bar.dart‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -273,9 +273,7 @@ class _FlexibleSpaceBarState extends State<FlexibleSpaceBar> {
273273
sigmaX: blurAmount,
274274
sigmaY: blurAmount,
275275
),
276-
child: Container(
277-
color: Colors.transparent,
278-
),
276+
child: const ColoredBox(color: Colors.transparent),
279277
),
280278
));
281279
}
@@ -331,7 +329,7 @@ class _FlexibleSpaceBarState extends State<FlexibleSpaceBar> {
331329
final Matrix4 scaleTransform = Matrix4.identity()
332330
..scale(scaleValue, scaleValue, 1.0);
333331
final Alignment titleAlignment = _getTitleAlignment(effectiveCenterTitle);
334-
children.add(Container(
332+
children.add(Padding(
335333
padding: padding,
336334
child: Transform(
337335
alignment: titleAlignment,
@@ -342,10 +340,12 @@ class _FlexibleSpaceBarState extends State<FlexibleSpaceBar> {
342340
style: titleStyle,
343341
child: LayoutBuilder(
344342
builder: (BuildContext context, BoxConstraints constraints) {
345-
return Container(
343+
return SizedBox(
346344
width: constraints.maxWidth / scaleValue,
347-
alignment: titleAlignment,
348-
child: title,
345+
child: Align(
346+
alignment: titleAlignment,
347+
child: title,
348+
),
349349
);
350350
},
351351
),

‎packages/flutter/lib/src/material/navigation_drawer.dart‎

Lines changed: 32 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -350,42 +350,47 @@ class _NavigationDestinationBuilder extends StatelessWidget {
350350
final NavigationDrawerThemeData navigationDrawerTheme = NavigationDrawerTheme.of(context);
351351
final NavigationDrawerThemeData defaults = _NavigationDrawerDefaultsM3(context);
352352

353-
final Row destinationBody = Row(
354-
children: <Widget>[
355-
const SizedBox(width: 16),
356-
buildIcon(context),
357-
const SizedBox(width: 12),
358-
buildLabel(context),
359-
],
353+
final InkWell inkWell = InkWell(
354+
highlightColor: Colors.transparent,
355+
onTap: enabled ? info.onTap : null,
356+
customBorder: info.indicatorShape ?? navigationDrawerTheme.indicatorShape ?? defaults.indicatorShape!,
357+
child: Stack(
358+
alignment: Alignment.center,
359+
children: <Widget>[
360+
NavigationIndicator(
361+
animation: info.selectedAnimation,
362+
color: info.indicatorColor ?? navigationDrawerTheme.indicatorColor ?? defaults.indicatorColor!,
363+
shape: info.indicatorShape ?? navigationDrawerTheme.indicatorShape ?? defaults.indicatorShape!,
364+
width: (navigationDrawerTheme.indicatorSize ?? defaults.indicatorSize!).width,
365+
height: (navigationDrawerTheme.indicatorSize ?? defaults.indicatorSize!).height,
366+
),
367+
Row(
368+
children: <Widget>[
369+
const SizedBox(width: 16),
370+
buildIcon(context),
371+
const SizedBox(width: 12),
372+
buildLabel(context),
373+
],
374+
),
375+
],
376+
),
360377
);
361378

362-
return Container(
379+
final Widget destination = Padding(
363380
padding: info.tilePadding,
364-
color: backgroundColor ?? navigationDrawerTheme.backgroundColor,
365381
child: _NavigationDestinationSemantics(
366382
child: SizedBox(
367383
height: navigationDrawerTheme.tileHeight ?? defaults.tileHeight,
368-
child: InkWell(
369-
highlightColor: Colors.transparent,
370-
onTap: enabled ? info.onTap : null,
371-
customBorder: info.indicatorShape ?? navigationDrawerTheme.indicatorShape ?? defaults.indicatorShape!,
372-
child: Stack(
373-
alignment: Alignment.center,
374-
children: <Widget>[
375-
NavigationIndicator(
376-
animation: info.selectedAnimation,
377-
color: info.indicatorColor ?? navigationDrawerTheme.indicatorColor ?? defaults.indicatorColor!,
378-
shape: info.indicatorShape ?? navigationDrawerTheme.indicatorShape ?? defaults.indicatorShape!,
379-
width: (navigationDrawerTheme.indicatorSize ?? defaults.indicatorSize!).width,
380-
height: (navigationDrawerTheme.indicatorSize ?? defaults.indicatorSize!).height,
381-
),
382-
destinationBody
383-
],
384-
),
385-
),
384+
child: inkWell,
386385
),
387386
),
388387
);
388+
389+
final Color? color = backgroundColor ?? navigationDrawerTheme.backgroundColor;
390+
if (color != null) {
391+
return ColoredBox(color: color, child: destination);
392+
}
393+
return destination;
389394
}
390395
}
391396

‎packages/flutter/lib/src/material/navigation_rail.dart‎

Lines changed: 51 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -746,39 +746,38 @@ class _RailDestinationState extends State<_RailDestination> {
746746
indicatorVerticalPadding + indicatorVerticalOffset,
747747
);
748748
}
749-
content = Container(
750-
constraints: BoxConstraints(
751-
minWidth: widget.minWidth,
752-
minHeight: minHeight,
753-
),
754-
padding: widget.padding ?? const EdgeInsets.symmetric(horizontal: _horizontalDestinationPadding),
755-
child: ClipRect(
756-
child: Column(
757-
mainAxisSize: MainAxisSize.min,
758-
mainAxisAlignment: MainAxisAlignment.center,
759-
children: <Widget>[
760-
topSpacing,
761-
_AddIndicator(
762-
addIndicator: widget.useIndicator,
763-
indicatorColor: widget.indicatorColor,
764-
indicatorShape: widget.indicatorShape,
765-
isCircular: false,
766-
indicatorAnimation: widget.destinationAnimation,
767-
child: themedIcon,
768-
),
769-
labelSpacing,
770-
Align(
771-
alignment: Alignment.topCenter,
772-
heightFactor: appearingAnimationValue,
773-
widthFactor: 1.0,
774-
child: FadeTransition(
775-
alwaysIncludeSemantics: true,
776-
opacity: labelFadeAnimation,
777-
child: styledLabel,
749+
content = ConstrainedBox(
750+
constraints: BoxConstraints(minWidth: widget.minWidth, minHeight: minHeight),
751+
child: Padding(
752+
padding: widget.padding ?? const EdgeInsets.symmetric(horizontal: _horizontalDestinationPadding),
753+
child: ClipRect(
754+
child: Column(
755+
mainAxisSize: MainAxisSize.min,
756+
mainAxisAlignment: MainAxisAlignment.center,
757+
children: <Widget>[
758+
topSpacing,
759+
_AddIndicator(
760+
addIndicator: widget.useIndicator,
761+
indicatorColor: widget.indicatorColor,
762+
indicatorShape: widget.indicatorShape,
763+
isCircular: false,
764+
indicatorAnimation: widget.destinationAnimation,
765+
child: themedIcon,
778766
),
779-
),
780-
bottomSpacing,
781-
],
767+
labelSpacing,
768+
Align(
769+
alignment: Alignment.topCenter,
770+
heightFactor: appearingAnimationValue,
771+
widthFactor: 1.0,
772+
child: FadeTransition(
773+
alwaysIncludeSemantics: true,
774+
opacity: labelFadeAnimation,
775+
child: styledLabel,
776+
),
777+
),
778+
bottomSpacing,
779+
],
780+
),
782781
),
783782
),
784783
);
@@ -799,27 +798,26 @@ class _RailDestinationState extends State<_RailDestination> {
799798
indicatorVerticalPadding + indicatorVerticalOffset,
800799
);
801800
}
802-
content = Container(
803-
constraints: BoxConstraints(
804-
minWidth: widget.minWidth,
805-
minHeight: minHeight,
806-
),
807-
padding: widget.padding ?? const EdgeInsets.symmetric(horizontal: _horizontalDestinationPadding),
808-
child: Column(
809-
children: <Widget>[
810-
topSpacing,
811-
_AddIndicator(
812-
addIndicator: widget.useIndicator,
813-
indicatorColor: widget.indicatorColor,
814-
indicatorShape: widget.indicatorShape,
815-
isCircular: false,
816-
indicatorAnimation: widget.destinationAnimation,
817-
child: themedIcon,
818-
),
819-
labelSpacing,
820-
styledLabel,
821-
bottomSpacing,
822-
],
801+
content = ConstrainedBox(
802+
constraints: BoxConstraints(minWidth: widget.minWidth, minHeight: minHeight),
803+
child: Padding(
804+
padding: widget.padding ?? const EdgeInsets.symmetric(horizontal: _horizontalDestinationPadding),
805+
child: Column(
806+
children: <Widget>[
807+
topSpacing,
808+
_AddIndicator(
809+
addIndicator: widget.useIndicator,
810+
indicatorColor: widget.indicatorColor,
811+
indicatorShape: widget.indicatorShape,
812+
isCircular: false,
813+
indicatorAnimation: widget.destinationAnimation,
814+
child: themedIcon,
815+
),
816+
labelSpacing,
817+
styledLabel,
818+
bottomSpacing,
819+
],
820+
),
823821
),
824822
);
825823
}

‎packages/flutter/lib/src/material/page_transitions_theme.dart‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -158,16 +158,18 @@ class _OpenUpwardsPageTransitionState extends State<_OpenUpwardsPageTransition>
158158
return AnimatedBuilder(
159159
animation: widget.animation,
160160
builder: (BuildContext context, Widget? child) {
161-
return Container(
161+
return ColoredBox(
162162
color: Colors.black.withOpacity(opacityAnimation.value),
163-
alignment: Alignment.bottomLeft,
164-
child: ClipRect(
165-
child: SizedBox(
166-
height: clipAnimation.value,
167-
child: OverflowBox(
168-
alignment: Alignment.bottomLeft,
169-
maxHeight: size.height,
170-
child: child,
163+
child: Align(
164+
alignment: Alignment.bottomLeft,
165+
child: ClipRect(
166+
child: SizedBox(
167+
height: clipAnimation.value,
168+
child: OverflowBox(
169+
alignment: Alignment.bottomLeft,
170+
maxHeight: size.height,
171+
child: child,
172+
),
171173
),
172174
),
173175
),

‎packages/flutter/lib/src/material/paginated_data_table.dart‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -500,7 +500,8 @@ class PaginatedDataTableState extends State<PaginatedDataTable> {
500500
})
501501
.toList();
502502
footerWidgets.addAll(<Widget>[
503-
Container(width: 14.0), // to match trailing padding in case we overflow and end up scrolling
503+
// Match trailing padding, in case we overflow and end up scrolling.
504+
const SizedBox(width: 14.0),
504505
Text(localizations.rowsPerPageTitle),
505506
ConstrainedBox(
506507
constraints: const BoxConstraints(minWidth: 64.0), // 40.0 for the text, 24.0 for the icon
@@ -519,7 +520,7 @@ class PaginatedDataTableState extends State<PaginatedDataTable> {
519520
]);
520521
}
521522
footerWidgets.addAll(<Widget>[
522-
Container(width: 32.0),
523+
const SizedBox(width: 32.0),
523524
Text(
524525
localizations.pageRowsInfoTitle(
525526
_firstRowIndex + 1,
@@ -528,7 +529,7 @@ class PaginatedDataTableState extends State<PaginatedDataTable> {
528529
_rowCountApproximate,
529530
),
530531
),
531-
Container(width: 32.0),
532+
const SizedBox(width: 32.0),
532533
if (widget.showFirstLastButtons)
533534
IconButton(
534535
icon: Icon(Icons.skip_previous, color: widget.arrowHeadColor),
@@ -542,7 +543,7 @@ class PaginatedDataTableState extends State<PaginatedDataTable> {
542543
tooltip: localizations.previousPageTooltip,
543544
onPressed: _firstRowIndex <= 0 ? null : _handlePrevious,
544545
),
545-
Container(width: 24.0),
546+
const SizedBox(width: 24.0),
546547
IconButton(
547548
icon: Icon(Icons.chevron_right, color: widget.arrowHeadColor),
548549
padding: EdgeInsets.zero,
@@ -558,7 +559,7 @@ class PaginatedDataTableState extends State<PaginatedDataTable> {
558559
? null
559560
: _handleLast,
560561
),
561-
Container(width: 14.0),
562+
const SizedBox(width: 14.0),
562563
]);
563564

564565
// CARD

‎packages/flutter/lib/src/material/popup_menu.dart‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -375,15 +375,22 @@ class PopupMenuItemState<T, W extends PopupMenuItem<T>> extends State<W> {
375375
if (!widget.enabled && !theme.useMaterial3) {
376376
style = style.copyWith(color: theme.disabledColor);
377377
}
378+
final EdgeInsetsGeometry padding = widget.padding
379+
?? (theme.useMaterial3 ? _PopupMenuDefaultsM3.menuItemPadding : _PopupMenuDefaultsM2.menuItemPadding);
378380

379381
Widget item = AnimatedDefaultTextStyle(
380382
style: style,
381383
duration: kThemeChangeDuration,
382-
child: Container(
383-
alignment: AlignmentDirectional.centerStart,
384+
child: ConstrainedBox(
384385
constraints: BoxConstraints(minHeight: widget.height),
385-
padding: widget.padding ?? (theme.useMaterial3 ? _PopupMenuDefaultsM3.menuItemPadding : _PopupMenuDefaultsM2.menuItemPadding),
386-
child: buildChild(),
386+
child: Padding(
387+
key: const Key('menu item padding'),
388+
padding: padding,
389+
child: Align(
390+
alignment: AlignmentDirectional.centerStart,
391+
child: buildChild(),
392+
),
393+
),
387394
),
388395
);
389396

0 commit comments

Comments
 (0)