Unverified Commit cd125e1f authored by Greg Price's avatar Greg Price Committed by GitHub

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: 's avatarMichael Goderbauer <goderbauer@google.com>

* Cut comments

---------
Co-authored-by: 's avatarMichael Goderbauer <goderbauer@google.com>
parent 98b3e48e
......@@ -53,8 +53,10 @@ class RenderProxyBox extends RenderBox with RenderObjectWithChildMixin<RenderBox
///
/// Use this mixin in situations where the proxying behavior
/// of [RenderProxyBox] is desired but inheriting from [RenderProxyBox] is
/// impractical (e.g. because you want to mix in other classes as well).
// TODO(ianh): Remove this class once https://github.com/dart-lang/sdk/issues/31543 is fixed
/// impractical (e.g. because you want to inherit from a different class).
///
/// If a class already inherits from [RenderProxyBox] and also uses this mixin,
/// you can safely delete the use of the mixin.
@optionalTypeArgs
mixin RenderProxyBoxMixin<T extends RenderBox> on RenderBox, RenderObjectWithChildMixin<T> {
@override
......@@ -1095,7 +1097,7 @@ mixin RenderAnimatedOpacityMixin<T extends RenderObject> on RenderObjectWithChil
///
/// This is a variant of [RenderOpacity] that uses an [Animation<double>] rather
/// than a [double] to control the opacity.
class RenderAnimatedOpacity extends RenderProxyBox with RenderProxyBoxMixin, RenderAnimatedOpacityMixin<RenderBox> {
class RenderAnimatedOpacity extends RenderProxyBox with RenderAnimatedOpacityMixin<RenderBox> {
/// Creates a partially transparent render object.
///
/// The [opacity] argument must not be null.
......
......@@ -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<Rect> {
......@@ -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<RenderBox>, RenderProxyBoxMixin<RenderBox> {
RenderFancyProxyBox({required super.fancy});
}
void expectAssertionError() {
final FlutterErrorDetails errorDetails = TestRenderingFlutterBinding.instance.takeFlutterErrorDetails()!;
final bool asserted = errorDetails.toString().contains('Failed assertion');
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment