Skip to content

Clip search view content during animation #126975

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
auto-submit merged 1 commit into from
May 18, 2023
Merged

Clip search view content during animation #126975

auto-submit merged 1 commit into from
May 18, 2023

Conversation

@QuncCccccc
Copy link
Contributor

@QuncCccccc QuncCccccc commented May 17, 2023 •
edited
Loading

Fixes #126590

This PR is to clip the view content when the view size is not big enough, such as during animation.
Also removes some unused properties in _ViewContent class: getRect and viewConstraints

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I signed the [CLA].
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • All existing and new tests are passing.

@flutter-dashboard flutter-dashboard bot added f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels May 17, 2023
@QuncCccccc QuncCccccc force-pushed the fix_crash_during_animation branch from 699d302 to d7101f7 Compare May 17, 2023 03:44
@QuncCccccc QuncCccccc force-pushed the fix_crash_during_animation branch from d7101f7 to 1c12fb5 Compare May 17, 2023 03:55
bernaferrari
bernaferrari reviewed May 17, 2023
View reviewed changes
packages/flutter/lib/src/material/search_anchor.dart
Comment on lines +784 to +785
child: ClipRect(
clipBehavior: Clip.antiAlias,
Copy link
Contributor

@bernaferrari bernaferrari May 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Material has clipBehavior, maybe you could use its clipBehavior instead?

Copy link
Contributor

@bernaferrari bernaferrari May 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edit: it has and it is already being used. I don't understand why. Is this ClipRect needed?

Copy link
Contributor Author

@QuncCccccc QuncCccccc May 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two clips are for different purposes. The clipBehavior in Material is clipping the overflow text, the hovered color and the thumb of the scrollbar to match the view's shape:
Screenshot 2023-05-16 at 9 32 59 PM

But during the animation, when the view route is too small to contain the content list, for example a ListTile with leading and trailing widgets, a crash will still happen(#126590). To fix that, I use the OverflowBox to give fixed constraints, then use the ClipRect to clip the overflowed content from OverflowBox. It's parent constraints is not fixed because it is animating.

Please let me know if this doesn't make sense:) Any suggestions for the fix are welcome!!

Copy link
Contributor

@bernaferrari bernaferrari May 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I understand, lol. It is confusing, but I got it! Thanks for explaining. Stack also requires something like this :)

@QuncCccccc QuncCccccc changed the title Clip content during animation Clip search view content during animation May 17, 2023
@QuncCccccc QuncCccccc marked this pull request as ready for review May 17, 2023 16:56
@QuncCccccc QuncCccccc requested a review from HansMuller May 17, 2023 17:00
HansMuller
HansMuller approved these changes May 17, 2023
View reviewed changes
Copy link
Contributor

@HansMuller HansMuller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@QuncCccccc QuncCccccc added the autosubmit Merge PR when tree becomes green via auto submit App label May 18, 2023
@auto-submit auto-submit bot merged commit f31dae2 into master May 18, 2023
@auto-submit auto-submit bot deleted the fix_crash_during_animation branch May 18, 2023 00:18
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request May 18, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 18, 2023
@engine-flutter-autoroll
f31dae2 Clip search view content during animation (flutter/flutter#12…
8fccdb9 
…6975)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request May 18, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 18, 2023
@engine-flutter-autoroll
f31dae2 Clip search view content during animation (flutter/flutter#12…
243a0ac 
…6975)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request May 18, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 18, 2023
@engine-flutter-autoroll
f31dae2 Clip search view content during animation (flutter/flutter#12…
d105c5f 
…6975)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request May 18, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 18, 2023
@engine-flutter-autoroll
f31dae2 Clip search view content during animation (flutter/flutter#12…
a8c2628 
…6975)
auto-submit bot pushed a commit to flutter/packages that referenced this pull request May 18, 2023
@engine-flutter-autoroll
Roll Flutter from d0d1feb to 5ae6438 (42 revisions) (#4038)
1e214d7 
Roll Flutter from d0d1feb to 5ae6438 (42 revisions)

flutter/flutter@d0d1feb...5ae6438

2023-05-18 [email protected] Revert "Handle null return from WillPopCallback" (flutter/flutter#127112)
2023-05-18 [email protected] Roll Flutter Engine from a22dd6438335 to 843ce0bba356 (6 revisions) (flutter/flutter#127116)
2023-05-18 [email protected] mark windows build tests non-bringup (flutter/flutter#127059)
2023-05-18 [email protected] Roll Flutter Engine from d97037077963 to a22dd6438335 (1 revision) (flutter/flutter#127071)
2023-05-18 [email protected] Roll Packages from 5c69914 to b31a128 (9 revisions) (flutter/flutter#127109)
2023-05-18 [email protected] Revert "Remove obsolete drawShadow bounds workaround" (flutter/flutter#127110)
2023-05-18 [email protected] Clip search view content during animation (flutter/flutter#126975)
2023-05-18 [email protected] Remove obsolete drawShadow bounds workaround (flutter/flutter#127052)
2023-05-17 [email protected] [Windows] Improve version migration message (flutter/flutter#127048)
2023-05-17 49699333+dependabot[bot]@users.noreply.github.com Bump actions/labeler from 9471598e3b7ff22b2fa181bd79addf94cb3e0847 to 6b107e7a7ee5e054e0bcce60de5181d21e2f00fb (flutter/flutter#127056)
2023-05-17 [email protected] Remove deprecated fixTextFieldOutlineLabel (flutter/flutter#125893)
2023-05-17 [email protected] Roll Flutter Engine from 331c5769e291 to d97037077963 (3 revisions) (flutter/flutter#127053)
2023-05-17 [email protected] shard windows build tests 3 -> 4 (flutter/flutter#127057)
2023-05-17 [email protected] Roll Flutter Engine from 0ae3719d7043 to 331c5769e291 (1 revision) (flutter/flutter#127049)
2023-05-17 [email protected] Ignore unused_element_parameter (flutter/flutter#126926)
2023-05-17 [email protected] Roll Flutter Engine from 47fd496c6f8d to 0ae3719d7043 (2 revisions) (flutter/flutter#127043)
2023-05-17 [email protected] Roll Flutter Engine from 400a26ad8343 to 47fd496c6f8d (3 revisions) (flutter/flutter#127041)
2023-05-17 [email protected] [flutter_tools] unpin and roll camera_android (flutter/flutter#126945)
2023-05-17 [email protected] Handle null return from WillPopCallback (flutter/flutter#127039)
2023-05-17 [email protected] Roll Flutter Engine from 6048360c1837 to 400a26ad8343 (1 revision) (flutter/flutter#127036)
2023-05-17 [email protected] Remove unused (and untested) parameters from private _MasterDetailFlow (flutter/flutter#126935)
2023-05-17 [email protected] Autocomplete async examples (flutter/flutter#126283)
2023-05-17 [email protected] Expose callback that allows focus traversal customization (flutter/flutter#120235)
2023-05-17 [email protected] Roll Packages from b971830 to 5c69914 (5 revisions) (flutter/flutter#127034)
2023-05-17 [email protected] Roll Flutter Engine from ea3f27383acc to 6048360c1837 (2 revisions) (flutter/flutter#127032)
2023-05-17 [email protected] Add missing `Switch.onFocusChange` test (flutter/flutter#126685)
2023-05-17 [email protected] Remove 'url_launcher' from pubspec.yaml. (flutter/flutter#126939)
2023-05-17 [email protected] Roll Flutter Engine from 4b7c37532ea4 to ea3f27383acc (1 revision) (flutter/flutter#126994)
2023-05-17 [email protected] Roll Flutter Engine from 170b45bae571 to 4b7c37532ea4 (1 revision) (flutter/flutter#126985)
2023-05-17 [email protected] Roll Flutter Engine from 28b9dc993c30 to 170b45bae571 (1 revision) (flutter/flutter#126981)
2023-05-17 [email protected] Roll Flutter Engine from 87a03e107df7 to 28b9dc993c30 (1 revision) (flutter/flutter#126974)
2023-05-17 [email protected] Roll Flutter Engine from 1c775e34e2d5 to 87a03e107df7 (3 revisions) (flutter/flutter#126968)
2023-05-16 [email protected] Fix style issues (flutter/flutter#122586)
2023-05-16 [email protected] Manual roll Flutter Engine from fe2476743b59 to 1c775e34e2d5 (10 revisions) (flutter/flutter#126961)
2023-05-16 [email protected] Make SlottedMultiChildRenderObjectWidgetMixin a concrete class (flutter/flutter#126108)
2023-05-16 [email protected] Add ScrollMetrics.extentTotal for completeness (flutter/flutter#126607)
2023-05-16 [email protected] Fix drone_dimensions. (flutter/flutter#126953)
2023-05-16 [email protected] Add checkmark style to CupertinoRadio (flutter/flutter#126480)
2023-05-16 [email protected] Move `Mac_build_test flutter_gallery__transition_perf_e2e_ios` to prod (flutter/flutter#126941)
2023-05-16 [email protected] Fix DataTableThemeData.copyWith handling of dataRowHeight (flutter/flutter#126943)
2023-05-16 [email protected] Fix copyWith method of ActionIconThemeData (flutter/flutter#126763)
2023-05-16 [email protected] Revert "Roll Flutter Engine from fe2476743b59 to 5cf141f7c03c (2 revisions)" (flutter/flutter#126954)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
...
@HansMuller HansMuller mentioned this pull request May 19, 2023
Closed
CaseyHillers pushed a commit to CaseyHillers/flutter that referenced this pull request May 24, 2023
@QuncCccccc
Clip search view content during animation (flutter#126975)
b0fa867 
Fixes flutter#126590

This PR is to clip the view content when the view size is not big enough, such as during animation.
Also removes some unused properties in `_ViewContent` class: `getRect` and `viewConstraints`
This was referenced May 25, 2023
Merged
Merged
nploi pushed a commit to nploi/packages that referenced this pull request Jul 16, 2023
@engine-flutter-autoroll @nploi
Roll Flutter from d0d1feb to 5ae6438 (42 revisions) (flutter#4038)
bfd8555 
Roll Flutter from d0d1feb to 5ae6438 (42 revisions)

flutter/flutter@d0d1feb...5ae6438

2023-05-18 [email protected] Revert "Handle null return from WillPopCallback" (flutter/flutter#127112)
2023-05-18 [email protected] Roll Flutter Engine from a22dd6438335 to 843ce0bba356 (6 revisions) (flutter/flutter#127116)
2023-05-18 [email protected] mark windows build tests non-bringup (flutter/flutter#127059)
2023-05-18 [email protected] Roll Flutter Engine from d97037077963 to a22dd6438335 (1 revision) (flutter/flutter#127071)
2023-05-18 [email protected] Roll Packages from 5c69914 to b31a128 (9 revisions) (flutter/flutter#127109)
2023-05-18 [email protected] Revert "Remove obsolete drawShadow bounds workaround" (flutter/flutter#127110)
2023-05-18 [email protected] Clip search view content during animation (flutter/flutter#126975)
2023-05-18 [email protected] Remove obsolete drawShadow bounds workaround (flutter/flutter#127052)
2023-05-17 [email protected] [Windows] Improve version migration message (flutter/flutter#127048)
2023-05-17 49699333+dependabot[bot]@users.noreply.github.com Bump actions/labeler from 9471598e3b7ff22b2fa181bd79addf94cb3e0847 to 6b107e7a7ee5e054e0bcce60de5181d21e2f00fb (flutter/flutter#127056)
2023-05-17 [email protected] Remove deprecated fixTextFieldOutlineLabel (flutter/flutter#125893)
2023-05-17 [email protected] Roll Flutter Engine from 331c5769e291 to d97037077963 (3 revisions) (flutter/flutter#127053)
2023-05-17 [email protected] shard windows build tests 3 -> 4 (flutter/flutter#127057)
2023-05-17 [email protected] Roll Flutter Engine from 0ae3719d7043 to 331c5769e291 (1 revision) (flutter/flutter#127049)
2023-05-17 [email protected] Ignore unused_element_parameter (flutter/flutter#126926)
2023-05-17 [email protected] Roll Flutter Engine from 47fd496c6f8d to 0ae3719d7043 (2 revisions) (flutter/flutter#127043)
2023-05-17 [email protected] Roll Flutter Engine from 400a26ad8343 to 47fd496c6f8d (3 revisions) (flutter/flutter#127041)
2023-05-17 [email protected] [flutter_tools] unpin and roll camera_android (flutter/flutter#126945)
2023-05-17 [email protected] Handle null return from WillPopCallback (flutter/flutter#127039)
2023-05-17 [email protected] Roll Flutter Engine from 6048360c1837 to 400a26ad8343 (1 revision) (flutter/flutter#127036)
2023-05-17 [email protected] Remove unused (and untested) parameters from private _MasterDetailFlow (flutter/flutter#126935)
2023-05-17 [email protected] Autocomplete async examples (flutter/flutter#126283)
2023-05-17 [email protected] Expose callback that allows focus traversal customization (flutter/flutter#120235)
2023-05-17 [email protected] Roll Packages from b971830 to 5c69914 (5 revisions) (flutter/flutter#127034)
2023-05-17 [email protected] Roll Flutter Engine from ea3f27383acc to 6048360c1837 (2 revisions) (flutter/flutter#127032)
2023-05-17 [email protected] Add missing `Switch.onFocusChange` test (flutter/flutter#126685)
2023-05-17 [email protected] Remove 'url_launcher' from pubspec.yaml. (flutter/flutter#126939)
2023-05-17 [email protected] Roll Flutter Engine from 4b7c37532ea4 to ea3f27383acc (1 revision) (flutter/flutter#126994)
2023-05-17 [email protected] Roll Flutter Engine from 170b45bae571 to 4b7c37532ea4 (1 revision) (flutter/flutter#126985)
2023-05-17 [email protected] Roll Flutter Engine from 28b9dc993c30 to 170b45bae571 (1 revision) (flutter/flutter#126981)
2023-05-17 [email protected] Roll Flutter Engine from 87a03e107df7 to 28b9dc993c30 (1 revision) (flutter/flutter#126974)
2023-05-17 [email protected] Roll Flutter Engine from 1c775e34e2d5 to 87a03e107df7 (3 revisions) (flutter/flutter#126968)
2023-05-16 [email protected] Fix style issues (flutter/flutter#122586)
2023-05-16 [email protected] Manual roll Flutter Engine from fe2476743b59 to 1c775e34e2d5 (10 revisions) (flutter/flutter#126961)
2023-05-16 [email protected] Make SlottedMultiChildRenderObjectWidgetMixin a concrete class (flutter/flutter#126108)
2023-05-16 [email protected] Add ScrollMetrics.extentTotal for completeness (flutter/flutter#126607)
2023-05-16 [email protected] Fix drone_dimensions. (flutter/flutter#126953)
2023-05-16 [email protected] Add checkmark style to CupertinoRadio (flutter/flutter#126480)
2023-05-16 [email protected] Move `Mac_build_test flutter_gallery__transition_perf_e2e_ios` to prod (flutter/flutter#126941)
2023-05-16 [email protected] Fix DataTableThemeData.copyWith handling of dataRowHeight (flutter/flutter#126943)
2023-05-16 [email protected] Fix copyWith method of ActionIconThemeData (flutter/flutter#126763)
2023-05-16 [email protected] Revert "Roll Flutter Engine from fe2476743b59 to 5cf141f7c03c (2 revisions)" (flutter/flutter#126954)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
...
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 16, 2023
@engine-flutter-autoroll
f31dae2 Clip search view content during animation (flutter/flutter#12…
b4c9c74 
…6975)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023
@engine-flutter-autoroll
f31dae2 Clip search view content during animation (flutter/flutter#12…
fe811a5 
…6975)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023
@engine-flutter-autoroll
f31dae2 Clip search view content during animation (flutter/flutter#12…
4a5a1b5 
…6975)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023
@engine-flutter-autoroll
f31dae2 Clip search view content during animation (flutter/flutter#12…
bb23a96 
…6975)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bernaferrari bernaferrari bernaferrari left review comments

+1 more reviewer

@HansMuller HansMuller HansMuller approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

autosubmit Merge PR when tree becomes green via auto submit App f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[SearchAnchor] ListTile with trailing widget throws _AssertionError when closing SearchAnchor

3 participants

@QuncCccccc @bernaferrari @HansMuller