Open linked message at top - 3 #95045
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,166 @@ | ||
| diff --git a/node_modules/@shopify/flash-list/dist/FlashListProps.d.ts b/node_modules/@shopify/flash-list/dist/FlashListProps.d.ts | ||
| index fa786bf..586014c 100644 | ||
| --- a/node_modules/@shopify/flash-list/dist/FlashListProps.d.ts | ||
| +++ b/node_modules/@shopify/flash-list/dist/FlashListProps.d.ts | ||
| @@ -127,10 +127,12 @@ export interface FlashListProps<TItem> extends Omit<ScrollViewProps, "maintainVi | ||
| /** | ||
| * Additional configuration for initialScrollIndex. | ||
| * Use viewOffset to apply an offset to the initial scroll position as defined by initialScrollIndex. | ||
| + * Use viewPosition to position the item within the viewport (0 = start, 0.5 = center, 1 = end), mirroring scrollToIndex. | ||
| * Ignored if initialScrollIndex is not set. | ||
| */ | ||
| initialScrollIndexParams?: { | ||
| viewOffset?: number; | ||
| + viewPosition?: number; | ||
| } | null | undefined; | ||
| /** | ||
| * Used to extract a unique key for a given item at the specified index. | ||
| diff --git a/node_modules/@shopify/flash-list/dist/recyclerview/RecyclerViewManager.js b/node_modules/@shopify/flash-list/dist/recyclerview/RecyclerViewManager.js | ||
| index 3b69234..a9474c1 100644 | ||
| --- a/node_modules/@shopify/flash-list/dist/recyclerview/RecyclerViewManager.js | ||
| +++ b/node_modules/@shopify/flash-list/dist/recyclerview/RecyclerViewManager.js | ||
| @@ -294,11 +294,26 @@ export class RecyclerViewManager { | ||
| // re-estimate unmeasured items with an updated average height, changing | ||
| // the target item's position. Reading before recompute would capture a | ||
| // stale offset, causing the wrong items to be rendered. | ||
| - this.layoutManager.recomputeLayouts(0, initialScrollIndex); | ||
| + this.layoutManager.recomputeLayouts(0, this.getDataLength() - 1); | ||
| const initialItemLayout = this.layoutManager.getLayout(initialScrollIndex); | ||
| - const initialItemOffset = this.propsRef.horizontal | ||
| + let initialItemOffset = this.propsRef.horizontal | ||
| ? initialItemLayout.x | ||
| : initialItemLayout.y; | ||
| + // Anchor the initial render window according to initialScrollIndexParams.viewPosition so the | ||
| + // first painted frame already shows the target item at the requested position in the viewport. | ||
| + const initialScrollIndexParams = this.propsRef.initialScrollIndexParams; | ||
| + const viewPosition = initialScrollIndexParams === null || initialScrollIndexParams === void 0 ? void 0 : initialScrollIndexParams.viewPosition; | ||
|
Comment on lines
+35
to
+36
There was a problem hiding this comment.
This initial-render adjustment reads only Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. I'm not sure we need this change here.
This asymmetry is also the pre-existing upstream design: before this patch, |
||
| + if (viewPosition !== undefined) { | ||
| + const windowSize = this.propsRef.horizontal | ||
| + ? this.getWindowSize().width | ||
| + : this.getWindowSize().height; | ||
| + const itemSize = this.propsRef.horizontal | ||
| + ? initialItemLayout.width | ||
| + : initialItemLayout.height; | ||
| + if (windowSize > 0) { | ||
| + initialItemOffset = Math.max(0, initialItemOffset - (windowSize - itemSize) * viewPosition); | ||
| + } | ||
| + } | ||
| this.engagedIndicesTracker.scrollOffset = initialItemOffset; | ||
| } | ||
| else { | ||
| @@ -317,8 +332,20 @@ export class RecyclerViewManager { | ||
| this.applyInitialScrollAdjustment(); | ||
| const visibleIndices = this.computeVisibleIndices(); | ||
| // console.log("---------> visibleIndices", visibleIndices); | ||
| - this.hasRenderedProgressively = visibleIndices.every((index) => layoutManager.getLayout(index).isHeightMeasured && | ||
| - layoutManager.getLayout(index).isWidthMeasured); | ||
| + const isFullyMeasured = (index) => layoutManager.getLayout(index).isHeightMeasured && | ||
| + layoutManager.getLayout(index).isWidthMeasured; | ||
| + // With an explicit initialScrollIndex, also wait for the drawDistance buffer to be measured before | ||
| + // completing the first layout, so estimate-driven layout shifts converge before anything is on screen. | ||
| + let targetIndices = visibleIndices; | ||
| + if (this.propsRef.initialScrollIndex !== undefined && visibleIndices.length > 0 && visibleIndices.every(isFullyMeasured)) { | ||
| + const windowSize = this.propsRef.horizontal ? this.getWindowSize().width : this.getWindowSize().height; | ||
| + const viewportStart = this.engagedIndicesTracker.scrollOffset; | ||
| + // Cover the worst-case one-sided buffer the engaged tracker can mount after | ||
| + // first layout (totalBuffer * largeMultiplier in the scroll direction). | ||
| + const bufferDistance = this.engagedIndicesTracker.drawDistance * 2 * this.engagedIndicesTracker.largeMultiplier; | ||
| + targetIndices = layoutManager.getVisibleLayouts(Math.max(0, viewportStart - bufferDistance), viewportStart + windowSize + bufferDistance); | ||
| + } | ||
| + this.hasRenderedProgressively = targetIndices.every(isFullyMeasured); | ||
| if (this.hasRenderedProgressively) { | ||
| this.isFirstLayoutComplete = true; | ||
| } | ||
| @@ -327,9 +354,13 @@ export class RecyclerViewManager { | ||
| // If everything is measured then render stack will be in sync. The buffer items will get rendered in the next update | ||
| // triggered by the useOnLoad hook. | ||
| !this.hasRenderedProgressively && | ||
| - this.updateRenderStack( | ||
| - // pick first n indices from visible ones based on batch size | ||
| - visibleIndices.slice(0, Math.min(visibleIndices.length, this.getRenderStack().size + batchSize))); | ||
| + this.updateRenderStack(targetIndices === visibleIndices | ||
| + ? // pick first n indices from visible ones based on batch size | ||
| + visibleIndices.slice(0, Math.min(visibleIndices.length, this.getRenderStack().size + batchSize)) | ||
| + : // buffer phase: visible items are already measured, mount the whole | ||
| + // buffer window at once. Same single-commit cost the engaged tracker | ||
| + // would pay post-paint, just moved to where nothing is visible yet. | ||
| + targetIndices); | ||
| } | ||
| } | ||
| getItemType(index) { | ||
| diff --git a/node_modules/@shopify/flash-list/dist/recyclerview/hooks/useRecyclerViewController.js b/node_modules/@shopify/flash-list/dist/recyclerview/hooks/useRecyclerViewController.js | ||
| index 18e59ce..40bdddb 100644 | ||
| --- a/node_modules/@shopify/flash-list/dist/recyclerview/hooks/useRecyclerViewController.js | ||
| +++ b/node_modules/@shopify/flash-list/dist/recyclerview/hooks/useRecyclerViewController.js | ||
| @@ -25,6 +25,10 @@ export function useRecyclerViewController(recyclerViewManager, ref, scrollViewRe | ||
| const isUnmounted = useUnmountFlag(); | ||
| const [_, setRenderId] = useState(0); | ||
| const pauseOffsetCorrection = useRef(false); | ||
| + // Latest offset computed by applyInitialScrollIndex. The deferred (setTimeout) re-scroll reads this at | ||
| + // fire-time instead of the value it closed over, so a stale timeout scheduled by an earlier commit can't | ||
| + // snap back to an outdated offset after a newer commit. | ||
| + const latestInitialScrollOffsetRef = useRef(0); | ||
| // True while a `scrollToIndex` / `scrollToOffset` smooth scroll is in | ||
| // flight. Cleared exactly once on `isMomentumEnd` via | ||
| // `notifyProgrammaticScrollSettled`. | ||
| @@ -566,18 +570,55 @@ export function useRecyclerViewController(recyclerViewManager, ref, scrollViewRe | ||
| }, 500); | ||
| pauseOffsetCorrection.current = true; | ||
| const additionalOffset = (_c = initialScrollIndexParams === null || initialScrollIndexParams === void 0 ? void 0 : initialScrollIndexParams.viewOffset) !== null && _c !== void 0 ? _c : 0; | ||
| - const offset = horizontal | ||
| - ? recyclerViewManager.getLayout(initialScrollIndex).x + additionalOffset | ||
| - : recyclerViewManager.getLayout(initialScrollIndex).y + | ||
| - additionalOffset; | ||
| + const initialItemLayout = recyclerViewManager.getLayout(initialScrollIndex); | ||
| + let offset = (horizontal ? initialItemLayout.x : initialItemLayout.y) + | ||
| + additionalOffset; | ||
| + // Position the target item within the viewport (0 = start, 0.5 = center, 1 = end), mirroring scrollToIndex. | ||
| + const viewPosition = initialScrollIndexParams === null || initialScrollIndexParams === void 0 ? void 0 : initialScrollIndexParams.viewPosition; | ||
| + if (viewPosition !== undefined) { | ||
| + const containerSize = horizontal | ||
| + ? recyclerViewManager.getWindowSize().width | ||
| + : recyclerViewManager.getWindowSize().height; | ||
| + const itemSize = horizontal | ||
| + ? initialItemLayout.width | ||
| + : initialItemLayout.height; | ||
| + if (containerSize > 0) { | ||
| + offset = Math.max(0, offset - (containerSize - itemSize) * viewPosition); | ||
| + } | ||
| + } | ||
| + // Make it clear there are more items to scroll to underneath the bottom edge. | ||
| + // If the bottom item is (essentially) fully visible against the bottom edge AND there | ||
| + // is an item underneath it, nudge the bottom edge up so CROP_OFFSET px of the current | ||
| + // bottom item gets cropped, signalling that more content can be scrolled into view. | ||
| + if (viewPosition !== undefined && !horizontal && recyclerViewManager.props.inverted && offset > 0) { | ||
| + const CROP_OFFSET = 10; | ||
| + let bottomIndex = -1; | ||
| + for (let i = initialScrollIndex; i >= 0; i--) { | ||
| + if (recyclerViewManager.getLayout(i).y <= offset) { | ||
| + bottomIndex = i; | ||
| + break; | ||
| + } | ||
| + } | ||
| + if (bottomIndex > 0) { | ||
| + const bottomItemLayout = recyclerViewManager.getLayout(bottomIndex); | ||
| + const hiddenPortion = offset - bottomItemLayout.y; | ||
| + // 8px is bottom padding of every item | ||
| + if (hiddenPortion <= 8) { | ||
| + // Crop the current bottom item rather than letting it sit flush against the edge. | ||
| + offset = bottomItemLayout.y + CROP_OFFSET; | ||
| + } | ||
| + } | ||
| + } | ||
| + latestInitialScrollOffsetRef.current = offset; | ||
| handlerMethods.scrollToOffset({ | ||
| offset, | ||
| animated: false, | ||
| skipFirstItemOffset: false, | ||
| }); | ||
| + | ||
| setTimeout(() => { | ||
| handlerMethods.scrollToOffset({ | ||
| - offset, | ||
| + offset: latestInitialScrollOffsetRef.current, | ||
| animated: false, | ||
| skipFirstItemOffset: false, | ||
| }); | ||
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
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing this to
this.getDataLength() - 1makes every FlashList withinitialScrollIndexrecompute layouts for the entire dataset during mount, not only report links. I checked existing callers such assrc/components/LHNOptionsList/LHNOptionsList.tsx, where web restores the saved LHN scroll withinitialScrollIndex; on accounts with large report lists this turns reopening the LHN into an O(n) layout pass before first paint even when the saved index is near the top. Limit the widened range to the linked-message/viewPosition path or to the indices needed to calculate the target.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change intentionally matches the upstream fix Shopify/flash-list#2318, you can check for more details there. Narrowing the range would also bring back the bug this fixes: stopping at
initialScrollIndexleaves the layout table non-monotonic, so the binary search resolves to the wrong item.