You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
sticky_header: Cut wrong use of calculatePaintOffset
This commit is NFC for the actual app, or at least nearly so.
This call to calculatePaintOffset was conceptually wrong: it's
asking how much of this sliver's region to be painted is within the
range of scroll offsets from zero to headerExtent. That'd be a
pertinent question if we were locating something in that range of
scroll offsets... but that range is not at all where the header
goes, unless by happenstance. So the value returned is meaningless.
One reason this buggy line has survived is that the bug is largely
latent -- we can remove it entirely, as in this commit, and get
exactly the same behavior except in odd circumstances.
Specifically:
* This paintedHeaderSize variable can only have any effect
by being greater than childExtent.
* In this case childExtent is smaller than headerExtent, too.
* The main way that childExtent can be so small is if
remainingPaintExtent, which constrains it, is equally small.
* But calculatePaintOffset constrains its result, aka
paintedHeaderSize, to at most remainingPaintExtent too,
so then paintedHeaderSize still won't exceed childExtent.
I say "main way" because the alternative is for the child to run out
of content before finding as much as headerExtent of content to
show. That could happen if the list just has less than that much
content; but that means the header's own item is smaller than the
header, which is a case that sticky_header doesn't really support
well anyway and we don't have in the app. Otherwise, this would
have to mean that some of the content was scrolled out of the
viewport and then the child ran out of content before filling its
allotted remainingPaintExtent of the viewport (and indeed before
even reaching a headerExtent amount of content). This is actually
not quite impossible, if the scrollable permits overscroll... but
making it happen would require piling edge case upon edge case.
Anyway, this call never made sense, so remove it.
The resulting code in this headerExtent > childExtent case still
isn't right. Removing this wrong logic helps clear the ground for
fixing that.
0 commit comments