fix(s2): apply styling to CardView when using an action bar #7795

yihuiliao · 2025-02-20T01:20:46Z

Closes #7684

✅ Pull Request Checklist:

Included link to corresponding React Spectrum GitHub Issue.
Added/updated unit tests and storybook for this change (for new code or code which already has tests).
Filled out test instructions.
Updated documentation (if it already exists for this component).
Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

🧢 Your Project:

rspbot · 2025-02-20T01:29:18Z

Build successful! 🎉

rspbot · 2025-02-20T01:36:44Z

Build successful! 🎉

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

rspbot · 2025-02-21T19:28:55Z

Build successful! 🎉

rspbot · 2025-02-21T20:57:58Z

Build successful! 🎉

rspbot · 2025-02-21T22:48:46Z

Build successful! 🎉

rspbot · 2025-02-21T22:48:48Z

## API Changes

react-aria-components

/react-aria-components:GridList

 GridList <T extends {}> {
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   autoFocus?: boolean | FocusStrategy
   children?: ReactNode | ({}) => ReactNode
   className?: string | ((GridListRenderProps & {
     defaultClassName: string | undefined
 })) => string
   defaultSelectedKeys?: 'all' | Iterable<Key>
   dependencies?: ReadonlyArray<any>
   disabledBehavior?: DisabledBehavior
   disabledKeys?: Iterable<Key>
   disallowEmptySelection?: boolean
-  disallowTypeAhead?: boolean = false
   dragAndDropHooks?: DragAndDropHooks
   id?: string
   items?: Iterable<T>
   keyboardNavigationBehavior?: 'arrow' | 'tab' = 'arrow'
   onAction?: (Key) => void
   onScroll?: (UIEvent<Element>) => void
   onSelectionChange?: (Selection) => void
   renderEmptyState?: (GridListRenderProps) => ReactNode
   selectedKeys?: 'all' | Iterable<Key>
   selectionBehavior?: SelectionBehavior
   selectionMode?: SelectionMode
   slot?: string | null
   style?: CSSProperties | ((GridListRenderProps & {
     defaultStyle: CSSProperties
 })) => CSSProperties | undefined
 }

/react-aria-components:UNSTABLE_Toast

-UNSTABLE_Toast <T> {
-  aria-describedby?: string
-  aria-details?: string
-  aria-label?: string
-  aria-labelledby?: string
-  children?: ReactNode | ((ToastRenderProps<T> & {
-    defaultChildren: ReactNode | undefined
-})) => ReactNode
-  className?: string | ((ToastRenderProps<T> & {
-    defaultClassName: string | undefined
-})) => string
-  style?: CSSProperties | ((ToastRenderProps<T> & {
-    defaultStyle: CSSProperties
-})) => CSSProperties | undefined
-  toast: QueuedToast<T>
-}

/react-aria-components:UNSTABLE_ToastRegion

-UNSTABLE_ToastRegion <T> {
-  aria-describedby?: string
-  aria-details?: string
-  aria-label?: string = "Notifications"
-  aria-labelledby?: string
-  children: ({
-    toast: QueuedToast<T>
-}) => ReactElement
-  className?: string | ((ToastRegionRenderProps<T> & {
-    defaultClassName: string | undefined
-})) => string
-  queue: ToastQueue<T>
-  style?: CSSProperties | ((ToastRegionRenderProps<T> & {
-    defaultStyle: CSSProperties
-})) => CSSProperties | undefined
-}

/react-aria-components:UNSTABLE_ToastContent

-UNSTABLE_ToastContent {
-  UNTYPED
-}

/react-aria-components:UNSTABLE_ToastQueue

-UNSTABLE_ToastQueue <T> {
-  add: (T, ToastOptions) => void
-  close: (string) => void
-  constructor: (ToastStateProps) => void
-  pauseAll: () => void
-  resumeAll: () => void
-  subscribe: (() => void) => void
-  visibleToasts: Array<QueuedToast<T>>
-}

/react-aria-components:GridListProps

 GridListProps <T> {
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   autoFocus?: boolean | FocusStrategy
   children?: ReactNode | (T) => ReactNode
   className?: string | ((GridListRenderProps & {
     defaultClassName: string | undefined
 })) => string
   defaultSelectedKeys?: 'all' | Iterable<Key>
   dependencies?: ReadonlyArray<any>
   disabledBehavior?: DisabledBehavior
   disabledKeys?: Iterable<Key>
   disallowEmptySelection?: boolean
-  disallowTypeAhead?: boolean = false
   dragAndDropHooks?: DragAndDropHooks
   id?: string
   items?: Iterable<T>
   keyboardNavigationBehavior?: 'arrow' | 'tab' = 'arrow'
   onAction?: (Key) => void
   onScroll?: (UIEvent<Element>) => void
   onSelectionChange?: (Selection) => void
   renderEmptyState?: (GridListRenderProps) => ReactNode
   selectedKeys?: 'all' | Iterable<Key>
   selectionBehavior?: SelectionBehavior
   selectionMode?: SelectionMode
   slot?: string | null
   style?: CSSProperties | ((GridListRenderProps & {
     defaultStyle: CSSProperties
 })) => CSSProperties | undefined
 }

/react-aria-components:ToastRegionProps

-ToastRegionProps <T> {
-  aria-describedby?: string
-  aria-details?: string
-  aria-label?: string = "Notifications"
-  aria-labelledby?: string
-  children: ({
-    toast: QueuedToast<T>
-}) => ReactElement
-  className?: string | ((ToastRegionRenderProps<T> & {
-    defaultClassName: string | undefined
-})) => string
-  queue: ToastQueue<T>
-  style?: CSSProperties | ((ToastRegionRenderProps<T> & {
-    defaultStyle: CSSProperties
-})) => CSSProperties | undefined
-}

/react-aria-components:ToastRegionRenderProps

-ToastRegionRenderProps <T> {
-  isFocusVisible: boolean
-  isFocused: boolean
-  visibleToasts: Array<QueuedToast<T>>
-}

/react-aria-components:ToastProps

-ToastProps <T> {
-  aria-describedby?: string
-  aria-details?: string
-  aria-label?: string
-  aria-labelledby?: string
-  children?: ReactNode | ((ToastRenderProps<T> & {
-    defaultChildren: ReactNode | undefined
-})) => ReactNode
-  className?: string | ((ToastRenderProps<T> & {
-    defaultClassName: string | undefined
-})) => string
-  style?: CSSProperties | ((ToastRenderProps<T> & {
-    defaultStyle: CSSProperties
-})) => CSSProperties | undefined
-  toast: QueuedToast<T>
-}

/react-aria-components:ToastRenderProps

-ToastRenderProps <T> {
-  isFocusVisible: boolean
-  isFocused: boolean
-  toast: QueuedToast<T>
-}

/react-aria-components:ToastOptions

-ToastOptions {
-  onClose?: () => void
-  timeout?: number
-}

@react-aria/grid

/@react-aria/grid:GridProps

 GridProps {
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
-  disallowTypeAhead?: boolean = false
   focusMode?: 'row' | 'cell' = 'row'
   getRowText?: (Key) => string = (key) => state.collection.getItem(key)?.textValue
   id?: string
   isVirtualized?: boolean
   onCellAction?: (Key) => void
   onRowAction?: (Key) => void
   scrollRef?: RefObject<HTMLElement | null>
 }

@react-aria/gridlist

/@react-aria/gridlist:AriaGridListOptions

 AriaGridListOptions <T> {
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   autoFocus?: boolean | FocusStrategy
   defaultSelectedKeys?: 'all' | Iterable<Key>
   disabledBehavior?: DisabledBehavior
   disabledKeys?: Iterable<Key>
   disallowEmptySelection?: boolean
-  disallowTypeAhead?: boolean = false
   id?: string
   isVirtualized?: boolean
   items?: Iterable<T>
   keyboardDelegate?: KeyboardDelegate
   layoutDelegate?: LayoutDelegate
   linkBehavior?: 'action' | 'selection' | 'override' = 'action'
   onAction?: (Key) => void
   onSelectionChange?: (Selection) => void
   selectedKeys?: 'all' | Iterable<Key>
   selectionMode?: SelectionMode
   shouldFocusWrap?: boolean = false
 }

@react-aria/table

/@react-aria/table:AriaTableProps

 AriaTableProps {
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
-  disallowTypeAhead?: boolean = false
   focusMode?: 'row' | 'cell' = 'row'
   getRowText?: (Key) => string = (key) => state.collection.getItem(key)?.textValue
   id?: string
   isVirtualized?: boolean
   layoutDelegate?: LayoutDelegate
   onCellAction?: (Key) => void
   onRowAction?: (Key) => void
   scrollRef?: RefObject<HTMLElement | null>
 }

@react-aria/tree

/@react-aria/tree:AriaTreeOptions

 AriaTreeOptions <T> {
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   autoFocus?: boolean | FocusStrategy
   defaultSelectedKeys?: 'all' | Iterable<Key>
   disabledBehavior?: DisabledBehavior
   disabledKeys?: Iterable<Key>
   disallowEmptySelection?: boolean
-  disallowTypeAhead?: boolean = false
   id?: string
   isVirtualized?: boolean
   items?: Iterable<T>
   keyboardDelegate?: KeyboardDelegate
   layoutDelegate?: LayoutDelegate
   linkBehavior?: 'action' | 'selection' | 'override' = 'action'
   onAction?: (Key) => void
   onSelectionChange?: (Selection) => void
   selectedKeys?: 'all' | Iterable<Key>
   selectionMode?: SelectionMode
 }

@react-spectrum/s2

/@react-spectrum/s2:CardView

 CardView <T extends {}> {
   UNSAFE_className?: string
   UNSAFE_style?: CSSProperties
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   autoFocus?: boolean | FocusStrategy
   children?: ReactNode | (T) => ReactNode
   defaultSelectedKeys?: 'all' | Iterable<Key>
   density?: 'compact' | 'regular' | 'spacious' = 'regular'
   dependencies?: ReadonlyArray<any>
   disabledBehavior?: DisabledBehavior
   disabledKeys?: Iterable<Key>
   disallowEmptySelection?: boolean
-  disallowTypeAhead?: boolean = false
   dragAndDropHooks?: DragAndDropHooks
   id?: string
   items?: Iterable<T>
   layout?: 'grid' | 'waterfall' = 'grid'
   onAction?: (Key) => void
   onLoadMore?: () => void
   onScroll?: (UIEvent<Element>) => void
   onSelectionChange?: (Selection) => void
   renderActionBar?: ('all' | Set<Key>) => ReactElement
   renderEmptyState?: (GridListRenderProps) => ReactNode
   selectedKeys?: 'all' | Iterable<Key>
   selectionMode?: SelectionMode
   selectionStyle?: 'checkbox' | 'highlight' = 'checkbox'
   size?: 'XS' | 'S' | 'M' | 'L' | 'XL' = 'M'
   slot?: string | null
   styles?: StylesPropWithHeight
   variant?: 'primary' | 'secondary' | 'tertiary' | 'quiet' = 'primary'
 }

/@react-spectrum/s2:CardViewProps

 CardViewProps <T> {
   UNSAFE_className?: string
   UNSAFE_style?: CSSProperties
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   autoFocus?: boolean | FocusStrategy
   children?: ReactNode | (T) => ReactNode
   defaultSelectedKeys?: 'all' | Iterable<Key>
   density?: 'compact' | 'regular' | 'spacious' = 'regular'
   dependencies?: ReadonlyArray<any>
   disabledBehavior?: DisabledBehavior
   disabledKeys?: Iterable<Key>
   disallowEmptySelection?: boolean
-  disallowTypeAhead?: boolean = false
   dragAndDropHooks?: DragAndDropHooks
   id?: string
   items?: Iterable<T>
   layout?: 'grid' | 'waterfall' = 'grid'
   onAction?: (Key) => void
   onLoadMore?: () => void
   onScroll?: (UIEvent<Element>) => void
   onSelectionChange?: (Selection) => void
   renderActionBar?: ('all' | Set<Key>) => ReactElement
   renderEmptyState?: (GridListRenderProps) => ReactNode
   selectedKeys?: 'all' | Iterable<Key>
   selectionMode?: SelectionMode
   selectionStyle?: 'checkbox' | 'highlight' = 'checkbox'
   size?: 'XS' | 'S' | 'M' | 'L' | 'XL' = 'M'
   slot?: string | null
   styles?: StylesPropWithHeight
   variant?: 'primary' | 'secondary' | 'tertiary' | 'quiet' = 'primary'
 }

@react-stately/toast

/@react-stately/toast:QueuedToast

 QueuedToast <T> {
   content: T
   key: string
   onClose?: () => void
+  priority?: number
   timeout?: number
   timer?: Timer
 }

/@react-stately/toast:ToastStateProps

 ToastStateProps {
   maxVisibleToasts?: number
-  wrapUpdate?: (() => void) => void
+  wrapUpdate?: (() => R) => R
 }

/@react-stately/toast:ToastOptions

 ToastOptions {
   onClose?: () => void
+  priority?: number
   timeout?: number
 }

snowystinger · 2025-02-25T23:10:37Z

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

                // Add padding at the bottom when the action bar is visible so users can scroll to the last items.
                // Also add scroll padding so keyboard navigating preserves the padding.
                paddingBottom: actionBarHeight > 0 ? actionBarHeight + options.minSpace.height : 0,
                scrollPadding: options.minSpace.height,
                scrollPaddingBottom: actionBarHeight + options.minSpace.height
              }}
-              className={renderProps => UNSAFE_className + cardViewStyles({...renderProps, isLoading: props.loadingState === 'loading'}, styles)}>
+              className={renderProps => (!props.renderActionBar ? UNSAFE_className + cardViewStyles({...renderProps, isLoading: props.loadingState === 'loading'}, styles) : '')}>


I'm not sure this is right, I think you still want cardViewStyles with all the states it comes with

Suggested change

className={renderProps => (!props.renderActionBar ? UNSAFE_className + cardViewStyles({...renderProps, isLoading: props.loadingState === 'loading'}, styles) : '')}>

className={renderProps => (!props.renderActionBar ? UNSAFE_className + cardViewStyles({...renderProps, isLoading: props.loadingState === 'loading'}, styles) : cardViewStyles({...renderProps, isLoading: props.loadingState === 'loading'}))}>

but you don't want either of the ways to overrides styles as provided by the user

snowystinger · 2025-02-25T23:12:40Z

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

@@ -262,7 +263,7 @@ export const CardView = /*#__PURE__*/ (forwardRef as forwardRefType)(function Ca
  // ActionBar cannot be inside the GridList due to ARIA and focus management requirements.
  if (props.renderActionBar) {
    return (
-      <div ref={domRef} className={style({position: 'relative', overflow: 'clip', size: 'fit'})}>
+      <div ref={domRef} className={UNSAFE_className + mergeStyles(style({position: 'relative', overflow: 'clip', size: 'fit'}), styles)} style={UNSAFE_style}>


I think you still want to do a safe merge of styles with allowed overrides, so you may want to pull out this style like the other overridable ones style(..., getAllowedOverrides({height: true}))

fix(s2): apply styling to cardview when using an action bar

970d29e

yihuiliao marked this pull request as ready for review February 20, 2025 01:20

remove comment

c8edfb4

snowystinger reviewed Feb 20, 2025

View reviewed changes

packages/@react-spectrum/s2/src/CardView.tsx Show resolved Hide resolved

Merge branch 'main' into style-cardview

c6118e3

don't apply styles twice

bc35675

add styles back in

3eed5d8

yihuiliao added the release label Feb 25, 2025

snowystinger reviewed Feb 25, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(s2): apply styling to CardView when using an action bar #7795

fix(s2): apply styling to CardView when using an action bar #7795

yihuiliao commented Feb 20, 2025

rspbot commented Feb 20, 2025

rspbot commented Feb 20, 2025

rspbot commented Feb 21, 2025

rspbot commented Feb 21, 2025

rspbot commented Feb 21, 2025

rspbot commented Feb 21, 2025

snowystinger Feb 25, 2025

snowystinger Feb 25, 2025

	className={renderProps => (!props.renderActionBar ? UNSAFE_className + cardViewStyles({...renderProps, isLoading: props.loadingState === 'loading'}, styles) : '')}>
	className={renderProps => (!props.renderActionBar ? UNSAFE_className + cardViewStyles({...renderProps, isLoading: props.loadingState === 'loading'}, styles) : cardViewStyles({...renderProps, isLoading: props.loadingState === 'loading'}))}>

fix(s2): apply styling to CardView when using an action bar #7795

Are you sure you want to change the base?

fix(s2): apply styling to CardView when using an action bar #7795

Conversation

yihuiliao commented Feb 20, 2025

✅ Pull Request Checklist:

📝 Test Instructions:

🧢 Your Project:

rspbot commented Feb 20, 2025

rspbot commented Feb 20, 2025

rspbot commented Feb 21, 2025

rspbot commented Feb 21, 2025

rspbot commented Feb 21, 2025

rspbot commented Feb 21, 2025

react-aria-components

/react-aria-components:GridList

/react-aria-components:UNSTABLE_Toast

/react-aria-components:UNSTABLE_ToastRegion

/react-aria-components:UNSTABLE_ToastContent

/react-aria-components:UNSTABLE_ToastQueue

/react-aria-components:GridListProps

/react-aria-components:ToastRegionProps

/react-aria-components:ToastRegionRenderProps

/react-aria-components:ToastProps

/react-aria-components:ToastRenderProps

/react-aria-components:ToastOptions

@react-aria/grid

/@react-aria/grid:GridProps

@react-aria/gridlist

/@react-aria/gridlist:AriaGridListOptions

@react-aria/table

/@react-aria/table:AriaTableProps

@react-aria/tree

/@react-aria/tree:AriaTreeOptions

@react-spectrum/s2

/@react-spectrum/s2:CardView

/@react-spectrum/s2:CardViewProps

@react-stately/toast

/@react-stately/toast:QueuedToast

/@react-stately/toast:ToastStateProps

/@react-stately/toast:ToastOptions

snowystinger Feb 25, 2025

Choose a reason for hiding this comment

snowystinger Feb 25, 2025

Choose a reason for hiding this comment