diff --git a/packages/flutter/lib/src/widgets/slotted_render_object_widget.dart b/packages/flutter/lib/src/widgets/slotted_render_object_widget.dart index 7f3613a2da..8bbf6a56ef 100644 --- a/packages/flutter/lib/src/widgets/slotted_render_object_widget.dart +++ b/packages/flutter/lib/src/widgets/slotted_render_object_widget.dart @@ -182,6 +182,15 @@ mixin SlottedContainerRenderObjectMixin on RenderBox { adoptChild(child); } } + + void _moveChild(RenderBox child, S slot, S oldSlot) { + assert(slot != oldSlot); + final RenderBox? oldChild = _slotToChild[oldSlot]; + if (oldChild == child) { + _setChild(null, oldSlot); + } + _setChild(child, slot); + } } /// Element used by the [SlottedMultiChildRenderObjectWidgetMixin]. @@ -189,7 +198,8 @@ class SlottedRenderObjectElement extends RenderObjectElement { /// Creates an element that uses the given widget as its configuration. SlottedRenderObjectElement(SlottedMultiChildRenderObjectWidgetMixin super.widget); - final Map _slotToChild = {}; + Map _slotToChild = {}; + Map _keyedChildren = {}; @override SlottedContainerRenderObjectMixin get renderObject => super.renderObject as SlottedContainerRenderObjectMixin; @@ -231,21 +241,73 @@ class SlottedRenderObjectElement extends RenderObjectElement { }(), '${widget.runtimeType}.slots must not change.'); assert(slottedMultiChildRenderObjectWidgetMixin.slots.toSet().length == slottedMultiChildRenderObjectWidgetMixin.slots.length, 'slots must be unique'); + final Map oldKeyedElements = _keyedChildren; + _keyedChildren = {}; + final Map oldSlotToChild = _slotToChild; + _slotToChild = {}; + + Map>? debugDuplicateKeys; + for (final S slot in slottedMultiChildRenderObjectWidgetMixin.slots) { - _updateChild(slottedMultiChildRenderObjectWidgetMixin.childForSlot(slot), slot); + final Widget? widget = slottedMultiChildRenderObjectWidgetMixin.childForSlot(slot); + final Key? newWidgetKey = widget?.key; + + final Element? oldSlotChild = oldSlotToChild[slot]; + final Element? oldKeyChild = oldKeyedElements[newWidgetKey]; + + // Try to find the slot for the correct Element that `widget` should update. + // If key matching fails, resort to `oldSlotChild` from the same slot. + final Element? fromElement; + if (oldKeyChild != null) { + fromElement = oldSlotToChild.remove(oldKeyChild.slot as S); + } else if (oldSlotChild?.widget.key == null) { + fromElement = oldSlotToChild.remove(slot); + } else { + // The only case we can't use `oldSlotChild` is when its widget has a key. + assert(oldSlotChild!.widget.key != newWidgetKey); + fromElement = null; + } + final Element? newChild = updateChild(fromElement, widget, slot); + + if (newChild != null) { + _slotToChild[slot] = newChild; + + if (newWidgetKey != null) { + assert(() { + final Element? existingElement = _keyedChildren[newWidgetKey]; + if (existingElement != null) { + (debugDuplicateKeys ??= >{}) + .putIfAbsent(newWidgetKey, () => [existingElement]) + .add(newChild); + } + return true; + }()); + _keyedChildren[newWidgetKey] = newChild; + } + } } + oldSlotToChild.values.forEach(deactivateChild); + assert(_debugDuplicateKeys(debugDuplicateKeys)); + assert(_keyedChildren.values.every(_slotToChild.values.contains), '_keyedChildren ${_keyedChildren.values} should be a subset of ${_slotToChild.values}'); } - void _updateChild(Widget? widget, S slot) { - final Element? oldChild = _slotToChild[slot]; - assert(oldChild == null || oldChild.slot == slot); // Reason why [moveRenderObjectChild] is not reachable. - final Element? newChild = updateChild(oldChild, widget, slot); - if (oldChild != null) { - _slotToChild.remove(slot); + bool _debugDuplicateKeys(Map>? debugDuplicateKeys) { + if (debugDuplicateKeys == null) { + return true; } - if (newChild != null) { - _slotToChild[slot] = newChild; + for (final MapEntry> duplicateKey in debugDuplicateKeys.entries) { + throw FlutterError.fromParts([ + ErrorSummary('Multiple widgets used the same key in ${widget.runtimeType}.'), + ErrorDescription( + 'The key ${duplicateKey.key} was used by multiple widgets. The parents of those widgets were:\n' + ), + for (final Element element in duplicateKey.value) ErrorDescription(' - $element\n'), + ErrorDescription( + 'A key can only be specified on one widget at a time in the same parent widget.', + ), + ]); } + return true; } @override @@ -256,14 +318,14 @@ class SlottedRenderObjectElement extends RenderObjectElement { @override void removeRenderObjectChild(RenderBox child, S slot) { - assert(renderObject._slotToChild[slot] == child); - renderObject._setChild(null, slot); - assert(renderObject._slotToChild[slot] == null); + if (renderObject._slotToChild[slot] == child) { + renderObject._setChild(null, slot); + assert(renderObject._slotToChild[slot] == null); + } } @override - void moveRenderObjectChild(RenderBox child, Object? oldSlot, Object? newSlot) { - // Existing elements are never moved to a new slot, see assert in [_updateChild]. - assert(false, 'not reachable'); + void moveRenderObjectChild(RenderBox child, S oldSlot, S newSlot) { + renderObject._moveChild(child, newSlot, oldSlot); } } diff --git a/packages/flutter/test/widgets/slotted_render_object_widget_test.dart b/packages/flutter/test/widgets/slotted_render_object_widget_test.dart index 43f7bebfed..7fc1a9ba62 100644 --- a/packages/flutter/test/widgets/slotted_render_object_widget_test.dart +++ b/packages/flutter/test/widgets/slotted_render_object_widget_test.dart @@ -139,6 +139,72 @@ void main() { expect(_RenderTest().publicNameForSlot(slot), slot.toString()); }); + testWidgets('key reparenting', (WidgetTester tester) async { + const Widget widget1 = SizedBox(key: ValueKey('smol'), height: 10, width: 10); + const Widget widget2 = SizedBox(key: ValueKey('big'), height: 100, width: 100); + const Widget nullWidget = SizedBox(key: ValueKey('null'), height: 50, width: 50); + + await tester.pumpWidget(buildWidget(topLeft: widget1, bottomRight: widget2, nullSlot: nullWidget)); + final _RenderDiagonal renderObject = tester.renderObject(find.byType(_Diagonal)); + expect(renderObject._topLeft!.size, const Size(10, 10)); + expect(renderObject._bottomRight!.size, const Size(100, 100)); + expect(renderObject._nullSlot!.size, const Size(50, 50)); + + final Element widget1Element = tester.element(find.byWidget(widget1)); + final Element widget2Element = tester.element(find.byWidget(widget2)); + final Element nullWidgetElement = tester.element(find.byWidget(nullWidget)); + + // Swapping 1 and 2. + await tester.pumpWidget(buildWidget(topLeft: widget2, bottomRight: widget1, nullSlot: nullWidget)); + expect(renderObject._topLeft!.size, const Size(100, 100)); + expect(renderObject._bottomRight!.size, const Size(10, 10)); + expect(renderObject._nullSlot!.size, const Size(50, 50)); + expect(widget1Element, same(tester.element(find.byWidget(widget1)))); + expect(widget2Element, same(tester.element(find.byWidget(widget2)))); + expect(nullWidgetElement, same(tester.element(find.byWidget(nullWidget)))); + + // Shifting slots + await tester.pumpWidget(buildWidget(topLeft: nullWidget, bottomRight: widget2, nullSlot: widget1)); + expect(renderObject._topLeft!.size, const Size(50, 50)); + expect(renderObject._bottomRight!.size, const Size(100, 100)); + expect(renderObject._nullSlot!.size, const Size(10, 10)); + expect(widget1Element, same(tester.element(find.byWidget(widget1)))); + expect(widget2Element, same(tester.element(find.byWidget(widget2)))); + expect(nullWidgetElement, same(tester.element(find.byWidget(nullWidget)))); + + // Moving + Deleting. + await tester.pumpWidget(buildWidget(bottomRight: widget2)); + expect(renderObject._topLeft, null); + expect(renderObject._bottomRight!.size, const Size(100, 100)); + expect(renderObject._nullSlot, null); + expect(widget1Element.debugIsDefunct, isTrue); + expect(nullWidgetElement.debugIsDefunct, isTrue); + expect(widget2Element, same(tester.element(find.byWidget(widget2)))); + + // Moving. + await tester.pumpWidget(buildWidget(topLeft: widget2)); + expect(renderObject._topLeft!.size, const Size(100, 100)); + expect(renderObject._bottomRight, null); + expect(widget2Element, same(tester.element(find.byWidget(widget2)))); + }); + + testWidgets('duplicated key error message', (WidgetTester tester) async { + const Widget widget1 = SizedBox(key: ValueKey('widget 1'), height: 10, width: 10); + const Widget widget2 = SizedBox(key: ValueKey('widget 1'), height: 100, width: 100); + const Widget widget3 = SizedBox(key: ValueKey('widget 1'), height: 50, width: 50); + + await tester.pumpWidget(buildWidget(topLeft: widget1, bottomRight: widget2, nullSlot: widget3)); + + expect((tester.takeException() as FlutterError).toString(), equalsIgnoringHashCodes( + 'Multiple widgets used the same key in _Diagonal.\n' + "The key [<'widget 1'>] was used by multiple widgets. The parents of those widgets were:\n" + " - SizedBox-[<'widget 1'>](width: 50.0, height: 50.0, renderObject: RenderConstrainedBox#00000 NEEDS-LAYOUT NEEDS-PAINT)\n" + " - SizedBox-[<'widget 1'>](width: 10.0, height: 10.0, renderObject: RenderConstrainedBox#00000 NEEDS-LAYOUT NEEDS-PAINT)\n" + " - SizedBox-[<'widget 1'>](width: 100.0, height: 100.0, renderObject: RenderConstrainedBox#a4685 NEEDS-LAYOUT NEEDS-PAINT)\n" + 'A key can only be specified on one widget at a time in the same parent widget.' + )); + }); + testWidgets('debugDescribeChildren', (WidgetTester tester) async { await tester.pumpWidget(buildWidget( topLeft: const SizedBox( @@ -178,7 +244,7 @@ _RenderDiagonal#00000 relayoutBoundary=up1 }); } -Widget buildWidget({Widget? topLeft, Widget? bottomRight}) { +Widget buildWidget({Widget? topLeft, Widget? bottomRight, Widget? nullSlot}) { return Directionality( textDirection: TextDirection.ltr, child: Align( @@ -186,6 +252,7 @@ Widget buildWidget({Widget? topLeft, Widget? bottomRight}) { child: _Diagonal( topLeft: topLeft, bottomRight: bottomRight, + nullSlot: nullSlot, ), ), ); @@ -196,21 +263,25 @@ enum _DiagonalSlot { bottomRight, } -class _Diagonal extends RenderObjectWidget with SlottedMultiChildRenderObjectWidgetMixin<_DiagonalSlot> { +class _Diagonal extends RenderObjectWidget with SlottedMultiChildRenderObjectWidgetMixin<_DiagonalSlot?> { const _Diagonal({ this.topLeft, this.bottomRight, + this.nullSlot, }); final Widget? topLeft; final Widget? bottomRight; + final Widget? nullSlot; @override - Iterable<_DiagonalSlot> get slots => _DiagonalSlot.values; + Iterable<_DiagonalSlot?> get slots => <_DiagonalSlot?>[null, ..._DiagonalSlot.values]; @override - Widget? childForSlot(_DiagonalSlot slot) { + Widget? childForSlot(_DiagonalSlot? slot) { switch (slot) { + case null: + return nullSlot; case _DiagonalSlot.topLeft: return topLeft; case _DiagonalSlot.bottomRight: @@ -219,16 +290,17 @@ class _Diagonal extends RenderObjectWidget with SlottedMultiChildRenderObjectWid } @override - SlottedContainerRenderObjectMixin<_DiagonalSlot> createRenderObject( + SlottedContainerRenderObjectMixin<_DiagonalSlot?> createRenderObject( BuildContext context, ) { return _RenderDiagonal(); } } -class _RenderDiagonal extends RenderBox with SlottedContainerRenderObjectMixin<_DiagonalSlot> { +class _RenderDiagonal extends RenderBox with SlottedContainerRenderObjectMixin<_DiagonalSlot?> { RenderBox? get _topLeft => childForSlot(_DiagonalSlot.topLeft); RenderBox? get _bottomRight => childForSlot(_DiagonalSlot.bottomRight); + RenderBox? get _nullSlot => childForSlot(null); @override void performLayout() { @@ -251,9 +323,17 @@ class _RenderDiagonal extends RenderBox with SlottedContainerRenderObjectMixin<_ bottomRightSize = _bottomRight!.size; } + Size nullSlotSize = Size.zero; + final RenderBox? nullSlot = _nullSlot; + if (nullSlot != null) { + nullSlot.layout(childConstraints, parentUsesSize: true); + _positionChild(nullSlot, Offset.zero); + nullSlotSize = nullSlot.size; + } + size = constraints.constrain(Size( - topLeftSize.width + bottomRightSize.width, - topLeftSize.height + bottomRightSize.height, + topLeftSize.width + bottomRightSize.width + nullSlotSize.width, + topLeftSize.height + bottomRightSize.height + nullSlotSize.height, )); }