Refactor scrolling code to prepare for nested scrolling #9575
Conversation
|
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the |
|
CLAs look good, thanks! |
abarth
force-pushed
the
nested3
branch
2 times, most recently
from
17613cc to
1285f71
Compare
There was a problem hiding this comment.
I have mixed feelings about the existence of this interface since we just got rid of it last week, but I'm willing to see where it takes us.
There was a problem hiding this comment.
Yeah, at least it has a clearer purpose now.
There was a problem hiding this comment.
there's a bunch of mentions of ScrollIndependentPosition and ScrollPosition in this file
There was a problem hiding this comment.
these docs are out of date. search for ScrollIndependentPosition and ScrollPositionReadInterface in particular
There was a problem hiding this comment.
maybe && initialPixels != null for clarity (not that it really makes a difference in practice)
There was a problem hiding this comment.
yikes these docs are out of date. ScrollActivityManager died long ago.
There was a problem hiding this comment.
Yeah, the docs that say who calls who are really likely to be out of date.
There was a problem hiding this comment.
ditto. i think this text appears a few times in this file.
There was a problem hiding this comment.
I think it would be more consistent to call this "didOverscrollBy"
|
Reviewing this was a bit of a surreal experience. |
This patch reworks some of the guts of scrolling to make it easier to implement nested scrolling effects. The actually nested scrolling effect will be included in a later patch.
3 participants
This patch reworks some of the guts of scrolling to make it easier to
implement nested scrolling effects. The actually nested scrolling effect
will be included in a later patch.