Repository navigation
fix(virtual-list): keep the cross size when layout effects re-run without a data change - #148
Merged
Merged
Conversation
…hout a data change
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
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.
VirtualList reset its cross size every time the layout effect fired, not only when
dataorextraDataactually changed. In the Plex app Home sits under a Suspense based Freeze, so coming Back from a preplay screen re-runs every layout effect with the same data. Each inner row list dropped its cross size to the 200 default for one pass and measured itself short (poster rows 928 to 672, 892 to 636). Visible rows grow back, but a row that left the draw window first keeps 672, so everything below it sits 136px too high and the hero seeds shift. Visible layout shift after Back, plus hero text overlapping the row title after Down/Up.Fix: the effect now remembers the
data/extraDatait last reset for and bails when both are unchanged. Moved the grow-only max, the reset and the generation into a smalluseContentCrosshook so it can be tested on its own. Side effect: no more reset (and extra render) on mount, which was a no-op anyway since the max starts at 0.The test renders the hook with a bare react-reconciler host (added as a devDependency, same version react-lightning uses) and hides/reveals it with
Activity, which tears down and re-runs layout effects the same way the Freeze does. Fails without the guard, passes with it. Data and extraData changes still reset.Verified in the app by swapping the change in at runtime: row and hero positions are identical before and after the Back round trip. Not checked on a real TV yet.
Left alone:
handleContentCrossLayoutis still a grow-only max per dataset, so when cells recycle between item types with different cross sizes it can keep a stale bigger value. Tracking cross per cell/key would fix that, but it changes when rows are allowed to shrink while scrolling, so I think it deserves its own PR.