Skip to content

Conversation

xu-baolin
Copy link
Member

@xu-baolin xu-baolin commented Jan 10, 2022

Fixes #96024
Fixes #55170
Fixes #25861

The RenderViewport keep a child RO by 'center', it will be updated in [_ViewportElement.mount] or [Update].
But sometimes the child RO will be changed without [_ViewportElement] change when only the subtree of Viewport is dirty.
In that case, we should also update the center RO.

Similar to the root cause of this issue, it has a similar solution.

@flutter-dashboard flutter-dashboard bot added f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels. labels Jan 10, 2022
@xu-baolin xu-baolin requested review from goderbauer and Hixie January 10, 2022 02:53
}
}

// `moveRenderObjectChild` only happens during [update] stage.
Copy link
Member

Choose a reason for hiding this comment

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

Seems an invalid comment that's indicating the wrong method and didn't start with ///?

Copy link
Member Author

Choose a reason for hiding this comment

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

Just comment that why not override moveRenderObjectChild

Copy link
Member

Choose a reason for hiding this comment

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

Could it say it? It wasn't immediately clear for me either
For example

// We don't override `moveRenderObjectChild` here, because it only happens
// during [update] stage.

Copy link
Member

Choose a reason for hiding this comment

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

Or: Do override moveRenderObjectChild, which just calls super and has an assert that _doingMountOrUpdate is true to show & proof that the comment is true.

Copy link
Member Author

Choose a reason for hiding this comment

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

Or: Do override moveRenderObjectChild, which just calls super and has an assert that _doingMountOrUpdate is true to show & proof that the comment is true.

Done and add a test to test it.

}
}

// `moveRenderObjectChild` only happens during [update] stage.
Copy link
Member

Choose a reason for hiding this comment

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

Or: Do override moveRenderObjectChild, which just calls super and has an assert that _doingMountOrUpdate is true to show & proof that the comment is true.

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

@fluttergithubbot fluttergithubbot merged commit 9e9a728 into flutter:master Jan 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Jan 18, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Jan 18, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Jan 18, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Feb 4, 2022
clocksmith pushed a commit to clocksmith/flutter that referenced this pull request Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
5 participants