Skip to content

Add clipBehavior to ListView, GridView, PageView - #63147

Merged
liyuqian merged 3 commits into
flutter:masterfrom
liyuqian:more_clip_viewport
Aug 13, 2020
Merged

Add clipBehavior to ListView, GridView, PageView#63147
liyuqian merged 3 commits into
flutter:masterfrom
liyuqian:more_clip_viewport

Conversation

@liyuqian

@liyuqian liyuqian commented Aug 7, 2020

Copy link
Copy Markdown
Contributor

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
Comment thread packages/flutter/lib/src/widgets/scroll_view.dart Outdated

@xu-baolin xu-baolin Aug 7, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is ShrinkWrappingViewport need clipBehavior?

@liyuqian liyuqian Aug 7, 2020

Copy link
Copy Markdown
Contributor Author

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;

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

Copy link
Copy Markdown
Member

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, :)

@liyuqian liyuqian Aug 10, 2020

Copy link
Copy Markdown
Contributor Author

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

xu-baolin commented Aug 7, 2020

Copy link
Copy Markdown
Member

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

@goderbauer

goderbauer commented Aug 7, 2020

Copy link
Copy Markdown
Member

Looks like the analyzer is not happy about this.

@liyuqian liyuqian mentioned this pull request Aug 7, 2020
Closed
@liyuqian

liyuqian commented Aug 7, 2020

Copy link
Copy Markdown
Contributor Author

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 done
goderbauer
goderbauer approved these changes Aug 12, 2020
View reviewed changes

@goderbauer goderbauer left a comment

Copy link
Copy Markdown
Member

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

fluttergithubbot commented Aug 12, 2020

Copy link
Copy Markdown
Contributor

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