Skip to content

Grid: next row overlaps taller items after data changes (stale minHeight picks the wrong tallest item) #2519

Description

@MarkusAbtion

Description

In a grid (numColumns > 1) with items of varying height, changing data can leave a row positioned using its shortest item's height, so the next row overlaps the taller items in that row. It never self-corrects, because nothing changes size afterwards.

The cause is in GridLayoutManager.processAndReturnTallestItemInRow. Layouts are kept per index across data changes, including minHeight. When the new item at an index happens to measure exactly its stale minHeight, the check layout.height > layout.minHeight skips it as a tallest candidate. A shorter item becomes tallestItem, the "uneven row" branch (maxHeight - tallestItem.height > 1) resets every minHeight to 0, but it still returns the shorter item. recomputeLayouts then starts the next row at tallestItem.y + tallestItem.height, which is too small.

Current behavior

After filtering or searching a list (same or different length, new items at old indices), some rows overlap the row above by the height difference, e.g. one line of a wrapped title. The tall cards' bottoms are covered by the next row's cards.

Expected behavior

The next row starts below the row's tallest measured item.

Reproduction

No Snack, sorry, but here is a deterministic repro that drives RVGridLayoutManagerImpl from the published 2.3.3 package directly. It needs a minimal react-native stub (only Platform and PixelRatio are touched by the layout code) and runs with tsx:

import { RVGridLayoutManagerImpl } from "@shopify/flash-list/dist/recyclerview/layout-managers/GridLayoutManager";

const manager = new RVGridLayoutManagerImpl({
  windowSize: { width: 300, height: 800 },
  maxColumns: 3,
  horizontal: false,
  optimizeItemArrangement: false,
  getItemType: () => "card",
  overrideItemLayout: () => {},
} as never);
const measure = (heights: number[]) =>
  manager.modifyLayout(
    heights.map((height, index) => ({ index, dimensions: { width: 100, height } })),
    heights.length,
  );
const rowY = (start: number) => manager.getLayout(start).y;

// 1. Row 1 has a tall middle item: its neighbours get minHeight 437.
measure([400, 400, 400, 408, 437, 408, 400, 400, 400]);

// 2. Data changes. The new row-1 items measure tall, short, tall. Cells 3 and 5
//    still carry minHeight 437, equal to their new natural height.
measure([400, 400, 400, 437, 408, 437, 400, 400, 400]);
console.log(rowY(6) - rowY(3)); // 408, but the row's tallest item is 437 → 29px overlap

// 3. Measuring the same sizes again doesn't fix it.
measure([400, 400, 400, 437, 408, 437, 400, 400, 400]);
console.log(rowY(6) - rowY(3)); // still 408

Output on 2.3.3:

BUG: row 2 starts 408px below row 1, but row 1's tallest item is 437px, so it overlaps by 29px
after another identical measurement: row gap 408px

In a real app: a 3-column web grid of cards with titles that wrap to one or two lines, plus a search input. Type a search, scroll, clear it, and some rows overlap. It reproduced every time for us in Chrome and Firefox.

Suggested fix

When the uneven-row branch resets the min heights, return the tallest measured item instead of the stale pick. With this change, the repro gives a 437px row gap and stays stable:

             if (maxHeight - tallestItem.height > 1) {
                 targetHeight = 0;
                 this.requiresRepaint = true;
+                for (let j = startIndex; j <= endIndex && j < this.layouts.length; j++) {
+                    if (this.layouts[j].height > tallestItem.height) tallestItem = this.layouts[j];
+                }
             }

Clearing minHeight for indices whose data changed would also work, but the above is local to the grid manager.

Platform

  • iOS
  • Android
  • Web (if applicable)

Observed on web. The layout manager is shared, so iOS and Android are probably affected too, but I haven't verified that.

Environment

Versions
@shopify/flash-list 2.0.2 in the app; the repro above runs against 2.3.3
react 19.2.0
react-native 0.83.6
react-native-web 0.21
expo ~55.0.26

FlashList version: 2.3.3 (repro), 2.0.2 (app)

Additional context

Workaround: call listRef.current?.clearLayoutCacheOnUpdate() during the render in which data changes. That drops the stale layouts, so no minHeight carries over.

Possibly related: #1797 (also overlapping items, but fixed for the initialScrollIndex case in #2133).

Checklist

  • I've searched existing issues and couldn't find a duplicate
  • I've provided a minimal reproduction (a script against the layout manager, not a Snack)
  • I'm using the latest version of @shopify/flash-list (repro run against 2.3.3)
  • I've included all required information above

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions