Update keyboard navigation in CardView (#2737)

* remove action menu and footer from cards in cardview

* removing footer + action menu from standalone cards and fixing css for detail

* fixing Card tests

* adding prop to stop focus from making it to cell children

* adding keyboard nav tests for CardView

* fixing tests

* reserving more room for detail text in large scale

* adding warning if focusable elements are present in card

also removed footer and action menu slots from card

* fixing lint

* removing preventChildrenFocus prop in favor of deleting gridcell keydown handler from Card

stopgap solution so that we dont have to add a new prop just to prevent focus on Card checkbox during keyboard nav

* Reduce vertical item spacing in CardView and remove item padding option from Waterfall layout (#2758)

* reducing gallery and waterfall quiet card vertical in between spacing

* removing item padding from waterfall layout

* fixing errorneous scaled height calculation

Author: LFDanLu

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

@@ -13,10 +13,10 @@
 import {AriaCardProps, SpectrumCardProps} from '@react-types/card';
 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, useResizeObserver, useSlotId} from '@react-aria/utils';
 import {FocusRing} from '@react-aria/focus';
+import {getFocusableTreeWalker} from  '@react-aria/focus';
 import React, {HTMLAttributes, useCallback, useMemo, useRef, useState} from 'react';
 import styles from '@adobe/spectrum-css-temp/components/card/vars.css';
 import {useCardViewContext} from './CardViewContext';
@@ -38,7 +38,8 @@ function CardBase<T extends object>(props: CardBaseProps<T>, ref: DOMRef<HTMLDiv
     orientation = 'vertical',
     articleProps = {},
     item,
-    layout
+    layout,
+    children
   } = props;
 
   let key = item?.key;
@@ -50,6 +51,7 @@ function CardBase<T extends object>(props: CardBaseProps<T>, ref: DOMRef<HTMLDiv
   let {cardProps, titleProps, contentProps} = useCard(props);
   let domRef = useDOMRef(ref);
   let gridRef = useRef<HTMLDivElement>();
+  let checkboxRef = useRef(null);
 
   // cards are only interactive if there is a selection manager and it allows selection
   let {hoverProps, isHovered} = useHover({isDisabled: manager === undefined || manager?.selectionMode === 'none' || isDisabled});
@@ -59,8 +61,6 @@ function CardBase<T extends object>(props: CardBaseProps<T>, ref: DOMRef<HTMLDiv
     isDisabled
   });
 
-  let hasFooter = useHasChild(`.${styles['spectrum-Card-footer']}`, gridRef);
-
   // ToDo: see css for comment about avatar under selector .spectrum-Card--noLayout.spectrum-Card--default
   let hasPreviewImage = useHasChild(`.${styles['spectrum-Card-image']}`, gridRef);
   let hasPreviewIllustration = useHasChild(`.${styles['spectrum-Card-illustration']}`, gridRef);
@@ -92,10 +92,7 @@ function CardBase<T extends object>(props: CardBaseProps<T>, ref: DOMRef<HTMLDiv
     avatar: {UNSAFE_className: classNames(styles, 'spectrum-Card-avatar'), size: 'avatar-size-400'},
     heading: {UNSAFE_className: classNames(styles, 'spectrum-Card-heading'), ...titleProps},
     content: {UNSAFE_className: classNames(styles, 'spectrum-Card-content'), ...contentProps},
-    detail: {UNSAFE_className: classNames(styles, 'spectrum-Card-detail')},
-    actionmenu: {UNSAFE_className: classNames(styles, 'spectrum-Card-actions'), align: 'end', isQuiet: true},
-    footer: {UNSAFE_className: classNames(styles, 'spectrum-Card-footer'), isHidden: isQuiet},
-    divider: {UNSAFE_className: classNames(styles, 'spectrum-Card-divider'), size: 'S'}
+    detail: {UNSAFE_className: classNames(styles, 'spectrum-Card-detail')}
   }), [titleProps, contentProps, height, isQuiet, orientation]);
 
   // This is only for quiet grid cards
@@ -147,8 +144,21 @@ function CardBase<T extends object>(props: CardBaseProps<T>, ref: DOMRef<HTMLDiv
       };
     }
     // ToDo: how to re-run if image src changes?
-  }, [props.children, setIsCloseToSquare, isQuiet, layout]);
+  }, [children, setIsCloseToSquare, isQuiet, layout]);
 
+  useLayoutEffect(() => {
+    if (gridRef?.current) {
+      let walker = getFocusableTreeWalker(gridRef.current);
+      let nextNode = walker.nextNode();
+      while (nextNode != null) {
+        if (checkboxRef.current && !checkboxRef.current.UNSAFE_getDOMNode().contains(nextNode)) {
+          console.warn('Card does not support focusable elements, please contact the team regarding your use case.');
+          break;
+        }
+        nextNode = walker.nextNode();
+      }
+    }
+  }, [children]);
 
   return (
     <FocusRing focusRingClass={classNames(styles, 'focus-ring')}>
@@ -174,6 +184,7 @@ function CardBase<T extends object>(props: CardBaseProps<T>, ref: DOMRef<HTMLDiv
           {manager && manager.selectionMode !== 'none' && (
             <div className={classNames(styles, 'spectrum-Card-checkboxWrapper')}>
               <Checkbox
+                ref={checkboxRef}
                 isDisabled={isDisabled}
                 excludeFromTabOrder
                 isSelected={isSelected}
@@ -184,8 +195,7 @@ function CardBase<T extends object>(props: CardBaseProps<T>, ref: DOMRef<HTMLDiv
             </div>
           )}
           <SlotProvider slots={slots}>
-            {props.children}
-            {hasFooter && <Divider />}
+            {children}
           </SlotProvider>
           <div className={classNames(styles, 'spectrum-Card-decoration')} />
         </div>

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

@@ -204,6 +204,10 @@ function InternalCard(props) {
     cardOrientation = 'vertical';
   }
 
+  // We don't want to focus the checkbox (or any other focusable elements) within the Card
+  // when pressing the arrow keys so we delete the key down handler here. Arrow key navigation between
+  // the cards in the CardView is handled by useGrid => useSelectableCollection instead.
+  delete gridCellProps.onKeyDownCapture;
   return (
     <div {...rowProps} ref={rowRef} className={classNames(styles, 'spectrum-CardView-row')}>
       <CardBase

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

@@ -30,7 +30,7 @@ export interface GalleryLayoutOptions extends BaseLayoutOptions {
   itemSpacing?: Size,
   /**
    * The vertical padding for an item.
-   * @default 114
+   * @default 78
    */
   itemPadding?: number,
   /**
@@ -60,8 +60,8 @@ const DEFAULT_OPTIONS = {
     minItemSize: new Size(136, 136),
     itemSpacing: new Size(18, 18),
     itemPadding: {
-      'medium': 114,
-      'large': 143
+      'medium': 78,
+      'large': 99
     },
     dropSpacing: 100,
     margin: 24

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

@@ -35,12 +35,7 @@ export interface WaterfallLayoutOptions extends BaseLayoutOptions {
    * The maximum number of columns.
    * @default Infinity
    */
-  maxColumns?: number,
-  /**
-   * The vertical padding for an item.
-   * @default 56
-   */
-  itemPadding?: number
+  maxColumns?: number
 }
 
 // TODO: this didn't have any options that varied with card size, should it have?
@@ -49,7 +44,6 @@ export class WaterfallLayout<T> extends BaseLayout<T> implements KeyboardDelegat
   protected maxItemSize: Size;
   protected minSpace: Size;
   protected maxColumns: number;
-  itemPadding: number;
   protected numColumns: number;
   protected itemWidth: number;
   protected horizontalSpacing: number;
@@ -63,8 +57,6 @@ export class WaterfallLayout<T> extends BaseLayout<T> implements KeyboardDelegat
     this.margin = options.margin != null ? options.margin : 24;
     this.minSpace = options.minSpace || new Size(18, 18);
     this.maxColumns = options.maxColumns || Infinity;
-    // TODO: not entirely sure what this is for since the layout will automatically shift itself to the correct vertical space for the card
-    this.itemPadding = options.itemPadding != null ? options.itemPadding : 56;
 
     this.itemWidth = 0;
     this.numColumns = 0;
@@ -113,8 +105,8 @@ export class WaterfallLayout<T> extends BaseLayout<T> implements KeyboardDelegat
       } else if (node.props.width && node.props.height) {
         let nodeWidth = node.props.width;
         let nodeHeight = node.props.height;
-        let scaledHeight = Math.round(nodeWidth * ((itemWidth) / nodeHeight));
-        height = Math.max(this.minItemSize.height, Math.min(this.maxItemSize.height, scaledHeight)) + this.itemPadding;
+        let scaledHeight = Math.round(nodeHeight * ((itemWidth) / nodeWidth));
+        height = Math.max(this.minItemSize.height, Math.min(this.maxItemSize.height, scaledHeight));
       } else {
         height = itemWidth;
       }