S2 CardView and ghost loading (#6978)

Author: devongovett

.eslintrc.js

@@ -120,7 +120,8 @@ module.exports = {
     'AsyncIterable': 'readonly',
     'FileSystemFileEntry': 'readonly',
     'FileSystemDirectoryEntry': 'readonly',
-    'FileSystemEntry': 'readonly'
+    'FileSystemEntry': 'readonly',
+    'IS_REACT_ACT_ENVIRONMENT': 'readonly'
   },
   settings: {
     jsdoc: {

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

@@ -74,32 +74,27 @@ export class ListKeyboardDelegate<T> implements KeyboardDelegate {
     return this.disabledBehavior === 'all' && (item.props?.isDisabled || this.disabledKeys.has(item.key));
   }
 
-  getNextKey(key: Key) {
-    key = this.collection.getKeyAfter(key);
+  private findNextNonDisabled(key: Key, getNext: (key: Key) => Key | null): Key | null {
     while (key != null) {
       let item = this.collection.getItem(key);
-      if (item.type === 'item' && !this.isDisabled(item)) {
+      if (item?.type === 'item' && !this.isDisabled(item)) {
         return key;
       }
 
-      key = this.collection.getKeyAfter(key);
+      key = getNext(key);
     }
 
     return null;
   }
 
+  getNextKey(key: Key) {
+    key = this.collection.getKeyAfter(key);
+    return this.findNextNonDisabled(key, key => this.collection.getKeyAfter(key));
+  }
+
   getPreviousKey(key: Key) {
     key = this.collection.getKeyBefore(key);
-    while (key != null) {
-      let item = this.collection.getItem(key);
-      if (item.type === 'item' && !this.isDisabled(item)) {
-        return key;
-      }
-
-      key = this.collection.getKeyBefore(key);
-    }
-
-    return null;
+    return this.findNextNonDisabled(key, key => this.collection.getKeyBefore(key));
   }
 
   private findKey(
@@ -151,6 +146,14 @@ export class ListKeyboardDelegate<T> implements KeyboardDelegate {
   }
 
   getKeyRightOf(key: Key) {
+    // This is a temporary solution for CardView until we refactor useSelectableCollection.
+    // https://github.com/orgs/adobe/projects/19/views/32?pane=issue&itemId=77825042
+    let layoutDelegateMethod = this.direction === 'ltr' ? 'getKeyRightOf' : 'getKeyLeftOf';
+    if (this.layoutDelegate[layoutDelegateMethod]) {
+      key = this.layoutDelegate[layoutDelegateMethod](key);
+      return this.findNextNonDisabled(key, key => this.layoutDelegate[layoutDelegateMethod](key));
+    }
+
     if (this.layout === 'grid') {
       if (this.orientation === 'vertical') {
         return this.getNextColumn(key, this.direction === 'rtl');
@@ -165,6 +168,12 @@ export class ListKeyboardDelegate<T> implements KeyboardDelegate {
   }
 
   getKeyLeftOf(key: Key) {
+    let layoutDelegateMethod = this.direction === 'ltr' ? 'getKeyLeftOf' : 'getKeyRightOf';
+    if (this.layoutDelegate[layoutDelegateMethod]) {
+      key = this.layoutDelegate[layoutDelegateMethod](key);
+      return this.findNextNonDisabled(key, key => this.layoutDelegate[layoutDelegateMethod](key));
+    }
+
     if (this.layout === 'grid') {
       if (this.orientation === 'vertical') {
         return this.getNextColumn(key, this.direction === 'ltr');
@@ -180,30 +189,12 @@ export class ListKeyboardDelegate<T> implements KeyboardDelegate {
 
   getFirstKey() {
     let key = this.collection.getFirstKey();
-    while (key != null) {
-      let item = this.collection.getItem(key);
-      if (item?.type === 'item' && !this.isDisabled(item)) {
-        return key;
-      }
-
-      key = this.collection.getKeyAfter(key);
-    }
-
-    return null;
+    return this.findNextNonDisabled(key, key => this.collection.getKeyAfter(key));
   }
 
   getLastKey() {
     let key = this.collection.getLastKey();
-    while (key != null) {
-      let item = this.collection.getItem(key);
-      if (item.type === 'item' && !this.isDisabled(item)) {
-        return key;
-      }
-
-      key = this.collection.getKeyBefore(key);
-    }
-
-    return null;
+    return this.findNextNonDisabled(key, key => this.collection.getKeyBefore(key));
   }
 
   getKeyPageAbove(key: Key) {
@@ -280,7 +271,7 @@ export class ListKeyboardDelegate<T> implements KeyboardDelegate {
         return key;
       }
 
-      key = this.getKeyBelow(key);
+      key = this.getNextKey(key);
     }
 
     return null;

packages/@react-aria/utils/src/useLoadMore.ts

@@ -29,7 +29,7 @@ export interface LoadMoreProps {
    */
   scrollOffset?: number,
   /** The data currently loaded. */
-  items?: any[]
+  items?: any
 }
 
 export function useLoadMore(props: LoadMoreProps, ref: RefObject<HTMLElement | null>) {

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

@@ -143,12 +143,17 @@ export function useScrollView(props: ScrollViewProps, ref: RefObject<HTMLElement
   // eslint-disable-next-line react-hooks/exhaustive-deps
   }, []);
 
+  let isUpdatingSize = useRef(false);
   let updateSize = useEffectEvent((flush: typeof flushSync) => {
     let dom = ref.current;
-    if (!dom) {
+    if (!dom && !isUpdatingSize.current) {
       return;
     }
 
+    // Prevent reentrancy when resize observer fires, triggers re-layout that results in
+    // content size update, causing below layout effect to fire. This avoids infinite loops.
+    isUpdatingSize.current = true;
+
     let isTestEnv = process.env.NODE_ENV === 'test' && !process.env.VIRT_ON;
     let isClientWidthMocked = Object.getOwnPropertyNames(window.HTMLElement.prototype).includes('clientWidth');
     let isClientHeightMocked = Object.getOwnPropertyNames(window.HTMLElement.prototype).includes('clientHeight');
@@ -177,27 +182,31 @@ export function useScrollView(props: ScrollViewProps, ref: RefObject<HTMLElement
         });
       }
     }
+
+    isUpdatingSize.current = false;
   });
 
-  let didUpdateSize = useRef(false);
+  // Update visible rect when the content size changes, in case scrollbars need to appear or disappear.
+  let lastContentSize = useRef<Size | null>(null);
   useLayoutEffect(() => {
-    // React doesn't allow flushSync inside effects, so queue a microtask.
-    // We also need to wait until all refs are set (e.g. when passing a ref down from a parent).
-    queueMicrotask(() => {
-      if (!didUpdateSize.current) {
-        didUpdateSize.current = true;
-        updateSize(flushSync);
+    if (!isUpdatingSize.current && (lastContentSize.current == null || !contentSize.equals(lastContentSize.current))) {
+      // React doesn't allow flushSync inside effects, so queue a microtask.
+      // We also need to wait until all refs are set (e.g. when passing a ref down from a parent).
+      // If we are in an `act` environment, update immediately without a microtask so you don't need
+      // to mock timers in tests. In this case, the update is synchronous already.
+      // IS_REACT_ACT_ENVIRONMENT is used by React 18. Previous versions checked for the `jest` global.
+      // https://github.com/reactwg/react-18/discussions/102
+      // @ts-ignore
+      if (typeof IS_REACT_ACT_ENVIRONMENT === 'boolean' ? IS_REACT_ACT_ENVIRONMENT : typeof jest !== 'undefined') {
+        updateSize(fn => fn());
+      } else {
+        queueMicrotask(() => updateSize(flushSync));
       }
-    });
-  }, [updateSize]);
-  useEffect(() => {
-    if (!didUpdateSize.current) {
-      // If useEffect ran before the above microtask, we are in a synchronous render (e.g. act).
-      // Update the size here so that you don't need to mock timers in tests.
-      didUpdateSize.current = true;
-      updateSize(fn => fn());
     }
-  }, [updateSize]);
+
+    lastContentSize.current = contentSize;
+  });
+
   let onResize = useCallback(() => {
     updateSize(flushSync);
   }, [updateSize]);

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

@@ -129,6 +129,7 @@
     "@react-aria/interactions": "^3.22.2",
     "@react-aria/utils": "^3.25.2",
     "@react-spectrum/utils": "^3.11.10",
+    "@react-stately/virtualizer": "^4.0.1",
     "@react-types/color": "3.0.0-rc.1",
     "@react-types/dialog": "^3.5.8",
     "@react-types/provider": "^3.7.2",

packages/@react-spectrum/s2/src/ActionButton.tsx

@@ -11,15 +11,17 @@
  */
 
 import {baseColor, fontRelative, style} from '../style/spectrum-theme' with { type: 'macro' };
-import {ButtonProps, ButtonRenderProps, ContextValue, OverlayTriggerStateContext, Provider, Button as RACButton} from 'react-aria-components';
+import {ButtonProps, ButtonRenderProps, ContextValue, OverlayTriggerStateContext, Provider, Button as RACButton, Text} from 'react-aria-components';
 import {centerBaseline} from './CenterBaseline';
 import {createContext, forwardRef, ReactNode, useContext} from 'react';
 import {FocusableRef, FocusableRefValue} from '@react-types/shared';
 import {focusRing, getAllowedOverrides, StyleProps} from './style-utils' with { type: 'macro' };
 import {IconContext} from './Icon';
 import {pressScale} from './pressScale';
-import {Text, TextContext} from './Content';
+import {SkeletonContext} from './Skeleton';
+import {TextContext} from './Content';
 import {useFocusableRef} from '@react-spectrum/utils';
+import {useFormProps} from './Form';
 import {useSpectrumContextProps} from './useSpectrumContextProps';
 
 export interface ActionButtonStyleProps {
@@ -175,6 +177,7 @@ export const ActionButtonContext = createContext<ContextValue<ActionButtonProps,
 
 function ActionButton(props: ActionButtonProps, ref: FocusableRef<HTMLButtonElement>) {
   [props, ref] = useSpectrumContextProps(props, ref, ActionButtonContext);
+  props = useFormProps(props as any);
   let domRef = useFocusableRef(ref);
   let overlayTriggerState = useContext(OverlayTriggerStateContext);
 
@@ -193,6 +196,7 @@ function ActionButton(props: ActionButtonProps, ref: FocusableRef<HTMLButtonElem
       }, props.styles)}>
       <Provider
         values={[
+          [SkeletonContext, null],
           [TextContext, {styles: style({paddingY: '--labelPadding', order: 1, truncate: true})}],
           [IconContext, {
             render: centerBaseline({slot: 'icon', styles: style({order: 0})}),

packages/@react-spectrum/s2/src/ActionMenu.tsx

@@ -26,10 +26,11 @@ import {useSpectrumContextProps} from './useSpectrumContextProps';
 
 export interface ActionMenuProps<T> extends
   Pick<MenuTriggerProps, 'isOpen' | 'defaultOpen' | 'onOpenChange' | 'align' | 'direction' | 'shouldFlip'>,
-  Pick<MenuProps<T>, 'children' | 'items' | 'disabledKeys' | 'onAction' | 'size'>,
-  Pick<ActionButtonProps, 'isDisabled' | 'isQuiet' | 'autoFocus'>,
+  Pick<MenuProps<T>, 'children' | 'items' | 'disabledKeys' | 'onAction'>,
+  Pick<ActionButtonProps, 'isDisabled' | 'isQuiet' | 'autoFocus' | 'size'>,
   StyleProps, DOMProps, AriaLabelingProps {
-  }
+  menuSize?: 'S' | 'M' | 'L' | 'XL'
+}
 
 export const ActionMenuContext = createContext<ContextValue<ActionMenuProps<any>, FocusableRefValue<HTMLButtonElement>>>(null);
 
@@ -41,7 +42,6 @@ function ActionMenu<T extends object>(props: ActionMenuProps<T>, ref: FocusableR
     buttonProps['aria-label'] = stringFormatter.format('menu.moreActions');
   }
 
-  // size independently controlled?
   return (
     <MenuTrigger
       isOpen={props.isOpen}
@@ -52,19 +52,19 @@ function ActionMenu<T extends object>(props: ActionMenuProps<T>, ref: FocusableR
       shouldFlip={props.shouldFlip}>
       <ActionButton
         ref={ref}
-        aria-label="Help"
         size={props.size}
         isDisabled={props.isDisabled}
         autoFocus={props.autoFocus}
         isQuiet={props.isQuiet}
+        styles={props.styles}
         {...buttonProps}>
         <MoreIcon />
       </ActionButton>
       <Menu
         items={props.items}
         disabledKeys={props.disabledKeys}
         onAction={props.onAction}
-        size={props.size}>
+        size={props.menuSize}>
         {/* @ts-ignore TODO: fix type, right now this component is the same as Menu */}
         {props.children}
       </Menu>

packages/@react-spectrum/s2/src/Avatar.tsx

@@ -15,6 +15,7 @@ import {createContext, forwardRef} from 'react';
 import {DOMProps, DOMRef, DOMRefValue} from '@react-types/shared';
 import {filterDOMProps} from '@react-aria/utils';
 import {getAllowedOverrides, StylesPropWithoutWidth, UnsafeStyles} from './style-utils' with {type: 'macro'};
+import {Image} from './Image';
 import {style} from '../style/spectrum-theme' with { type: 'macro' };
 import {useDOMRef} from '@react-spectrum/utils';
 import {useSpectrumContextProps} from './useSpectrumContextProps';
@@ -71,16 +72,17 @@ function Avatar(props: AvatarProps, ref: DOMRef<HTMLImageElement>) {
   let remSize = size / 16 + 'rem';
   let isLarge = size >= 64;
   return (
-    <img
+    <Image
       {...domProps}
       ref={domRef}
       alt={alt}
-      style={{
+      UNSAFE_style={{
         ...UNSAFE_style,
         width: remSize,
         height: remSize
       }}
-      className={(UNSAFE_className ?? '') + imageStyles({isOverBackground, isLarge}, props.styles)}
+      UNSAFE_className={UNSAFE_className}
+      styles={imageStyles({isOverBackground, isLarge}, props.styles)}
       src={src} />
   );
 }

packages/@react-spectrum/s2/src/Badge.tsx

@@ -18,20 +18,21 @@ import {filterDOMProps} from '@react-aria/utils';
 import {fontRelative, style} from '../style/spectrum-theme' with {type: 'macro'};
 import {IconContext} from './Icon';
 import React, {createContext, forwardRef, ReactNode} from 'react';
+import {SkeletonWrapper} from './Skeleton';
 import {Text, TextContext} from './Content';
 import {useDOMRef} from '@react-spectrum/utils';
 import {useSpectrumContextProps} from './useSpectrumContextProps';
 
 export interface BadgeStyleProps {
   /**
    * The size of the badge.
-   *
+   * 
    * @default 'S'
    */
   size?: 'S' | 'M' | 'L' | 'XL',
   /**
    * The variant changes the background color of the badge. When badge has a semantic meaning, they should use the variant for semantic colors.
-   *
+   * 
    * @default 'neutral'
    */
   variant?: 'accent' | 'informative' | 'neutral' | 'positive' | 'notice' | 'negative' | 'gray' | 'red' | 'orange' | 'yellow' | 'charteuse' | 'celery' | 'green' | 'seafoam' | 'cyan' | 'blue' | 'indigo' | 'purple' | 'fuchsia' | 'magenta' | 'pink' | 'turquoise' | 'brown' | 'cinnamon' | 'silver',
@@ -201,17 +202,20 @@ function Badge(props: BadgeProps, ref: DOMRef<HTMLDivElement>) {
           styles: style({size: fontRelative(20), marginStart: '--iconMargin', flexShrink: 0})
         }]
       ]}>
-      <span
-        {...filterDOMProps(otherProps)}
-        role="presentation"
-        className={(props.UNSAFE_className || '') + badge({variant, size, fillStyle}, props.styles)}
-        ref={domRef}>
-        {
-          typeof children === 'string' || isTextOnly
-            ? <Text>{children}</Text>
-            : children
-        }
-      </span>
+      <SkeletonWrapper>
+        <span
+          {...filterDOMProps(otherProps)}
+          role="presentation"
+          className={(props.UNSAFE_className || '') + badge({variant, size, fillStyle}, props.styles)}
+          style={props.UNSAFE_style}
+          ref={domRef}>
+          {
+            typeof children === 'string' || isTextOnly
+              ? <Text>{children}</Text>
+              : children
+          }
+        </span>
+      </SkeletonWrapper>
     </Provider>
   );
 }