Repository navigation
Exclude content behind a pinned sticky header from viewability - #59200
Open
ramya-2196 wants to merge 2 commits into
Open
ramya-2196 wants to merge 2 commits into
ramya-2196 wants to merge 2 commits into
Conversation
Summary: ### What `VirtualizedList` now records the position of a sticky header cell from its sticky wrapper, instead of from the cell's own layout. This affects `FlatList` with `stickyHeaderIndices` and `SectionList` with sticky section headers. JS only; no new props or public API. - `VirtualizedList.js`: sticky cells get an `onLayout` callback, and that layout takes priority over the cell's own `onCellLayout` measurement. - `VirtualizedListCellRenderer.js`: adds the optional `onLayout` prop type. - `VirtualizedList.macos.js`, `VirtualizedList.windows.js`, `VirtualizedListCellRenderer.macos.js`: the same change. ### Why Fixes part of react#37439. With sticky headers, `onViewableItemsChanged` never reports the next sticky header or any item after it, even when they are fully on screen. `ScrollView` wraps every sticky child in `ScrollViewStickyHeader`. The cell's inner `View` therefore measures its layout relative to that wrapper, so a sticky cell's offset is always about 0 instead of its position in the list. Viewability scans cells in order and stops at the first cell that is not visible after a visible one. A sticky header stored at offset 0 looks scrolled off the top, so the scan stops there and drops it and everything after it. Example: groups of one 50px header and five 80px rows, scrolled to y=254 with header A pinned. Header B (at 450) and B1-B4 are on screen, but only `A4, A5` are reported. ### How `ScrollViewStickyHeader` already calls the child element's `onLayout` with the wrapper's layout, which is relative to the list content. Since cells moved their layout callback to `onCellLayout`, nothing received it. `VirtualizedList` now passes `onLayout` to sticky cells and records that layout. The cell's own measurement is still recorded until the wrapper reports, so a custom `StickyHeaderComponent` that does not forward `onLayout` behaves exactly as before. Once the wrapper has reported, later inner measurements for that cell are ignored. The flag is cleared when the cell unmounts or stops being sticky. Alternative considered: changing `ScrollViewStickyHeader` to forward `onCellLayout` as well. Rejected because it would teach `ScrollView` about a list-specific prop. ### For Reviewers - This does not fix the other half of react#37439: items hidden behind the pinned header are still reported as viewable. That needs a separate change in `ViewabilityHelper`. - Sticky cell offsets also feed render windowing, `scrollToIndex`, and fill-rate tracking, so those now use the correct positions too. Changelog: [General][Fixed] - Fix `onViewableItemsChanged` skipping the next sticky header and the items after it in `FlatList` and `SectionList` Differential Revision: D126073361
Summary: ### What `onViewableItemsChanged` no longer reports items hidden behind a pinned sticky header, and reports the pinned header itself while it is on screen. This applies to `FlatList` and `VirtualizedList` with `stickyHeaderIndices` (including a sticky `ListHeaderComponent`) and to `SectionList` with sticky section headers. JS only; no new props or public API. - `VirtualizeUtils.js`: new `findPinnedStickyHeader` finds the pinned header and how much of the viewport it covers. - `ViewabilityHelper.js`: `computeViewableItems` and `onUpdate` take an optional pinned header. Content behind it does not count as visible, and the header is checked where it is drawn. - `VirtualizedList.js`: works out the pinned header from the sticky header layouts and passes it on. Same change in `VirtualizedList.macos.js` and `VirtualizedList.windows.js`. ### Why Fixes the other half of react#37439. A pinned sticky header covers the start of the viewport, but viewability treated the whole viewport as visible: - items fully or partly behind the header were reported as viewable - the pinned header itself dropped out as soon as its original position scrolled away, even though it is still on screen Example: a 120px sticky `ListHeaderComponent` over 80px rows, scrolled to 400. Only items 5-10 are visible, but item 4, which is completely behind the header, was reported too. ### How `findPinnedStickyHeader` follows the positioning in `ScrollViewStickyHeader`: the pinned header is the last sticky header whose start the scroll offset has reached, and it covers its full length until the next sticky header pushes it out. The helper does not allocate per header. `ViewabilityHelper` treats the viewport as starting where the pinned header ends. The view-area percentage still uses the whole viewport, since the header is part of it. When the pinned header is a list item, it is skipped in the normal scan and checked at its drawn position. The viewport is unchanged in cases where we cannot tell where the header is drawn: - horizontal lists (`ScrollView` does not support sticky headers there) - headers that stick to the bottom (`invertStickyHeaders`, which defaults to `inverted`) - `stickyHeaderHiddenOnScroll` - a custom `StickyHeaderComponent` - lists nested in a list with the same orientation ### For Reviewers - This builds on the previous diff, which records the real position of sticky header cells. The next header's position decides when the pinned one is pushed out. - The pinned header is now reported as viewable for as long as it is pinned. That is a visible change for apps that track section headers. - Inverted lists keep today's behaviour. Sticky headers there are drawn with a different formula that has open issues of its own (react#18945), so it seemed wrong to build viewability on it. - `ListHeaderComponent` is assumed to start at offset 0. Its own layout is measured inside the sticky wrapper, so its reported position is not reliable. Content container padding only delays pinning, and nothing else is under the header in that range. Changelog: [General][Fixed] - Fix `onViewableItemsChanged` reporting items hidden behind a pinned sticky header, and missing the pinned header itself, in `FlatList` and `SectionList` Differential Revision: D126079197
meta-cla
Bot
added
the
CLA Signed
|
@ramya-2196 has exported this pull request. If you are a Meta employee, you can view the originating Diff in D126079197. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
What
onViewableItemsChangedno longer reports items hidden behind a pinned sticky header, and reports the pinned header itself while it is on screen. This applies toFlatListandVirtualizedListwithstickyHeaderIndices(including a stickyListHeaderComponent) and toSectionListwith sticky section headers. JS only; no new props or public API.VirtualizeUtils.js: newfindPinnedStickyHeaderfinds the pinned header and how much of the viewport it covers.ViewabilityHelper.js:computeViewableItemsandonUpdatetake an optional pinned header. Content behind it does not count as visible, and the header is checked where it is drawn.VirtualizedList.js: works out the pinned header from the sticky header layouts and passes it on. Same change inVirtualizedList.macos.jsandVirtualizedList.windows.js.Why
Fixes the other half of #37439. A pinned sticky header covers the start of the viewport, but viewability treated the whole viewport as visible:
Example: a 120px sticky
ListHeaderComponentover 80px rows, scrolled to 400. Only items 5-10 are visible, but item 4, which is completely behind the header, was reported too.How
findPinnedStickyHeaderfollows the positioning inScrollViewStickyHeader: the pinned header is the last sticky header whose start the scroll offset has reached, and it covers its full length until the next sticky header pushes it out. The helper does not allocate per header.ViewabilityHelpertreats the viewport as starting where the pinned header ends. The view-area percentage still uses the whole viewport, since the header is part of it. When the pinned header is a list item, it is skipped in the normal scan and checked at its drawn position.The viewport is unchanged in cases where we cannot tell where the header is drawn:
ScrollViewdoes not support sticky headers there)invertStickyHeaders, which defaults toinverted)stickyHeaderHiddenOnScrollStickyHeaderComponentFor Reviewers
ListHeaderComponentis assumed to start at offset 0. Its own layout is measured inside the sticky wrapper, so its reported position is not reliable. Content container padding only delays pinning, and nothing else is under the header in that range.Changelog:
[General][Fixed] - Fix
onViewableItemsChangedreporting items hidden behind a pinned sticky header, and missing the pinned header itself, inFlatListandSectionListDifferential Revision: D126079197