Skip to content

fix(GridLayout): support RTL direction for drop target positioning#9848

Open
miguelcalderon wants to merge 3 commits intoadobe:mainfrom
miguelcalderon:fix/grid-layout-rtl-drop-targets
Open

fix(GridLayout): support RTL direction for drop target positioning#9848
miguelcalderon wants to merge 3 commits intoadobe:mainfrom
miguelcalderon:fix/grid-layout-rtl-drop-targets

Conversation

@miguelcalderon
Copy link

@miguelcalderon miguelcalderon commented Mar 26, 2026

Description

GridLayout.getDropTargetFromPoint() computes before/after drop positions using x-coordinates but has no awareness of layout direction. In RTL locales the Virtualizer positions items using CSS right with the layout rect's x value as the right-offset, while pointer coordinates remain in visual (left-origin) space. This causes drop targets to be inverted — dragging an item to the right of another shows a before indicator instead of after.

Changes

  • Adds an optional direction property to GridLayoutOptions ('ltr' | 'rtl', default 'ltr').
  • When 'rtl', getDropTargetFromPoint flips the x-coordinate to match the layout coordinate system before computing drop positions.
  • Adds direction to shouldInvalidateLayoutOptions so layout updates when direction changes.

Usage

Consumers pass direction via useLayoutOptions() in a GridLayout subclass:

class LocaleAwareGridLayout extends GridLayout {
  useLayoutOptions() {
    const { direction } = useLocale();
    return { direction };
  }
}

✅ Pull Request Checklist:

  • Added/updated unit tests for this change.
  • Filled out test instructions.

📝 Test Instructions:

  1. Run the new GridLayout unit tests:
    yarn jest packages/react-stately/test/virtualizer/GridLayout.test.ts --no-coverage
  2. All 6 tests should pass — 2 LTR, 2 RTL, 1 empty layout, 1 single-column direction invariance.
  3. To verify the RTL tests depend on the fix, comment out the if (this.direction === 'rtl' ...) block in GridLayout.ts — the 2 RTL tests should fail while LTR tests continue to pass.

Notes

  • Single-column layouts are unaffected (they use y-axis only).
  • ListLayout is not affected since it is single-column.
  • This is a non-breaking change — the default is 'ltr' which preserves existing behavior.

🧢 Your Project:

Nutrient (formerly PSPDFKit) — baseline-ui component library

GridLayout.getDropTargetFromPoint() computes 'before'/'after' drop positions
using x-coordinates, but has no awareness of layout direction. In RTL locales,
the Virtualizer positions items using CSS 'right' with the layout rect's x
value as the right-offset, while pointer coordinates remain in visual
(left-origin) space. This mismatch causes drop targets to be inverted —
dragging an item to the right of another shows a 'before' indicator instead
of 'after'.

This commit adds an optional 'direction' property to GridLayoutOptions.
When set to 'rtl', getDropTargetFromPoint flips the x-coordinate to match
the layout coordinate system before computing drop positions.

Consumers can pass direction via useLayoutOptions() in a GridLayout subclass:

  class LocaleAwareGridLayout extends GridLayout {
    useLayoutOptions() {
      const { direction } = useLocale();
      return { direction };
    }
  }
@miguelcalderon
Copy link
Author

Sign CLA

Tests getDropTargetFromPoint with both LTR and RTL directions to verify
that drop targets (before/after) are computed correctly in multi-column
grids. Also tests that single-column layouts are unaffected by direction
and that empty layouts return a root drop target.
@nwidynski
Copy link
Contributor

nwidynski commented Mar 26, 2026

@LFDanLu This should probably also be done for the horizontal list layout. I'm also wondering whether

class LocaleAwareGridLayout extends GridLayout {
  useLayoutOptions() {
    const { direction } = useLocale();
    return { direction };
  }
}

should be done by default in both Grid and ListLayout, similar to table layout internals. Alternatively, we could also consider pushing all of this into useDrop, which would avoid layouts having to be locale-aware in the first place.

@LFDanLu
Copy link
Member

LFDanLu commented Mar 26, 2026

Thanks for catching this issue @miguelcalderon, I'll see about taking a closer look soon.

@nwidynski Yep this will definitely need to be added to the horizontal ListLayout as well. At a glance I think making the layouts automatically determine locale makes sense, but mind expanding on the useDrop idea you have? Feels like the layouts need to know how to calculate where to position the drop indicators and thus need to use locale no?

@nwidynski
Copy link
Contributor

nwidynski commented Mar 27, 2026

I mean, all this PR really does is to align the coordinate space of event targets (which is always left-right) with the one of our layout (which is locale-aware due to VirtualizerItem). I think it should be possible to do this alignment before passing x,y into getDropTargetFromPoint, thereby avoiding the layout itself having to know about RTL at all, similar to how it doesnt know about its layout infos being positioned using either right or left.

Since our non-virtualized scenario is locale-aware though (see ListDropTargetDelegate), we would probably have trouble differentiating between those - so likely preferable to auto-determine as discussed.

Instead of requiring consumers to subclass GridLayout and pass direction
manually, create a RAC wrapper (like TableLayout) that calls useLocale()
internally via useLayoutOptions(). The base layout in react-stately still
accepts direction as an explicit option for non-RAC consumers.
@github-actions github-actions bot added the RAC label Mar 27, 2026
@miguelcalderon
Copy link
Author

Good call on auto-determining direction — I've added a RAC wrapper for GridLayout that calls useLocale() internally via useLayoutOptions(), same pattern as TableLayout. Consumers no longer need to subclass.

The base layout in react-stately still accepts direction as an explicit option for non-RAC use cases.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants