Skip to content

Add clipBehavior to ListView, GridView, PageView #63147

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
liyuqian merged 3 commits into from
Aug 13, 2020
Merged

Add clipBehavior to ListView, GridView, PageView #63147

liyuqian merged 3 commits into from
Aug 13, 2020

Conversation

@liyuqian
Copy link
Contributor

@liyuqian liyuqian commented Aug 7, 2020

These widgets are missing from
#59364

With this change, developers can use clipBehavior for
#59424

@liyuqian liyuqian requested a review from goderbauer August 7, 2020 00:03
@flutter-dashboard flutter-dashboard bot added the framework flutter/packages/flutter repository. See also f: labels. label Aug 7, 2020
@liyuqian liyuqian mentioned this pull request Aug 7, 2020
Closed
xu-baolin
xu-baolin reviewed Aug 7, 2020
View reviewed changes
packages/flutter/lib/src/widgets/scroll_view.dart Outdated
Copy link
Member

@xu-baolin xu-baolin Aug 7, 2020

Choose a reason for hiding this comment

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

Is ShrinkWrappingViewport need clipBehavior?

Copy link
Contributor Author

@liyuqian liyuqian Aug 7, 2020

Choose a reason for hiding this comment

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

ShrinkWrappingViewport seems to already have clipBehavior:

flutter/packages/flutter/lib/src/widgets/viewport.dart

Line 313 in 77b4505

final Clip clipBehavior;

Copy link
Member

@xu-baolin xu-baolin Aug 8, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I mean line 314. This function has two return branches, :)

Copy link
Contributor Author

@liyuqian liyuqian Aug 10, 2020

Choose a reason for hiding this comment

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

Ah yes, nice catch! Fixed now.

@xu-baolin
Copy link
Member

xu-baolin commented Aug 7, 2020

I searched for the usage of pushClipRect globally, I am not sure are those widgets OverLay AnimatedSize AndroidView need too.

@goderbauer
Copy link
Member

goderbauer commented Aug 7, 2020

Looks like the analyzer is not happy about this.

@liyuqian liyuqian mentioned this pull request Aug 7, 2020
Closed
@liyuqian
Copy link
Contributor Author

liyuqian commented Aug 7, 2020

I searched for the usage of pushClipRect globally, I am not sure are those widgets OverLay AnimatedSize AndroidView need too.

Yes, I should add it to them. Created #63246 to track the progress. I'll do it in another PR.

@Piinks Piinks added the f: scrolling Viewports, list views, slivers, etc. label Aug 10, 2020
@liyuqian liyuqian mentioned this pull request Aug 11, 2020
Merged
4 tasks
goderbauer
goderbauer approved these changes Aug 12, 2020
View reviewed changes
Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

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

LGTM

@liyuqian
Add clipBehavior to ListView, GridView, PageView
df68412 
These widgets are missing from
flutter#59364

With this change, developers can use clipBehavior for
flutter#59424
@liyuqian
Fix analyzer issue
83e58aa
@liyuqian
Add missing clipBehavior
5fccb42
@liyuqian liyuqian force-pushed the more_clip_viewport branch from 1916d21 to 5fccb42 Compare August 12, 2020 19:10
@fluttergithubbot
Copy link
Contributor

fluttergithubbot commented Aug 12, 2020

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite Linux hostonly_devicelab_tests has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Linux tool_tests has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Linux web_tests has failed. Please fix the issues identified (or deflake) before re-applying this label.

@liyuqian liyuqian merged commit 3ff76f4 into flutter:master Aug 13, 2020
@liyuqian liyuqian deleted the more_clip_viewport branch August 14, 2020 22:18
mingwandroid pushed a commit to mingwandroid/flutter that referenced this pull request Sep 6, 2020
@liyuqian @mingwandroid
Add clipBehavior to ListView, GridView, PageView (flutter#63147)
6a25b9f 
These widgets are missing from
flutter#59364

With this change, developers can use clipBehavior for
flutter#59424
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@xu-baolin xu-baolin xu-baolin left review comments

@goderbauer goderbauer goderbauer approved these changes

Assignees

No one assigned

Labels

f: scrolling Viewports, list views, slivers, etc. 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.

6 participants

@liyuqian @xu-baolin @goderbauer @fluttergithubbot @googlebot @Piinks