[Animations] Add support for custom shadows to OpenContainer (#62475)#11401
[Animations] Add support for custom shadows to OpenContainer (#62475)#11401moepanda wants to merge 2 commits intoflutter:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds support for custom closedShadows and openShadows to the OpenContainer widget, enabling custom shadow animations during transitions. It also fixes a layout overflow in the example app by transitioning from a fixed height to minimum height constraints and includes new tests to verify shadow behavior. A review comment suggests conditionally wrapping the Material widget with DecoratedBox only when custom shadows are provided to avoid unnecessary widgets in the tree.
2f3b47b to
88d57a1
Compare
88d57a1 to
f6d83d3
Compare
hannah-hyj
left a comment
There was a problem hiding this comment.
LGTM, thank you for fixing the layout overflow issue too!
Thank you! Glad to help. |
|
From triage: @hannah-hyj Please add a second reviewer familiar with the package, so this can get the secondary review for landing. |
QuncCccccc
left a comment
There was a problem hiding this comment.
Thank you for your contribution! Overall looks good! Just left one comment for adding an additional unit test.
| final Widget material = Material( | ||
| clipBehavior: widget.clipBehavior, | ||
| color: widget.closedColor, | ||
| elevation: widget.closedShadows == null ? widget.closedElevation : 0.0, |
There was a problem hiding this comment.
Just for my learning, why do we need to set elevation to 0.0 when closedShadows is non-null.
There was a problem hiding this comment.
We set elevation to 0.0 so the shadow comes from a single source when custom shadows are provided.
In this case the visual shadow is drawn by the wrapping DecoratedBox, so keeping Material.elevation would stack the framework shadow on top of the custom BoxShadows.
The same rule is applied in the fully-open state as well.
| animationDuration: Duration.zero, | ||
| color: colorTween!.evaluate(animation), | ||
| shape: _shapeTween.evaluate(curvedAnimation), | ||
| elevation: _elevationTween.evaluate(curvedAnimation), |
There was a problem hiding this comment.
do we need to check currentShadows here like above?
There was a problem hiding this comment.
This is intentional. The elevation tween already accounts for whether custom shadows are provided on either side, so mixed cases can smoothly transition between Material elevation and custom shadows. If we zeroed elevation whenever currentShadows is non-null, the transition would become abrupt in those mixed cases.
| expect(dataClosed.rect, dataTransitionDone.rect); | ||
| }); | ||
|
|
||
| testWidgets('Custom shadows work', (WidgetTester tester) async { |
There was a problem hiding this comment.
Can we add a test case for when one of the shadow properties is null and the other is provided? This would ensure the fallback to elevation behavior is verified and doesn't break in the future.
There was a problem hiding this comment.
Good point, I added mixed null/non-null shadow test coverage for both directions to lock in the elevation fallback behavior during the transition.
Add coverage for null/non-null shadow combinations in OpenContainer. This verifies the elevation fallback behavior during mixed shadow transitions.
Summary
This PR adds support for custom
BoxShadowto theOpenContainerwidget. This allows developers to specifyclosedShadowsandopenShadowsto create more advanced and visually appealing transitions that go beyond the standard materialelevation._ExampleSingleTileby replacing the fixed height with responsive constraints, ensuring the example remains stable across different system font sizes and shadow distributions.Motivation
Currently,
OpenContaineronly supports a numericelevationproperty, which limits modern UI designs requiring colored or multi-layered shadows. This PR also refactors the internal rendering hierarchy to move the shadow decoration outside the clipping surface, fixing a legacy issue where custom shadows were clipped byMaterial.Issues
Fixes flutter/flutter#62475
Pre-Review Checklist
[animations]///).