More cleanup and docs

Author: devongovett

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

@@ -10,19 +10,43 @@ import {useLayoutEffect} from '@react-aria/utils';
 import {useSpectrumContextProps} from './useSpectrumContextProps';
 
 export interface ImageProps extends UnsafeStyles, SlotProps {
+  /** The URL of the image. */
   src?: string,
   // TODO
   // srcSet?: string,
   // sizes?: string,
+  /** Accessible alt text for the image. */
   alt?: string,
+  /**
+   * Indicates if the fetching of the image must be done using a CORS request.
+   * [See MDN](https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/crossorigin).
+   */
   crossOrigin?: 'anonymous' | 'use-credentials',
+  /**
+   * Whether the browser should decode images synchronously or asynchronously.
+   * [See MDN](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/img#decoding).
+   */
   decoding?: 'async' | 'auto' | 'sync',
   // Only supported in React 19...
   // fetchPriority?: 'high' | 'low' | 'auto',
+  /**
+   * Whether the image should be loaded immediately or lazily when scrolled into view.
+   * [See MDN](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/img#loading).
+   */
   loading?: 'eager' | 'lazy',
+  /**
+   * A string indicating which referrer to use when fetching the resource.
+   * [See MDN](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/img#referrerpolicy).
+   */
   referrerPolicy?: HTMLAttributeReferrerPolicy,
+  /** Spectrum-defined styles, returned by the `style()` macro. */
   styles?: StyleString,
+  /** A function that is called to render a fallback when the image fails to load. */
   renderError?: () => ReactNode,
+  /**
+   * A group of images to coordinate between, matching the group passed to the `<ImageCoordinator>` component.
+   * If not provided, the default image group is used.
+   */
   group?: ImageGroup
 }
 
@@ -84,11 +108,28 @@ function reducer(state: State, action: Action): State {
   }
 }
 
-const imageStyles = style({
+const wrapperStyles = style({
   backgroundColor: 'gray-100',
   overflow: 'hidden'
 });
 
+const imgStyles = style({
+  display: 'block',
+  width: 'full',
+  height: 'full',
+  objectFit: '[inherit]',
+  objectPosition: '[inherit]',
+  opacity: {
+    default: 0,
+    isRevealed: 1
+  },
+  transition: {
+    default: 'none',
+    isTransitioning: 'opacity'
+  },
+  transitionDuration: 500
+});
+
 function Image(props: ImageProps, domRef: ForwardedRef<HTMLDivElement>) {
   [props, domRef] = useSpectrumContextProps(props, domRef, ImageContext);
 
@@ -171,12 +212,12 @@ function Image(props: ImageProps, domRef: ForwardedRef<HTMLDivElement>) {
 
   let errorState = !isSkeleton && state === 'error' && renderError?.();
   let isRevealed = state === 'revealed' && !isSkeleton;
-  let transition = isRevealed && loadTime > 200 ? 'opacity 500ms' : undefined;
+  let isTransitioning = isRevealed && loadTime > 200;
   return useMemo(() => hidden ? null : (
     <div
       ref={domRef}
       style={UNSAFE_style}
-      className={UNSAFE_className + mergeStyles(imageStyles, styles) + ' '  + (isAnimating ? loadingStyle : '')}>
+      className={UNSAFE_className + mergeStyles(wrapperStyles, styles) + ' '  + (isAnimating ? loadingStyle : '')}>
       {errorState}
       {!errorState && (
         <img
@@ -189,18 +230,10 @@ function Image(props: ImageProps, domRef: ForwardedRef<HTMLDivElement>) {
           ref={imgRef}
           onLoad={onLoad}
           onError={onError}
-          style={{
-            display: 'block',
-            width: '100%',
-            height: '100%',
-            objectFit: 'inherit',
-            objectPosition: 'inherit',
-            opacity: isRevealed ? 1 : 0,
-            transition
-          }} />
+          className={imgStyles({isRevealed, isTransitioning})} />
         )}
     </div>
-  ), [hidden, domRef, UNSAFE_style, UNSAFE_className, styles, isAnimating, errorState, src, alt, crossOrigin, decoding, loading, referrerPolicy, onLoad, onError, isRevealed, transition]);
+  ), [hidden, domRef, UNSAFE_style, UNSAFE_className, styles, isAnimating, errorState, src, alt, crossOrigin, decoding, loading, referrerPolicy, onLoad, onError, isRevealed, isTransitioning]);
 }
 
 const _Image = forwardRef(Image);

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

@@ -1,8 +1,17 @@
 import {Context, createContext, ReactNode, useCallback, useContext, useEffect, useMemo, useReducer} from 'react';
 
 export interface ImageCoordinatorProps {
+  /** Children within the ImageCoordinator. */
   children: ReactNode,
+  /**
+   * Time in milliseconds after which images are always displayed, even if all images are not yet loaded.
+   * @default 5000
+   */
   timeout?: number,
+  /**
+   * A group of images to coordinate between, matching the group passed to the `<Image>` component.
+   * If not provided, the default image group is used.
+   */
   group?: ImageGroup
 }
 
@@ -107,6 +116,10 @@ function isAllLoaded(loaded: Map<string, boolean>) {
   return true;
 }
 
+/**
+ * An ImageCoordinator coordinates loading behavior for a group of images.
+ * Images within an ImageCoordinator are revealed together once all of them have loaded.
+ */
 export function ImageCoordinator(props: ImageCoordinatorProps) {
   // If we are already inside another ImageCoordinator, just pass
   // through children and coordinate loading at the root.

packages/@react-spectrum/s2/src/style-utils.ts

@@ -188,7 +188,7 @@ const heightProperties = [
 ] as const;
 
 export type StylesProp = StyleString<(typeof allowedOverrides)[number] | (typeof widthProperties)[number]>;
-export type StylesPropWithHeight = StyleString<(typeof allowedOverrides)[number] | (typeof heightProperties)[number]>;
+export type StylesPropWithHeight = StyleString<(typeof allowedOverrides)[number] | (typeof widthProperties)[number] | (typeof heightProperties)[number]>;
 export type StylesPropWithoutWidth = StyleString<(typeof allowedOverrides)[number]>;
 export interface UnsafeStyles {
   /** Sets the CSS [className](https://developer.mozilla.org/en-US/docs/Web/API/Element/className) for the element. Only use as a **last resort**. Use the `style` macro via the `styles` prop instead. */

packages/@react-spectrum/s2/stories/CardView.stories.tsx

@@ -28,6 +28,21 @@ const meta: Meta<typeof CardView> = {
 
 export default meta;
 
+const cardViewStyles = style({
+  width: {
+    default: 'screen',
+    viewMode: {
+      docs: 'full'
+    }
+  },
+  height: {
+    default: 'screen',
+    viewMode: {
+      docs: '[600px]'
+    }
+  }
+});
+
 type Item = {
   id: number,
   user: {
@@ -51,6 +66,7 @@ function PhotoCard({item, layout}: {item: Item, layout: string}) {
             width: 'full',
             pointerEvents: 'none'
           })}
+          // TODO - should we have a safe `dynamicStyles` or something for this?
           UNSAFE_style={{
             aspectRatio: layout === 'waterfall' ? `${item.width} / ${item.height}` : '4/3',
             objectFit: layout === 'waterfall' ? 'contain' : 'cover'
@@ -96,11 +112,11 @@ export const Example = (args: CardViewProps<any>, {viewMode}) => {
 
   return (
     <CardView
-      aria-label="Assets"
+      aria-label="Nature photos"
       {...args}
       loadingState={loadingState}
       onLoadMore={args.loadingState === 'idle' ? list.loadMore : undefined}
-      UNSAFE_style={{height: viewMode === 'docs' ? 600 : '100vh', width: viewMode === 'docs' ? '100%' : '100vw'}}>
+      styles={cardViewStyles({viewMode})}>
       <Collection items={items} dependencies={[args.layout]}>
         {item => <PhotoCard item={item} layout={args.layout || 'grid'} />}
       </Collection>
@@ -136,7 +152,7 @@ export const Empty = (args: CardViewProps<any>, {viewMode}) => {
     <CardView 
       aria-label="Assets"
       {...args}
-      UNSAFE_style={{height: viewMode === 'docs' ? 600 : '100vh', width: viewMode === 'docs' ? '100%' : '100vw'}}
+      styles={cardViewStyles({viewMode})}
       renderEmptyState={() => (
         <IllustratedMessage size="L">
           <EmptyIcon />
@@ -194,12 +210,12 @@ export const CollectionCards = (args: CardViewProps<any>, {viewMode}) => {
 
   return (
     <CardView
-      aria-label="Assets"
+      aria-label="Topics"
       {...args}
       loadingState={loadingState}
       onLoadMore={args.loadingState === 'idle' ? list.loadMore : undefined}
-      UNSAFE_style={{height: viewMode === 'docs' ? 600 : '100vh', width: viewMode === 'docs' ? '100%' : '100vw'}}>
-      <Collection items={items} dependencies={[args.layout]}>
+      styles={cardViewStyles({viewMode})}>
+      <Collection items={items}>
         {topic => <TopicCard topic={topic} />}
       </Collection>
       {(loadingState === 'loading' || loadingState === 'loadingMore') && (

packages/@react-spectrum/s2/style/spectrum-theme.ts

@@ -215,7 +215,6 @@ const sizing = {
   ...scaledSpacing,
   auto: 'auto',
   full: '100%',
-  screen: '100vh',
   min: 'min-content',
   max: 'max-content',
   fit: 'fit-content',
@@ -241,6 +240,16 @@ const sizing = {
   }
 };
 
+const height = {
+  ...sizing,
+  screen: '100vh'
+};
+
+const width = {
+  ...sizing,
+  screen: '100vw'
+};
+
 const margin = {
   ...spacing,
   ...negativeSpacing,
@@ -576,18 +585,18 @@ export const style = createTheme({
     },
     rowGap: spacing,
     columnGap: spacing,
-    height: sizing,
-    width: sizing,
-    containIntrinsicWidth: sizing,
-    containIntrinsicHeight: sizing,
-    minHeight: sizing,
+    height,
+    width,
+    containIntrinsicWidth: width,
+    containIntrinsicHeight: height,
+    minHeight: height,
     maxHeight: {
-      ...sizing,
+      ...height,
       none: 'none'
     },
-    minWidth: sizing,
+    minWidth: width,
     maxWidth: {
-      ...sizing,
+      ...width,
       none: 'none'
     },
     borderStartWidth: createRenamedProperty('borderInlineStartWidth', borderWidth),

packages/@react-spectrum/s2/style/style-macro.ts

@@ -224,9 +224,6 @@ export function createTheme<T extends Theme>(theme: T): StyleFunction<ThemePrope
       for (let property of allowedOverrides) {
         if (themePropertyMap.has(property as string)) {
           allowedOverridesSet.add(themePropertyMap.get(property as string)!);
-          // Merge custom properties for self() references too.
-          // TODO: optimize this
-          allowedOverridesSet.add(generateArbitraryValueSelector(`--${themePropertyMap.get(property)}`, true) + '_' + themePropertyMap.get(property as string)!);
         }
       }