Added clipBehavior to Overlay, Flow, AnimatedSize and AndroidView#65910
Added clipBehavior to Overlay, Flow, AnimatedSize and AndroidView#65910fluttergithubbot merged 5 commits intoflutter:masterfrom thecalamiity:issue-63246
Conversation
flutter-dashboard
bot
added
the
framework
liyuqian
left a comment
liyuqian
left a comment
There was a problem hiding this comment.
Thanks for writing this patch! It mostly looks good, but let's avoid the breaking change, a very lengthy process, by setting default clip to hardEdge for now. I think that would also fix some widget test failures with clip assertions.
| AlignmentGeometry alignment = Alignment.center, | ||
| TextDirection? textDirection, | ||
| RenderBox? child, | ||
| Clip clipBehavior = Clip.none, |
There was a problem hiding this comment.
Let's default this to Clip.hardEdge now to avoid breaking changes. We can change the default and go through the lengthy breaking change process later.
| /// | ||
| /// Defaults to [Clip.none], and must not be null. | ||
| Clip get clipBehavior => _clipBehavior; | ||
| Clip _clipBehavior = Clip.none; |
| RenderFlow({ | ||
| List<RenderBox>? children, | ||
| required FlowDelegate delegate, | ||
| Clip clipBehavior = Clip.none, |
| /// | ||
| /// Defaults to [Clip.none], and must not be null. | ||
| Clip get clipBehavior => _clipBehavior; | ||
| Clip _clipBehavior = Clip.none; |
| required AndroidViewController viewController, | ||
| required PlatformViewHitTestBehavior hitTestBehavior, | ||
| required Set<Factory<OneSequenceGestureRecognizer>> gestureRecognizers, | ||
| Clip clipBehavior = Clip.none, |
| const Overlay({ | ||
| Key? key, | ||
| this.initialEntries = const <OverlayEntry>[], | ||
| this.clipBehavior = Clip.none, |
|
|
||
| /// {@macro flutter.widgets.Clip} | ||
| /// | ||
| /// Defaults to [Clip.none], and must not be null. |
There was a problem hiding this comment.
remember to change documentation too when the default is changed to Clip.hardEdge.
| _Theatre({ | ||
| Key? key, | ||
| this.skipCount = 0, | ||
| this.clipBehavior = Clip.none, |
| List<RenderBox>? children, | ||
| required TextDirection textDirection, | ||
| int skipCount = 0, | ||
| Clip clipBehavior = Clip.none, |
| /// | ||
| /// Defaults to [Clip.none], and must not be null. | ||
| Clip get clipBehavior => _clipBehavior; | ||
| Clip _clipBehavior = Clip.none; |
|
@liyuqian Did the same to AndroidView and AnimatedSize because some tests were failing |
| @override | ||
| void paint(PaintingContext context, Offset offset) { | ||
| context.pushClipRect(needsCompositing, offset, Offset.zero & size, _paintWithDelegate); | ||
| if(clipBehavior == Clip.none) { |
There was a problem hiding this comment.
nit: space after if and before (.
flar
left a comment
flar
left a comment
There was a problem hiding this comment.
This is already merged, but I think that line of code needs to be reexamined. I will submit an issue to double check it.
| // (see comment in _paintTexture for an explanation of when this happens). | ||
| if (size.width < _currentAndroidViewSize.width || size.height < _currentAndroidViewSize.height) { | ||
| context.pushClipRect(true, offset, offset & size, _paintTexture); | ||
| if (size.width < _currentAndroidViewSize.width || size.height < _currentAndroidViewSize.height && clipBehavior != Clip.none) { |
There was a problem hiding this comment.
&& has higher precedence than ||
I have no idea what the logical flow was intended to be here, but it looks wrong.
There was a problem hiding this comment.
That does look wrong. Wanna send a PR to fix it?
There was a problem hiding this comment.
Agree it's wrong. Thanks for finding it!
There was a problem hiding this comment.
So sorry for this guys 😞 I somehow missed it
6 participants
Description
Added clipBehavior to Overlay, Flow, AnimatedSize and AndroidView.
/cc @liyuqian
Related Issues
Closes #63246
Tests
Added tests to check if the mentioned widgets can set and update their clipBehaviors.
Checklist
Before you create this PR, confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Did any tests fail when you ran them? Please read Handling breaking changes.