CardView grid layout UX fixes + horizontal card support (#2481)

* preventing clicks/taps on focusable elements from scrolling virtualized lists

* adding transitions on card removal + load to CardView

* rough support for horizontal card orientation in grid layout

* moving card view styles into spectrum-css and removing blue outline from clicking on card

* updating story format to csf as per review

* modifying default size of horizontal cards + removing selection mode for horizontal

* adding scale to layouts and shifting default itemPadding values to layouts

* fixing grid layout isVisible check

adding the transition duration broke async loading and toggling items stories in grid layout card view. Turns out that layoutInfo is not always defined in the isVisible check so accounting for that case

* fixing scroll when clicking on action menu trigger in card view

Turns out that opening the action menu in a card view triggers useVirtualizers onFocus when focusSafely is called on the actionmenu "ul". Added a extra conditional to stop useVirtualizer from scrolling to the focused key if focus target is happening on an element outside the virtualizer.

* fixing tests

* cleanup

* adding test for horizontal cards in cardview

* only calling isVisible in GridLayout if layoutInfo is available

* updating quiet vertical cards in grid cardview so that text content is single line

updates the grid layout calc so the content section is always single line height. The rest of the card then expands to fill the available vertical height to accomodate layouts that provide item padding values much greater than what is required to display the single line of text

* fixing horizontal card image aspect ratio resizing

previously we calculated the correct aspect ratio for the image for a horizontal card in a useLayoutEffect but this didnt quite work when the CardView changed in size. Now we calculate the aspect ratio during resize via useResizeObserver

* removing !important styles for cardview and moving them to layoutInfo

* fixing shift tab behavior

* adding offset to scrollToView for cardview so that focus rings are not cut off

* updating scrollToItem in CardView to use margin from layout

* removing some design specific TODOs and moving into issues

* removing another TODO

thought again about this again, we need cardOrientation in the CardViewProps in case the user passes in the layout constructor instead of a layout

* updating type description for gridlayout

* addressing review comments

Co-authored-by: Danni <[email protected]>

Author: LFDanLu

packages/@adobe/spectrum-css-temp/components/card/index.css

@@ -828,7 +828,7 @@ user-provided
     block-size: 100%;
     display: grid;
     grid-template-columns: minmax(0, auto) minmax(30px, 1fr) minmax(0, auto);
-    grid-template-rows: auto var(--spectrum-card-quiet-body-header-margin-top) minmax(var(--spectrum-actionbutton-height), auto) 1fr var(--spectrum-card-body-padding-bottom);
+    grid-template-rows: auto var(--spectrum-card-quiet-body-header-margin-top) minmax(var(--spectrum-actionbutton-height), auto) var(--spectrum-alias-single-line-height) 1fr;
     grid-template-areas:
       "preview preview preview"
       ".       .       ."
@@ -1241,3 +1241,20 @@ For now working around it with a useLayoutEffect in CardBase.
   }
 }
 /** END Standard Horizontal **/
+
+/* CardView styles */
+.spectrum-CardView {
+  outline: none;
+
+  .spectrum-CardView-centeredWrapper {
+    display: flex;
+    align-items: center;
+    justify-content: center;
+    width: 100%;
+    height: 100%;
+  }
+  .spectrum-CardView-row {
+    outline: none;
+    height: 100%
+  }
+}

packages/@react-aria/virtualizer/package.json

@@ -19,6 +19,7 @@
   "dependencies": {
     "@babel/runtime": "^7.6.2",
     "@react-aria/i18n": "^3.3.2",
+    "@react-aria/interactions": "^3.6.0",
     "@react-aria/utils": "^3.8.2",
     "@react-stately/virtualizer": "^3.1.5",
     "@react-types/shared": "^3.8.0"

packages/@react-aria/virtualizer/src/Virtualizer.tsx

@@ -12,6 +12,7 @@
 
 import {Collection} from '@react-types/shared';
 import {focusWithoutScrolling, mergeProps, useLayoutEffect} from '@react-aria/utils';
+import {getInteractionModality} from '@react-aria/interactions';
 import {Layout, Rect, ReusableView, useVirtualizerState, VirtualizerState} from '@react-stately/virtualizer';
 import React, {FocusEvent, HTMLAttributes, Key, ReactElement, RefObject, useCallback, useEffect, useRef} from 'react';
 import {ScrollView} from './ScrollView';
@@ -33,7 +34,8 @@ interface VirtualizerProps<T extends object, V> extends HTMLAttributes<HTMLEleme
   transitionDuration?: number,
   isLoading?: boolean,
   onLoadMore?: () => void,
-  shouldUseVirtualFocus?: boolean
+  shouldUseVirtualFocus?: boolean,
+  scrollToItem?: (key: Key) => void
 }
 
 function Virtualizer<T extends object, V>(props: VirtualizerProps<T, V>, ref: RefObject<HTMLDivElement>) {
@@ -51,6 +53,8 @@ function Virtualizer<T extends object, V>(props: VirtualizerProps<T, V>, ref: Re
     focusedKey,
     // eslint-disable-next-line @typescript-eslint/no-unused-vars
     shouldUseVirtualFocus,
+    // eslint-disable-next-line @typescript-eslint/no-unused-vars
+    scrollToItem,
     ...otherProps
   } = props;
 
@@ -125,8 +129,8 @@ export function useVirtualizer<T extends object, V, W>(props: VirtualizerOptions
     if (virtualizer.visibleRect.height === 0) {
       return;
     }
-
-    if (focusedKey !== lastFocusedKey.current) {
+    let modality = getInteractionModality();
+    if (focusedKey !== lastFocusedKey.current && modality !== 'pointer') {
       if (scrollToItem) {
         scrollToItem(focusedKey);
       } else {
@@ -141,7 +145,8 @@ export function useVirtualizer<T extends object, V, W>(props: VirtualizerOptions
   let onFocus = useCallback((e: FocusEvent) => {
     // If the focused item is scrolled out of view and is not in the DOM, the collection
     // will have tabIndex={0}. When tabbing in from outside, scroll the focused item into view.
-    if (!isFocusWithin.current) {
+    // Ignore focus events that bubble through portals (e.g. focus that happens on a menu popover child of the virtualizer)
+    if (!isFocusWithin.current && ref.current.contains(e.target)) {
       if (scrollToItem) {
         scrollToItem(focusedKey);
       } else {

packages/@react-aria/virtualizer/src/VirtualizerItem.tsx

@@ -57,7 +57,7 @@ export function layoutInfoToStyle(layoutInfo: LayoutInfo, dir: Direction, parent
 
   let style: CSSProperties = {
     position: layoutInfo.isSticky ? 'sticky' : 'absolute',
-    overflow: 'hidden',
+    overflow: layoutInfo.allowOverflow ? 'visible' : 'hidden',
     top: layoutInfo.rect.y - (parent ? parent.rect.y : 0),
     [xProperty]: layoutInfo.rect.x - (parent ? parent.rect.x : 0),
     transition: 'all',
@@ -69,7 +69,7 @@ export function layoutInfoToStyle(layoutInfo: LayoutInfo, dir: Direction, parent
     opacity: layoutInfo.opacity,
     zIndex: layoutInfo.zIndex,
     transform: layoutInfo.transform,
-    contain: 'size layout style paint'
+    contain: layoutInfo.allowOverflow ? 'size layout style' : 'size layout style paint'
   };
 
   cache.set(layoutInfo, style);

packages/@react-spectrum/card/package.json

@@ -49,6 +49,7 @@
     "@react-stately/utils": "^3.2.1",
     "@react-stately/virtualizer": "^3.1.5",
     "@react-types/card": "3.0.0-alpha.0",
+    "@react-types/provider": "^3.3.2",
     "@react-types/shared": "^3.0.0"
   },
   "devDependencies": {

packages/@react-spectrum/card/src/BaseLayout.tsx

@@ -14,9 +14,17 @@ import {Direction, KeyboardDelegate, Node} from '@react-types/shared';
 import {GridCollection} from '@react-stately/grid';
 import {InvalidationContext, Layout, LayoutInfo, Rect, Size} from '@react-stately/virtualizer';
 import {Key} from 'react';
+import {Scale} from '@react-types/provider';
 
 export interface BaseLayoutOptions {
-  collator?: Intl.Collator
+  collator?: Intl.Collator,
+  // TODO: is this valid or is scale a spectrum specific thing that should be left out of the layouts?
+  scale?: Scale,
+  /**
+   * The margin around the grid view between the edges and the items.
+   * @default 24
+   */
+  margin?: number
 }
 
 export class BaseLayout<T> extends Layout<Node<T>> implements KeyboardDelegate {
@@ -28,12 +36,16 @@ export class BaseLayout<T> extends Layout<Node<T>> implements KeyboardDelegate {
   isLoading: boolean;
   disabledKeys: Set<Key> = new Set();
   direction: Direction;
+  scale: Scale;
+  margin: number;
 
   constructor(options: BaseLayoutOptions = {}) {
     super();
     this.layoutInfos = new Map();
     this.collator = options.collator;
     this.lastCollection = null;
+    this.scale = options.scale || 'medium';
+    this.margin = options.margin || 24;
   }
 
   validate(invalidationContext: InvalidationContext<Node<T>, unknown>) {

packages/@react-spectrum/card/src/CardBase.tsx

@@ -15,9 +15,9 @@ import {Checkbox} from '@react-spectrum/checkbox';
 import {classNames, SlotProvider, useDOMRef, useHasChild, useStyleProps} from '@react-spectrum/utils';
 import {Divider} from '@react-spectrum/divider';
 import {DOMRef, Node} from '@react-types/shared';
-import {filterDOMProps, mergeProps, useLayoutEffect, useSlotId} from '@react-aria/utils';
+import {filterDOMProps, mergeProps, useLayoutEffect, useResizeObserver, useSlotId} from '@react-aria/utils';
 import {FocusRing} from '@react-aria/focus';
-import React, {HTMLAttributes, useMemo, useRef, useState} from 'react';
+import React, {HTMLAttributes, useCallback, useMemo, useRef, useState} from 'react';
 import styles from '@adobe/spectrum-css-temp/components/card/vars.css';
 import {useCardViewContext} from './CardViewContext';
 import {useFocusWithin, useHover} from '@react-aria/interactions';
@@ -30,7 +30,6 @@ interface CardBaseProps<T> extends SpectrumCardProps {
 
 function CardBase<T extends object>(props: CardBaseProps<T>, ref: DOMRef<HTMLDivElement>) {
   props = useProviderProps(props);
-  // TODO: Don't send in articleProps via context (unless we want to make another context for InternalCard)? Pass it in via props since it will only be provided via CardView's InternalCard
   let context = useCardViewContext() || {}; // we can call again here, won't change from Card.tsx
   let {state} = context;
   let manager = state?.selectionManager;
@@ -69,13 +68,16 @@ function CardBase<T extends object>(props: CardBaseProps<T>, ref: DOMRef<HTMLDiv
 
   // this is for horizontal cards
   let [height, setHeight] = useState(NaN);
-  useLayoutEffect(() => {
+  let updateHeight = useCallback(() => {
     if (orientation !== 'horizontal') {
       return;
     }
+
     let cardHeight = gridRef.current.getBoundingClientRect().height;
     setHeight(cardHeight);
-  }, [gridRef, setHeight, orientation]);
+  }, [orientation, gridRef, setHeight]);
+  useResizeObserver({ref: gridRef, onResize: updateHeight});
+
   let aspectRatioEnforce = undefined;
   if (orientation === 'horizontal' && !isNaN(height)) {
     aspectRatioEnforce = {

packages/@react-spectrum/card/src/CardView.tsx

@@ -12,16 +12,16 @@
 
 import {CardBase} from './CardBase';
 import {CardViewContext, useCardViewContext} from './CardViewContext';
-import cardViewStyles from './cardview.css';
 import {classNames, useDOMRef, useStyleProps, useUnwrapDOMRef} from '@react-spectrum/utils';
 import {DOMRef, DOMRefValue, Node} from '@react-types/shared';
 import {GridCollection, useGridState} from '@react-stately/grid';
 // @ts-ignore
 import intlMessages from '../intl/*.json';
 import {ProgressCircle} from '@react-spectrum/progress';
-import React, {ReactElement, useMemo, useRef} from 'react';
+import React, {ReactElement, useCallback, useMemo, useRef} from 'react';
 import {ReusableView} from '@react-stately/virtualizer';
 import {SpectrumCardViewProps} from '@react-types/card';
+import styles from '@adobe/spectrum-css-temp/components/card/vars.css';
 import {useCollator, useLocale, useMessageFormatter} from '@react-aria/i18n';
 import {useGrid, useGridCell, useGridRow} from '@react-aria/grid';
 import {useListState} from '@react-stately/list';
@@ -37,21 +37,15 @@ function CardView<T extends object>(props: SpectrumCardViewProps<T>, ref: DOMRef
     renderEmptyState,
     layout,
     loadingState,
-    onLoadMore
+    onLoadMore,
+    cardOrientation = 'vertical'
   } = props;
+
   let collator = useCollator({usage: 'search', sensitivity: 'base'});
   let isLoading = loadingState === 'loading' || loadingState === 'loadingMore';
-  let cardViewLayout = useMemo(() => typeof layout === 'function' ? new layout({collator}) : layout, [layout, collator]);
+  let cardViewLayout = useMemo(() => typeof layout === 'function' ? new layout({collator, cardOrientation, scale}) : layout, [layout, collator, cardOrientation, scale]);
   let layoutType = cardViewLayout.layoutType;
 
-  if (typeof layout === 'function') {
-    if (layoutType === 'grid') {
-      cardViewLayout.itemPadding = scale === 'large' ? 116 : 95;
-    } else if (layoutType === 'gallery') {
-      cardViewLayout.itemPadding = scale === 'large' ? 143 : 114;
-    }
-  }
-
   let formatMessage = useMessageFormatter(intlMessages);
   let {direction} = useLocale();
   let {collection} = useListState(props);
@@ -77,6 +71,7 @@ function CardView<T extends object>(props: SpectrumCardViewProps<T>, ref: DOMRef
 
   let state = useGridState({
     ...props,
+    selectionMode: cardOrientation === 'horizontal' && layoutType === 'grid' ? 'none' : props.selectionMode,
     collection: gridCollection,
     focusMode: 'cell'
   });
@@ -95,7 +90,6 @@ function CardView<T extends object>(props: SpectrumCardViewProps<T>, ref: DOMRef
   type View = ReusableView<Node<T>, unknown>;
   let renderWrapper = (parent: View, reusableView: View) => (
     <VirtualizerItem
-      className={classNames(cardViewStyles, 'react-spectrum-CardView-CardWrapper')}
       key={reusableView.key}
       reusableView={reusableView}
       parent={parent} />
@@ -107,21 +101,32 @@ function CardView<T extends object>(props: SpectrumCardViewProps<T>, ref: DOMRef
     focusedKey = focusedItem.parentKey;
   }
 
+  let margin = cardViewLayout.margin || 0;
+  let virtualizer = cardViewLayout.virtualizer;
+  let scrollToItem = useCallback((key) => {
+    virtualizer && virtualizer.scrollToItem(key, {
+      duration: 0,
+      offsetY: margin
+    });
+  }, [margin, virtualizer]);
+
   // TODO: does aria-row count and aria-col count need to be modified? Perhaps aria-col count needs to be omitted
   return (
-    <CardViewContext.Provider value={{state, isQuiet, layout: cardViewLayout}}>
+    <CardViewContext.Provider value={{state, isQuiet, layout: cardViewLayout, cardOrientation}}>
       <Virtualizer
         {...gridProps}
         {...styleProps}
-        className={classNames(cardViewStyles, 'react-spectrum-CardView')}
+        className={classNames(styles, 'spectrum-CardView')}
         ref={domRef}
         focusedKey={focusedKey}
         scrollDirection="vertical"
         layout={cardViewLayout}
         collection={gridCollection}
         isLoading={isLoading}
         onLoadMore={onLoadMore}
-        renderWrapper={renderWrapper}>
+        renderWrapper={renderWrapper}
+        transitionDuration={isLoading ? 160 : 220}
+        scrollToItem={scrollToItem}>
         {(type, item) => {
           if (type === 'item') {
             return (
@@ -160,7 +165,7 @@ function CenteredWrapper({children}) {
     <div
       role="row"
       aria-rowindex={state.collection.size + 1}
-      className={classNames(cardViewStyles, 'react-spectrum-CardView-centeredWrapper')}>
+      className={classNames(styles, 'spectrum-CardView-centeredWrapper')}>
       <div role="gridcell">
         {children}
       </div>
@@ -195,8 +200,12 @@ function InternalCard(props) {
     isQuiet = true;
   }
 
+  if (layoutType !== 'grid') {
+    cardOrientation = 'vertical';
+  }
+
   return (
-    <div {...rowProps} ref={rowRef} style={{height: '100%'}}>
+    <div {...rowProps} ref={rowRef} className={classNames(styles, 'spectrum-CardView-row')}>
       <CardBase
         ref={cellRef}
         articleProps={gridCellProps}

packages/@react-spectrum/card/src/GalleryLayout.tsx

@@ -43,22 +43,14 @@ export interface GalleryLayoutOptions extends BaseLayoutOptions {
    * will be targeted.
    * @type {number}
    */
-  threshold?: number,
-  /**
-   * The margin around the grid view between the edges and the items.
-   * @default 24
-   */
-  margin?: number // TODO: Perhaps should accept Responsive<DimensionValue>
+  threshold?: number
 }
 
-// TODO: copied from V2, update this with the proper spectrum values
-// Should these be affected by Scale as well?
 const DEFAULT_OPTIONS = {
   S: {
     idealRowHeight: 112,
     minItemSize: new Size(96, 96),
     itemSpacing: new Size(8, 16),
-    // TODO: will need to update as well
     itemPadding: 24,
     dropSpacing: 50,
     margin: 8
@@ -67,28 +59,28 @@ const DEFAULT_OPTIONS = {
     idealRowHeight: 208,
     minItemSize: new Size(136, 136),
     itemSpacing: new Size(18, 18),
-    // TODO: updated to work with new v3 cards (there is additional space required for the descriptions if there is a description)
-    itemPadding: 114,
+    itemPadding: {
+      'medium': 114,
+      'large': 143
+    },
     dropSpacing: 100,
     margin: 24
   }
 };
 
 export class GalleryLayout<T> extends BaseLayout<T> {
   protected idealRowHeight: number;
-  protected margin: number;
   protected itemSpacing: Size;
   itemPadding: number;
   protected minItemSize: Size;
   protected threshold: number;
 
   constructor(options: GalleryLayoutOptions = {}) {
     super(options);
-    // TODO: restore cardSize option when we support different size cards
     let cardSize = 'L';
     this.idealRowHeight = options.idealRowHeight || DEFAULT_OPTIONS[cardSize].idealRowHeight;
     this.itemSpacing = options.itemSpacing || DEFAULT_OPTIONS[cardSize].itemSpacing;
-    this.itemPadding = options.itemPadding != null ? options.itemPadding : DEFAULT_OPTIONS[cardSize].itemPadding;
+    this.itemPadding = options.itemPadding != null ? options.itemPadding : DEFAULT_OPTIONS[cardSize].itemPadding[this.scale];
     this.minItemSize = options.minItemSize || DEFAULT_OPTIONS[cardSize].minItemSize;
     this.threshold = options.threshold || 1;
     this.margin = options.margin != null ? options.margin : DEFAULT_OPTIONS[cardSize].margin;
@@ -213,6 +205,7 @@ export class GalleryLayout<T> extends BaseLayout<T> {
         let itemWidth = Math.max(widths[j - index][1], this.minItemSize.width);
         let rect = new Rect(x, y, itemWidth, itemHeight);
         let layoutInfo = new LayoutInfo(node.type, node.key, rect);
+        layoutInfo.allowOverflow = true;
         this.layoutInfos.set(node.key, layoutInfo);
         x += itemWidth + this.itemSpacing.width;
       }