Add layout and orientation props to RAC ListBox #4669

devongovett · 2023-06-15T02:25:29Z

This adds two new props to the RAC ListBox, as well as the ListKeyboardDelegate and ListDropTargetDelegate: layout, and orientation. This enables creating lists that are arranged in a horizontal stack, or a vertically or horizontally scrolling 2d grid (e.g. cards). Selection and keyboard navigation already worked, but useDroppableCollection did require some work to get left/right arrows working (as mentioned in #4506).

There is some complexity to handling all of these combinations (along with RTL) in the same delegates. I tried making separate GridKeyboardDelegate and GridDropTargetDelegate implementations but found I had to duplicate a lot of code vs just adding options. Open to changing it back if people think it's weird to have it all in the list implementations.

devongovett · 2023-06-15T02:27:22Z

packages/@react-aria/selection/src/useSelectableCollection.ts

-        if (modality === 'keyboard') {
-          scrollIntoViewport(element, {containingElement: ref.current});
-        }
+        scrollIntoViewport(element, {containingElement: ref.current});
      }


The changes in this file ensure we don't scroll an item into view when clicking with a mouse, only when navigating via a keyboard. This was already the case for virtualized collections, but I guess we never made the changes here as well.

devongovett · 2023-06-15T02:27:50Z

packages/react-aria-components/docs/ListBox.mdx

-    [slot=drag][data-focus-visible] {
-      box-shadow: 0 0 0 2px var(--highlight-foreground);
-    }
-  }


Unused CSS (copied from GridList). There is no drag affordance in ListBox.

devongovett · 2023-06-15T02:28:06Z

packages/react-aria-components/docs/Select.mdx

@@ -518,7 +518,7 @@ The `--trigger-width` CSS custom property will be set on the popover, which you

 ```css render=false
 .react-aria-Popover {
-  width: var(--button-width);
+  width: var(--trigger-width);


Random small bug (matching text in paragraph above).

rspbot · 2023-06-15T02:31:04Z

Build successful! 🎉

snowystinger

I think we've addressed this before, but I can currently mouse down on an item in the new ListBox examples in the docs, and if I drag off and back on to an item, it will toggle the selected state each time my mouse re-enters the element. probably not actually related to your changes.
https://reactspectrum.blob.core.windows.net/reactspectrum/ac065e7221226cf2ff5c664d7256543a9308d855/docs/react-aria/ListBox.html#column-layout

On the same example, page up behavior is a bit strange, it goes to the last item in the column instead of the first item in the column.

packages/@react-aria/dnd/src/ListDropTargetDelegate.ts

snowystinger · 2023-06-15T20:16:12Z

packages/react-aria-components/docs/ListBox.mdx

@@ -568,6 +568,239 @@ import {Text} from 'react-aria-components';
 </ListBox>
 ```

+### Horizontal layout
+
+By default, ListBox expects items to be arranged in a vertical stack, and implements keyboard navigation and drag and drop accordingly. The `layout` and `orientation` props can be used to change this behavior, allowing you to build horizontal stack, grid, and column layouts.


row, column, and grid layouts?
seems weird to say horizontal stack but not vertical stack, and CSS flex refers to them as row and column, so I think that's natural to say. where as stacks are really from iOS and a few other places that aren't web

i've been going back and forth while reviewing, because I do like layout: stack | grid because you could implement the grid using flex, so layout: flex | grid doesn't make sense, so I can understand using horizontal stack as well, but I still think we should choose either horizontal/vertical stack and grid or stack and grid or row, column, and grid

yeah flex doesn't really describe how the items are arranged. I said vertical stack in the previous sentence when describing the default so I didn't think I needed to repeat that?

personally, "column" layout is a bit confusing since my initial interpretation was "don't grids already have columns"? I like allowing you to build a horizontal stack or a grid personally, maybe further explaining that you can control the way the grid would "wrap" its contents persay (aka pointing out that it is like https://developer.mozilla.org/en-US/docs/Web/CSS/flex-flow, or at least thats how I think about it).

LFDanLu

Behavior looks good to me, tested the RAC story w/ DnD in a variety of locales + orientation/layout combos. Just some questions and requests for clarifications

LFDanLu · 2023-06-20T22:07:24Z

packages/react-aria-components/docs/ListBox.mdx

@@ -568,6 +568,239 @@ import {Text} from 'react-aria-components';
 </ListBox>
 ```

+### Horizontal layout
+
+By default, ListBox expects items to be arranged in a vertical stack, and implements keyboard navigation and drag and drop accordingly. The `layout` and `orientation` props can be used to change this behavior, allowing you to build horizontal stack, grid, and column layouts.


personally, "column" layout is a bit confusing since my initial interpretation was "don't grids already have columns"? I like allowing you to build a horizontal stack or a grid personally, maybe further explaining that you can control the way the grid would "wrap" its contents persay (aka pointing out that it is like https://developer.mozilla.org/en-US/docs/Web/CSS/flex-flow, or at least thats how I think about it).

LFDanLu · 2023-06-20T22:09:39Z

packages/react-aria-components/docs/ListBox.mdx

+
+### Column layout
+
+The `layout="grid"` and `orientation="horizontal"` props can be combined to build a two dimensional grid where the items are grouped into columns, and the grid scrolls horizontally.


Again, I felt it kinda confusing that this is called "Column layout" since the items are also grouped into columns in "Grid layout". Maybe state instead that items fill each column's height before wrapping/moving to the next column?

LFDanLu · 2023-06-20T22:22:06Z

packages/@react-aria/dnd/src/ListDropTargetDelegate.ts

As for your question about if some of this should be split out into a separate GridKeyboardDelegate, IMO it is fine to stay in here since it isn't a true grid movement pattern if that makes sense. The contents of a layout="grid" are "wrapping" in a specific direction rather than being truly laid out in a grid layout and thus left/right or up/down will move focus onto the next row/column depending on the orientation.

LFDanLu · 2023-06-20T22:36:20Z

packages/@react-aria/dnd/src/ListDropTargetDelegate.ts

@@ -35,11 +104,22 @@ export class ListDropTargetDelegate implements DropTargetDelegate {
      let item = items[mid];
      let element = elementMap.get(String(item.key));
      let rect = element.getBoundingClientRect();
+      let update = (isGreater: boolean) => {


Mind adding a comment or two to this code block (aka the stuff within the chain of if statements)? At a glance I think this is narrowing down which item in the collection to consider the drop target with respect to LTR/RTL + the primary/secondary directions but I can't easily tell if the else at the end only happens once that is determined.

LFDanLu · 2023-06-20T22:59:50Z

packages/@react-aria/selection/src/ListKeyboardDelegate.ts

+  private isSameRow(prevRect: DOMRect, itemRect: DOMRect) {
+    return prevRect.top === itemRect.top || prevRect.left !== itemRect.left;
+  }
+
+  private isSameColumn(prevRect: DOMRect, itemRect: DOMRect) {
+    return prevRect.left === itemRect.left || prevRect.top !== itemRect.top;
+  }


Do we have to account for RTL here? Thinking about the case where the rects in a list aren't uniform in width and thus it matters whether or not we are looking at the rect.left or rect.right

rspbot · 2023-06-23T21:29:33Z

Build successful! 🎉

rspbot · 2023-06-26T21:54:08Z

Build successful! 🎉

rspbot · 2023-06-26T21:54:11Z

## API Changes

unknown top level export { type: 'identifier', name: 'Column' }
unknown top level export { type: 'identifier', name: 'Column' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }

@react-aria/dnd

DragPreview

 ListDropTargetDelegate {
-  constructor: (Collection<Node<unknown>>, RefObject<HTMLElement>) => void
+  constructor: (Collection<Node<unknown>>, RefObject<HTMLElement>, ListDropTargetDelegateOptions) => void
   getDropTargetFromPoint: (number, number, (DropTarget) => boolean) => DropTarget
 }

@react-aria/selection

ListKeyboardDelegate

 ListKeyboardDelegate<T> {
   constructor: (Array<any>) => void
   getFirstKey: () => void
   getKeyAbove: (Key) => void
   getKeyBelow: (Key) => void
   getKeyForSearch: (string, Key) => void
   getKeyLeftOf: (Key) => void
   getKeyPageAbove: (Key) => void
   getKeyPageBelow: (Key) => void
   getKeyRightOf: (Key) => void
   getLastKey: () => void
+  getNextKey: (Key) => void
+  getPreviousKey: (Key) => void
 }

reidbarber

LGTM

Add layout and orientation props to RAC ListBox

ac065e7

devongovett commented Jun 15, 2023

View reviewed changes

snowystinger reviewed Jun 15, 2023

View reviewed changes

devongovett added the release label Jun 19, 2023

LFDanLu requested changes Jun 20, 2023

View reviewed changes

devongovett added 2 commits June 23, 2023 14:21

Update docs

8bb8ed3

Merge branch 'main' of github.com:adobe/react-spectrum into listbox-grid

ece67f9

devongovett added 2 commits June 26, 2023 14:27

Merge branch 'main' of github.com:adobe/react-spectrum into listbox-grid

578fc86

Force 2 columns

aa11522

snowystinger approved these changes Jun 26, 2023

View reviewed changes

reidbarber approved these changes Jun 26, 2023

View reviewed changes

devongovett merged commit 3c46c32 into main Jun 26, 2023

devongovett deleted the listbox-grid branch June 26, 2023 22:14

LFDanLu mentioned this pull request Jul 24, 2023

Listbox scrollIntoView behavior doesn't work anymore #4810

Closed

dannify removed the release label Aug 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add layout and orientation props to RAC ListBox #4669

Add layout and orientation props to RAC ListBox #4669

devongovett commented Jun 15, 2023

devongovett Jun 15, 2023

devongovett Jun 15, 2023

devongovett Jun 15, 2023

rspbot commented Jun 15, 2023

snowystinger left a comment •

edited

Loading

snowystinger Jun 15, 2023

snowystinger Jun 15, 2023

devongovett Jun 15, 2023

LFDanLu Jun 20, 2023

LFDanLu left a comment

LFDanLu Jun 20, 2023

LFDanLu Jun 20, 2023

LFDanLu Jun 20, 2023

LFDanLu Jun 20, 2023

LFDanLu Jun 20, 2023

rspbot commented Jun 23, 2023

rspbot commented Jun 26, 2023

rspbot commented Jun 26, 2023

reidbarber left a comment


		### Column layout

		The `layout="grid"` and `orientation="horizontal"` props can be combined to build a two dimensional grid where the items are grouped into columns, and the grid scrolls horizontally.

Add layout and orientation props to RAC ListBox #4669

Add layout and orientation props to RAC ListBox #4669

Conversation

devongovett commented Jun 15, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rspbot commented Jun 15, 2023

snowystinger left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LFDanLu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rspbot commented Jun 23, 2023

rspbot commented Jun 26, 2023

rspbot commented Jun 26, 2023

@react-aria/dnd

DragPreview

@react-aria/selection

ListKeyboardDelegate

reidbarber left a comment

Choose a reason for hiding this comment

snowystinger left a comment •

edited

Loading