Remove textScaleFactor dependent logic from AppBar
#128112
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,6 +31,8 @@ import 'theme.dart'; | |
| // late String _logoAsset; | ||
| // double _myToolbarHeight = 250.0; | ||
|
|
||
| typedef _FlexibleConfigBuilder = _ScrollUnderFlexibleConfig Function(BuildContext); | ||
|
|
||
| const double _kLeadingWidth = kToolbarHeight; // So the leading button is square. | ||
| const double _kMaxTitleTextScaleFactor = 1.34; // TODO(perc): Add link to Material spec when available, https://github.com/flutter/flutter/issues/58769. | ||
|
|
||
|
|
@@ -1259,17 +1261,15 @@ class _SliverAppBarDelegate extends SliverPersistentHeaderDelegate { | |
| final double toolbarOpacity = !pinned || isPinnedWithOpacityFade | ||
| ? clampDouble(visibleToolbarHeight / (toolbarHeight ?? kToolbarHeight), 0.0, 1.0) | ||
| : 1.0; | ||
| final Widget? effectiveTitle = switch (variant) { | ||
| _SliverAppVariant.small => title, | ||
| _SliverAppVariant.medium || _SliverAppVariant.large => AnimatedOpacity( | ||
| opacity: isScrolledUnder ? 1 : 0, | ||
| duration: const Duration(milliseconds: 500), | ||
| curve: const Cubic(0.2, 0.0, 0.0, 1.0), | ||
| child: title, | ||
| ), | ||
| }; | ||
|
|
||
| final Widget appBar = FlexibleSpaceBar.createSettings( | ||
| minExtent: minExtent, | ||
|
|
@@ -1971,8 +1971,7 @@ class _SliverAppBarState extends State<SliverAppBar> with TickerProviderStateMix | |
| effectiveFlexibleSpace = widget.flexibleSpace ?? _ScrollUnderFlexibleSpace( | ||
| title: widget.title, | ||
| foregroundColor: widget.foregroundColor, | ||
| configBuilder: _MediumScrollUnderFlexibleConfig.new, | ||
| titleTextStyle: widget.titleTextStyle, | ||
| bottomHeight: bottomHeight, | ||
| ); | ||
|
|
@@ -1984,8 +1983,7 @@ class _SliverAppBarState extends State<SliverAppBar> with TickerProviderStateMix | |
| effectiveFlexibleSpace = widget.flexibleSpace ?? _ScrollUnderFlexibleSpace( | ||
| title: widget.title, | ||
| foregroundColor: widget.foregroundColor, | ||
| configBuilder: _LargeScrollUnderFlexibleConfig.new, | ||
| titleTextStyle: widget.titleTextStyle, | ||
| bottomHeight: bottomHeight, | ||
| ); | ||
|
|
@@ -2081,79 +2079,48 @@ class _RenderAppBarTitleBox extends RenderAligningShiftedBox { | |
| } | ||
| } | ||
|
|
||
| class _ScrollUnderFlexibleSpace extends StatelessWidget { | ||
| const _ScrollUnderFlexibleSpace({ | ||
| this.title, | ||
| this.foregroundColor, | ||
| required this.configBuilder, | ||
| this.titleTextStyle, | ||
| required this.bottomHeight, | ||
| }); | ||
|
|
||
| final Widget? title; | ||
| final Color? foregroundColor; | ||
| final _FlexibleConfigBuilder configBuilder; | ||
| final TextStyle? titleTextStyle; | ||
| final double bottomHeight; | ||
|
|
||
| @override | ||
| Widget build(BuildContext context) { | ||
| late final AppBarTheme appBarTheme = AppBarTheme.of(context); | ||
TahaTesser marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| late final AppBarTheme defaults = Theme.of(context).useMaterial3 ? _AppBarDefaultsM3(context) : _AppBarDefaultsM2(context); | ||
TahaTesser marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| final double textScaleFactor = math.min(MediaQuery.textScaleFactorOf(context), _kMaxTitleTextScaleFactor); // TODO(tahatesser): Add link to Material spec when available, https://github.com/flutter/flutter/issues/58769. | ||
| final FlexibleSpaceBarSettings settings = context.dependOnInheritedWidgetOfExactType<FlexibleSpaceBarSettings>()!; | ||
| final _ScrollUnderFlexibleConfig config = configBuilder(context); | ||
| assert( | ||
| config.expandedTitlePadding.isNonNegative, | ||
| 'The _ExpandedTitleWithPadding widget assumes that the expanded title padding is non-negative. ' | ||
| 'Update its implementation to handle negative padding.', | ||
| ); | ||
|
|
||
| final TextStyle? expandedTextStyle = titleTextStyle | ||
| ?? appBarTheme.titleTextStyle | ||
| ?? config.expandedTextStyle?.copyWith(color: foregroundColor ?? appBarTheme.foregroundColor ?? defaults.foregroundColor); | ||
|
|
||
| final Widget? expandedTitle = switch ((title, expandedTextStyle)) { | ||
|
There was a problem hiding this comment. This is slick but I'm not sure the logic is the same or even simpler than before. if (title != null) {
...
expandedTitle = config.expandedTextStyle != null
? DefaultTextStyle(
style: expandedTextStyle!,
child: title!,
)
: title;
}Would only wrap the title with the DefaultTextStyle if There was a problem hiding this comment. @TahaTesser does this change look ok to you? There was a problem hiding this comment. This looks good to me!
|
||
| (null, _) => null, | ||
| (final Widget title, null) => title, | ||
| (final Widget title, final TextStyle textStyle) => DefaultTextStyle(style: textStyle, child: title), | ||
| }; | ||
|
|
||
| final EdgeInsets resolvedTitlePadding = config.expandedTitlePadding.resolve(Directionality.of(context)); | ||
| final EdgeInsetsGeometry expandedTitlePadding = bottomHeight > 0 | ||
| ? resolvedTitlePadding.copyWith(bottom: 0) | ||
| : resolvedTitlePadding; | ||
|
|
||
| // Set maximum text scale factor to [_kMaxTitleTextScaleFactor] for the | ||
| // title to keep the visual hierarchy the same even with larger font | ||
|
|
@@ -2162,35 +2129,167 @@ class _ScrollUnderFlexibleSpace extends StatelessWidget { | |
| // `MediaQuery.textScaleFactorOf(context)`. | ||
| return MediaQuery( | ||
| data: MediaQuery.of(context).copyWith(textScaleFactor: textScaleFactor), | ||
| // This column will assume the full height of the parent Stack. | ||
| child: Column( | ||
| children: <Widget>[ | ||
| Padding(padding: EdgeInsets.only(top: settings.minExtent - bottomHeight)), | ||
| Flexible( | ||
| child: ClipRect( | ||
| child: _ExpandedTitleWithPadding( | ||
| padding: expandedTitlePadding, | ||
| maxExtent: settings.maxExtent - settings.minExtent, | ||
| child: expandedTitle, | ||
| ), | ||
| ), | ||
| ), | ||
| // Reserve space for AppBar.bottom, which is a sibling of this widget, | ||
| // on the parent Stack. | ||
| if (bottomHeight > 0) Padding(padding: EdgeInsets.only(bottom: bottomHeight)), | ||
| ], | ||
| ), | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| // A widget that bottom-start aligns its child (the expanded title widget), and | ||
| // insets the child according to the specified padding. | ||
| // | ||
| // This widget gives the child an infinite max height constraint, and will also | ||
| // attempt to vertically limit the child's bounding box (not including the | ||
| // padding) to within the y range [0, maxExtent], to make sure the child is | ||
| // visible when the AppBar is fully expanded. | ||
| class _ExpandedTitleWithPadding extends SingleChildRenderObjectWidget { | ||
| const _ExpandedTitleWithPadding({ | ||
| required this.padding, | ||
| required this.maxExtent, | ||
| super.child, | ||
| }); | ||
|
|
||
| final EdgeInsetsGeometry padding; | ||
| final double maxExtent; | ||
|
|
||
| @override | ||
| _RenderExpandedTitleBox createRenderObject(BuildContext context) { | ||
| final TextDirection textDirection = Directionality.of(context); | ||
| return _RenderExpandedTitleBox( | ||
| padding.resolve(textDirection), | ||
| AlignmentDirectional.bottomStart.resolve(textDirection), | ||
| maxExtent, | ||
| null, | ||
| ); | ||
| } | ||
|
|
||
| @override | ||
| void updateRenderObject(BuildContext context, _RenderExpandedTitleBox renderObject) { | ||
| final TextDirection textDirection = Directionality.of(context); | ||
| renderObject | ||
| ..padding = padding.resolve(textDirection) | ||
| ..titleAlignment = AlignmentDirectional.bottomStart.resolve(textDirection) | ||
| ..maxExtent = maxExtent; | ||
| } | ||
| } | ||
|
|
||
| class _RenderExpandedTitleBox extends RenderShiftedBox { | ||
| _RenderExpandedTitleBox(this._padding, this._titleAlignment, this._maxExtent, super.child); | ||
|
|
||
| EdgeInsets get padding => _padding; | ||
| EdgeInsets _padding; | ||
| set padding(EdgeInsets value) { | ||
| if (_padding == value) { | ||
| return; | ||
| } | ||
| assert(value.isNonNegative); | ||
| _padding = value; | ||
| markNeedsLayout(); | ||
| } | ||
|
|
||
| Alignment get titleAlignment => _titleAlignment; | ||
| Alignment _titleAlignment; | ||
| set titleAlignment(Alignment value) { | ||
| if (_titleAlignment == value) { | ||
| return; | ||
| } | ||
| _titleAlignment = value; | ||
| markNeedsLayout(); | ||
| } | ||
|
|
||
| double get maxExtent => _maxExtent; | ||
| double _maxExtent; | ||
| set maxExtent(double value) { | ||
| if (_maxExtent == value) { | ||
| return; | ||
| } | ||
| _maxExtent = value; | ||
| markNeedsLayout(); | ||
| } | ||
|
|
||
| @override | ||
| double computeMaxIntrinsicHeight(double width) { | ||
| final RenderBox? child = this.child; | ||
| return child == null ? 0.0 : child.getMaxIntrinsicHeight(math.max(0, width - padding.horizontal)) + padding.vertical; | ||
| } | ||
|
|
||
| @override | ||
| double computeMaxIntrinsicWidth(double height) { | ||
| final RenderBox? child = this.child; | ||
| return child == null ? 0.0 : child.getMaxIntrinsicWidth(double.infinity) + padding.horizontal; | ||
| } | ||
|
|
||
| @override | ||
| double computeMinIntrinsicHeight(double width) { | ||
| final RenderBox? child = this.child; | ||
| return child == null ? 0.0 : child.getMinIntrinsicHeight(math.max(0, width - padding.horizontal)) + padding.vertical; | ||
| } | ||
|
|
||
| @override | ||
| double computeMinIntrinsicWidth(double height) { | ||
| final RenderBox? child = this.child; | ||
| return child == null ? 0.0 : child.getMinIntrinsicWidth(double.infinity) + padding.horizontal; | ||
| } | ||
|
|
||
| Size _computeSize(BoxConstraints constraints, ChildLayouter layoutChild) { | ||
| final RenderBox? child = this.child; | ||
| if (child == null) { | ||
| return Size.zero; | ||
| } | ||
| layoutChild(child, constraints.widthConstraints().deflate(padding)); | ||
| return constraints.biggest; | ||
| } | ||
|
|
||
| @override | ||
| Size computeDryLayout(BoxConstraints constraints) => _computeSize(constraints, ChildLayoutHelper.dryLayoutChild); | ||
|
|
||
| @override | ||
| void performLayout() { | ||
| final RenderBox? child = this.child; | ||
| if (child == null) { | ||
| this.size = constraints.smallest; | ||
| return; | ||
| } | ||
| final Size size = this.size = _computeSize(constraints, ChildLayoutHelper.layoutChild); | ||
| final Size childSize = child.size; | ||
|
|
||
| assert(padding.isNonNegative); | ||
| assert(titleAlignment.y == 1.0); | ||
| // yAdjustement is the minimum additional y offset to shift the child in | ||
| // the visible vertical space when AppBar is fully expanded. The goal is to | ||
| // prevent the expanded title from being clipped when the expanded title | ||
| // widget + the bottom padding is too tall to fit in the flexible space (the | ||
| // top padding is basically ignored since the expanded title is | ||
| // bottom-aligned). | ||
| final double yAdjustement = clampDouble(childSize.height + padding.bottom - maxExtent, 0, padding.bottom); | ||
| final double offsetY = size.height - childSize.height - padding.bottom + yAdjustement; | ||
| final double offsetX = (titleAlignment.x + 1) / 2 * (size.width - padding.horizontal - childSize.width) + padding.left; | ||
|
|
||
| final BoxParentData childParentData = child.parentData! as BoxParentData; | ||
| childParentData.offset = Offset(offsetX, offsetY); | ||
| } | ||
| } | ||
|
|
||
| mixin _ScrollUnderFlexibleConfig { | ||
| TextStyle? get collapsedTextStyle; | ||
| TextStyle? get expandedTextStyle; | ||
| EdgeInsetsGeometry get expandedTitlePadding; | ||
| } | ||
|
|
||
| // Hand coded defaults based on Material Design 2. | ||
|
|
@@ -2298,7 +2397,7 @@ class _MediumScrollUnderFlexibleConfig with _ScrollUnderFlexibleConfig { | |
| _textTheme.headlineSmall?.apply(color: _colors.onSurface); | ||
|
|
||
| @override | ||
| EdgeInsetsGeometry get expandedTitlePadding => const EdgeInsets.fromLTRB(16, 0, 16, 20); | ||
| } | ||
|
|
||
| class _LargeScrollUnderFlexibleConfig with _ScrollUnderFlexibleConfig { | ||
|
|
@@ -2321,7 +2420,7 @@ class _LargeScrollUnderFlexibleConfig with _ScrollUnderFlexibleConfig { | |
| _textTheme.headlineMedium?.apply(color: _colors.onSurface); | ||
|
|
||
| @override | ||
| EdgeInsetsGeometry get expandedTitlePadding => const EdgeInsets.fromLTRB(16, 0, 16, 28); | ||
| } | ||
|
|
||
| // END GENERATED TOKEN PROPERTIES - AppBar | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NICE
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1