Skip to content

Commit

Permalink
Fix chip baseline implementation (flutter#146162)
Browse files Browse the repository at this point in the history
It didn't include the label's Y offset. Also fixes the nullability of the slotted children: it seems `avatar`, `label`, `deleteIcon` can't be null when `_RenderChip` is in the render tree. We were using 
```dart
    _boxParentData(avatar!).offset = theme.padding.topLeft + avatarOffset;
    _boxParentData(label!).offset = theme.padding.topLeft + labelOffset + theme.labelPadding.topLeft;
    _boxParentData(deleteIcon!).offset = theme.padding.topLeft + deleteIconOffset;
```
in `performLayout` anyway.
  • Loading branch information
LongCatIsLooong authored Apr 3, 2024
1 parent 1088e51 commit bb7977e
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 112 deletions.
174 changes: 62 additions & 112 deletions packages/flutter/lib/src/material/chip.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1644,9 +1644,9 @@ class _RenderChip extends RenderBox with SlottedContainerRenderObjectMixin<_Chip
Animation<double> enableAnimation;
ShapeBorder? avatarBorder;

RenderBox? get avatar => childForSlot(_ChipSlot.avatar);
RenderBox? get deleteIcon => childForSlot(_ChipSlot.deleteIcon);
RenderBox? get label => childForSlot(_ChipSlot.label);
RenderBox get avatar => childForSlot(_ChipSlot.avatar)!;
RenderBox get deleteIcon => childForSlot(_ChipSlot.deleteIcon)!;
RenderBox get label => childForSlot(_ChipSlot.label)!;

_ChipRenderTheme get theme => _theme;
_ChipRenderTheme _theme;
Expand Down Expand Up @@ -1691,13 +1691,16 @@ class _RenderChip extends RenderBox with SlottedContainerRenderObjectMixin<_Chip
// The returned list is ordered for hit testing.
@override
Iterable<RenderBox> get children {
final RenderBox? avatar = childForSlot(_ChipSlot.avatar);
final RenderBox? label = childForSlot(_ChipSlot.label);
final RenderBox? deleteIcon = childForSlot(_ChipSlot.deleteIcon);
return <RenderBox>[
if (avatar != null)
avatar!,
avatar,
if (label != null)
label!,
label,
if (deleteIcon != null)
deleteIcon!,
deleteIcon,
];
}

Expand All @@ -1707,21 +1710,7 @@ class _RenderChip extends RenderBox with SlottedContainerRenderObjectMixin<_Chip
@override
bool get sizedByParent => false;

static double _minWidth(RenderBox? box, double height) {
return box == null ? 0.0 : box.getMinIntrinsicWidth(height);
}

static double _maxWidth(RenderBox? box, double height) {
return box == null ? 0.0 : box.getMaxIntrinsicWidth(height);
}

static double _minHeight(RenderBox? box, double width) {
return box == null ? 0.0 : box.getMinIntrinsicHeight(width);
}

static Size _boxSize(RenderBox? box) => box == null ? Size.zero : box.size;

static Rect _boxRect(RenderBox? box) => box == null ? Rect.zero : _boxParentData(box).offset & box.size;
static Rect _boxRect(RenderBox box) => _boxParentData(box).offset & box.size;

static BoxParentData _boxParentData(RenderBox box) => box.parentData! as BoxParentData;

Expand All @@ -1732,27 +1721,27 @@ class _RenderChip extends RenderBox with SlottedContainerRenderObjectMixin<_Chip
// when they're missing.
final double overallPadding = theme.padding.horizontal +
theme.labelPadding.horizontal;
return overallPadding +
_minWidth(avatar, height) +
_minWidth(label, height) +
_minWidth(deleteIcon, height);
return overallPadding
+ avatar.getMinIntrinsicWidth(height)
+ label.getMinIntrinsicWidth(height)
+ deleteIcon.getMinIntrinsicWidth(height);
}

@override
double computeMaxIntrinsicWidth(double height) {
final double overallPadding = theme.padding.horizontal +
theme.labelPadding.horizontal;
return overallPadding +
_maxWidth(avatar, height) +
_maxWidth(label, height) +
_maxWidth(deleteIcon, height);
return overallPadding
+ avatar.getMaxIntrinsicWidth(height)
+ label.getMaxIntrinsicWidth(height)
+ deleteIcon.getMaxIntrinsicWidth(height);
}

@override
double computeMinIntrinsicHeight(double width) {
return math.max(
_kChipHeight,
theme.padding.vertical + theme.labelPadding.vertical + _minHeight(label, width),
theme.padding.vertical + theme.labelPadding.vertical + label.getMinIntrinsicHeight(width),
);
}

Expand All @@ -1762,48 +1751,25 @@ class _RenderChip extends RenderBox with SlottedContainerRenderObjectMixin<_Chip
@override
double? computeDistanceToActualBaseline(TextBaseline baseline) {
// The baseline of this widget is the baseline of the label.
return label!.getDistanceToActualBaseline(baseline);
return (BaselineOffset(label.getDistanceToActualBaseline(baseline)) + _boxParentData(label).offset.dy).offset;
}

Size _layoutLabel(BoxConstraints contentConstraints, double iconSizes, Size size, Size rawSize, [ChildLayouter layoutChild = ChildLayoutHelper.layoutChild]) {
// Now that we know the label height and the width of the icons, we can
// determine how much to shrink the width constraints for the "real" layout.
if (contentConstraints.maxWidth.isFinite) {
final double maxWidth = math.max(
0.0,
contentConstraints.maxWidth
- iconSizes
- theme.labelPadding.horizontal
- theme.padding.horizontal,
);
final Size updatedSize = layoutChild(
label!,
BoxConstraints(
maxWidth: maxWidth,
minHeight: rawSize.height,
maxHeight: size.height,
),
);

return Size(
updatedSize.width + theme.labelPadding.horizontal,
updatedSize.height + theme.labelPadding.vertical,
);
}

final Size updatedSize = layoutChild(
label!,
BoxConstraints(
minHeight: rawSize.height,
maxHeight: size.height,
maxWidth: size.width,
),
);

return Size(
updatedSize.width + theme.labelPadding.horizontal,
updatedSize.height + theme.labelPadding.vertical,
final double maxLabelWidth = contentConstraints.maxWidth.isFinite
? math.max(
0.0,
contentConstraints.maxWidth - iconSizes - theme.labelPadding.horizontal - theme.padding.horizontal,
)
: size.width;
final BoxConstraints labelConstraints = BoxConstraints(
minHeight: rawSize.height,
maxHeight: size.height,
maxWidth: maxLabelWidth,
);
final Size updatedSize = layoutChild(label, labelConstraints);
return theme.labelPadding.inflateSize(updatedSize);
}

Size _layoutAvatar(double contentSize, [ChildLayouter layoutChild = ChildLayoutHelper.layoutChild]) {
Expand All @@ -1812,19 +1778,12 @@ class _RenderChip extends RenderBox with SlottedContainerRenderObjectMixin<_Chip
width: requestedSize,
height: requestedSize,
);
final Size avatarBoxSize = layoutChild(avatar!, avatarConstraints);
final Size avatarBoxSize = layoutChild(avatar, avatarConstraints);
if (!theme.showCheckmark && !theme.showAvatar) {
return Size(0.0, contentSize);
}
double avatarWidth = 0.0;
double avatarHeight = 0.0;
if (theme.showAvatar) {
avatarWidth += avatarDrawerAnimation.value * avatarBoxSize.width;
} else {
avatarWidth += avatarDrawerAnimation.value * contentSize;
}
avatarHeight += avatarBoxSize.height;
return Size(avatarWidth, avatarHeight);
final double avatarFullWidth = theme.showAvatar ? avatarBoxSize.width : contentSize;
return Size(avatarFullWidth * avatarDrawerAnimation.value, avatarBoxSize.height);
}

Size _layoutDeleteIcon(double contentSize, [ChildLayouter layoutChild = ChildLayoutHelper.layoutChild]) {
Expand All @@ -1833,46 +1792,37 @@ class _RenderChip extends RenderBox with SlottedContainerRenderObjectMixin<_Chip
width: requestedSize,
height: requestedSize,
);
final Size boxSize = layoutChild(deleteIcon!, deleteIconConstraints);
final Size boxSize = layoutChild(deleteIcon, deleteIconConstraints);
if (!deleteIconShowing) {
return Size(0.0, contentSize);
}
double deleteIconWidth = 0.0;
double deleteIconHeight = 0.0;
deleteIconWidth += deleteDrawerAnimation.value * boxSize.width;
deleteIconHeight += boxSize.height;
return Size(deleteIconWidth, deleteIconHeight);
return Size(deleteDrawerAnimation.value * boxSize.width, boxSize.height);
}

@override
bool hitTest(BoxHitTestResult result, { required Offset position }) {
if (!size.contains(position)) {
return false;
}
final bool hitIsOnDeleteIcon = deleteIcon != null && _hitIsOnDeleteIcon(
final bool hitIsOnDeleteIcon = _hitIsOnDeleteIcon(
padding: theme.padding,
labelPadding: theme.labelPadding,
tapPosition: position,
chipSize: size,
deleteButtonSize: deleteIcon!.size,
deleteButtonSize: deleteIcon.size,
textDirection: textDirection,
);
final RenderBox? hitTestChild = hitIsOnDeleteIcon
? (deleteIcon ?? label ?? avatar)
: (label ?? avatar);

if (hitTestChild != null) {
final Offset center = hitTestChild.size.center(Offset.zero);
return result.addWithRawTransform(
transform: MatrixUtils.forceToPoint(center),
position: position,
hitTest: (BoxHitTestResult result, Offset position) {
assert(position == center);
return hitTestChild.hitTest(result, position: center);
},
);
}
return false;
final RenderBox hitTestChild = hitIsOnDeleteIcon ? deleteIcon : label;

final Offset center = hitTestChild.size.center(Offset.zero);
return result.addWithRawTransform(
transform: MatrixUtils.forceToPoint(center),
position: position,
hitTest: (BoxHitTestResult result, Offset position) {
assert(position == center);
return hitTestChild.hitTest(result, position: center);
},
);
}

@override
Expand All @@ -1884,7 +1834,7 @@ class _RenderChip extends RenderBox with SlottedContainerRenderObjectMixin<_Chip
final BoxConstraints contentConstraints = constraints.loosen();
// Find out the height of the label within the constraints.
final Offset densityAdjustment = Offset(0.0, theme.visualDensity.baseSizeAdjustment.dy / 2.0);
final Size rawLabelSize = layoutChild(label!, contentConstraints);
final Size rawLabelSize = layoutChild(label, contentConstraints);
final double contentSize = math.max(
_kChipHeight - theme.padding.vertical + theme.labelPadding.vertical,
rawLabelSize.height + theme.labelPadding.vertical,
Expand Down Expand Up @@ -1981,7 +1931,7 @@ class _RenderChip extends RenderBox with SlottedContainerRenderObjectMixin<_Chip
case TextDirection.ltr:
double start = left;
if (theme.showCheckmark || theme.showAvatar) {
avatarOffset = centerLayout(sizes.avatar, start - _boxSize(avatar).width + sizes.avatar.width);
avatarOffset = centerLayout(sizes.avatar, start - avatar.size.width + sizes.avatar.width);
start += sizes.avatar.width;
}
labelOffset = centerLayout(sizes.label, start);
Expand All @@ -1998,7 +1948,7 @@ class _RenderChip extends RenderBox with SlottedContainerRenderObjectMixin<_Chip
} else {
_pressRect = Rect.zero;
}
start -= _boxSize(deleteIcon).width - sizes.deleteIcon.width;
start -= deleteIcon.size.width - sizes.deleteIcon.width;
if (deleteIconShowing) {
deleteIconOffset = centerLayout(sizes.deleteIcon, start);
_deleteButtonRect = Rect.fromLTWH(
Expand All @@ -2015,11 +1965,11 @@ class _RenderChip extends RenderBox with SlottedContainerRenderObjectMixin<_Chip
labelOffset = labelOffset +
Offset(
0.0,
((sizes.label.height - theme.labelPadding.vertical) - _boxSize(label).height) / 2.0,
((sizes.label.height - theme.labelPadding.vertical) - label.size.height) / 2.0,
);
_boxParentData(avatar!).offset = theme.padding.topLeft + avatarOffset;
_boxParentData(label!).offset = theme.padding.topLeft + labelOffset + theme.labelPadding.topLeft;
_boxParentData(deleteIcon!).offset = theme.padding.topLeft + deleteIconOffset;
_boxParentData(avatar).offset = theme.padding.topLeft + avatarOffset;
_boxParentData(label).offset = theme.padding.topLeft + labelOffset + theme.labelPadding.topLeft;
_boxParentData(deleteIcon).offset = theme.padding.topLeft + deleteIconOffset;
final Size paddedSize = Size(
sizes.overall.width + theme.padding.horizontal,
sizes.overall.height + theme.padding.vertical,
Expand Down Expand Up @@ -2070,7 +2020,7 @@ class _RenderChip extends RenderBox with SlottedContainerRenderObjectMixin<_Chip
final Paint paint = Paint()
..color = paintColor!
..style = PaintingStyle.stroke
..strokeWidth = _kCheckmarkStrokeWidth * (avatar != null ? avatar!.size.height / 24.0 : 1.0);
..strokeWidth = _kCheckmarkStrokeWidth * avatar.size.height / 24.0;
final double t = checkmarkAnimation.status == AnimationStatus.reverse
? 1.0
: checkmarkAnimation.value;
Expand Down Expand Up @@ -2111,9 +2061,9 @@ class _RenderChip extends RenderBox with SlottedContainerRenderObjectMixin<_Chip
context.canvas.drawPath(path, darkenPaint);
}
// Need to make the check mark be a little smaller than the avatar.
final double checkSize = avatar!.size.height * 0.75;
final Offset checkOffset = _boxParentData(avatar!).offset +
Offset(avatar!.size.height * 0.125, avatar!.size.height * 0.125);
final double checkSize = avatar.size.height * 0.75;
final Offset checkOffset = _boxParentData(avatar).offset +
Offset(avatar.size.height * 0.125, avatar.size.height * 0.125);
_paintCheck(context.canvas, offset + checkOffset, checkSize);
}
}
Expand All @@ -2122,7 +2072,7 @@ class _RenderChip extends RenderBox with SlottedContainerRenderObjectMixin<_Chip

void _paintAvatar(PaintingContext context, Offset offset) {
void paintWithOverlay(PaintingContext context, Offset offset) {
context.paintChild(avatar!, _boxParentData(avatar!).offset + offset);
context.paintChild(avatar, _boxParentData(avatar).offset + offset);
_paintSelectionOverlay(context, offset);
}

Expand Down
21 changes: 21 additions & 0 deletions packages/flutter/test/material/chip_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5684,6 +5684,27 @@ void main() {
expect(labelStyle.style.color, equals(Colors.black.withAlpha(0xde)));
});
});

testWidgets('Chip Baseline location', (WidgetTester tester) async {
const Text text = Text('A', style: TextStyle(fontSize: 10.0, height: 1.0));
await tester.pumpWidget(wrapForChip(child: const Align(
child: Row(
crossAxisAlignment: CrossAxisAlignment.baseline,
textBaseline: TextBaseline.alphabetic,
children: <Widget>[
text,
RawChip(label: text)
],
),
)));

expect(find.text('A'), findsNWidgets(2));
// Baseline aligning text.
expect(
tester.getTopLeft(find.text('A').first).dy,
tester.getTopLeft(find.text('A').last).dy,
);
});
}

class _MaterialStateOutlinedBorder extends StadiumBorder implements MaterialStateOutlinedBorder {
Expand Down

0 comments on commit bb7977e

Please sign in to comment.