From cd125e1f71209b425cc4fdf36b62fcef771ff743 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 7 Feb 2023 17:17:23 -0800 Subject: [PATCH] Add test for RenderProxyBoxMixin; clarify doc, resolve TODO (#117664) * Add test for RenderProxyBoxMixin; clarify doc, resolve TODO The TODO comment suggested this mixin would no longer be needed once a Dart issue on inherited constructors was fixed: https://github.com/dart-lang/sdk/issues/31543 That issue is now long since fixed, so I went to go carry out the TODO. But in doing so, I realized that the mixin's documentation was more right than the TODO comment: even with that issue fixed, there is a legitimate use case for this mixin, namely to reuse the implementation of RenderProxyBox in a class that also inherits from some other base class. Moreover, searching GitHub I found an example of a library that makes real use of that capability. So I think the right resolution is to accept that this separation is useful and delete the TODO. Then, add a test with an extremely simplified sketch of that real-world example. In case someone in the future attempts to simplify this mixin away, the test will point us at the use case that would be broken by such a change. Also remove the only in-tree use of the mixin, which was redundant; and expand the mixin's documentation to advise about that case. * Tweak formatting Co-authored-by: Michael Goderbauer * Cut comments --------- Co-authored-by: Michael Goderbauer --- .../flutter/lib/src/rendering/proxy_box.dart | 8 +++--- .../test/rendering/proxy_box_test.dart | 26 +++++++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/packages/flutter/lib/src/rendering/proxy_box.dart b/packages/flutter/lib/src/rendering/proxy_box.dart index 3f9b68dd7a..93dbcb86a0 100644 --- a/packages/flutter/lib/src/rendering/proxy_box.dart +++ b/packages/flutter/lib/src/rendering/proxy_box.dart @@ -53,8 +53,10 @@ class RenderProxyBox extends RenderBox with RenderObjectWithChildMixin on RenderBox, RenderObjectWithChildMixin { @override @@ -1095,7 +1097,7 @@ mixin RenderAnimatedOpacityMixin on RenderObjectWithChil /// /// This is a variant of [RenderOpacity] that uses an [Animation] rather /// than a [double] to control the opacity. -class RenderAnimatedOpacity extends RenderProxyBox with RenderProxyBoxMixin, RenderAnimatedOpacityMixin { +class RenderAnimatedOpacity extends RenderProxyBox with RenderAnimatedOpacityMixin { /// Creates a partially transparent render object. /// /// The [opacity] argument must not be null. diff --git a/packages/flutter/test/rendering/proxy_box_test.dart b/packages/flutter/test/rendering/proxy_box_test.dart index b397a7eeb6..2a2ec4e3d0 100644 --- a/packages/flutter/test/rendering/proxy_box_test.dart +++ b/packages/flutter/test/rendering/proxy_box_test.dart @@ -963,6 +963,17 @@ void main() { expect(debugPaintClipOval(Clip.none), paintsExactlyCountTimes(#drawPath, 0)); expect(debugPaintClipOval(Clip.none), paintsExactlyCountTimes(#drawParagraph, 0)); }); + + test('RenderProxyBox behavior can be mixed in along with another base class', () { + final RenderFancyProxyBox fancyProxyBox = RenderFancyProxyBox(fancy: 6); + // Box has behavior from its base class: + expect(fancyProxyBox.fancyMethod(), 36); + // Box has behavior from RenderProxyBox: + expect( + fancyProxyBox.computeDryLayout(const BoxConstraints(minHeight: 8)), + const Size(0, 8), + ); + }); } class _TestRectClipper extends CustomClipper { @@ -1061,6 +1072,21 @@ class ConditionalRepaintBoundary extends RenderProxyBox { class TestOffsetLayerA extends OffsetLayer {} +class RenderFancyBox extends RenderBox { + RenderFancyBox({required this.fancy}) : super(); + + late int fancy; + + int fancyMethod() { + return fancy * fancy; + } +} + +class RenderFancyProxyBox extends RenderFancyBox + with RenderObjectWithChildMixin, RenderProxyBoxMixin { + RenderFancyProxyBox({required super.fancy}); +} + void expectAssertionError() { final FlutterErrorDetails errorDetails = TestRenderingFlutterBinding.instance.takeFlutterErrorDetails()!; final bool asserted = errorDetails.toString().contains('Failed assertion');