From 970d29e7ce056d64c3da9a37bd3d156cd616a360 Mon Sep 17 00:00:00 2001 From: Yihui Liao <44729383+yihuiliao@users.noreply.github.com> Date: Wed, 19 Feb 2025 17:19:09 -0800 Subject: [PATCH 1/5] fix(s2): apply styling to cardview when using an action bar --- packages/@react-spectrum/s2/src/CardView.tsx | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/@react-spectrum/s2/src/CardView.tsx b/packages/@react-spectrum/s2/src/CardView.tsx index 7dc90d37b31..1aee50c4634 100644 --- a/packages/@react-spectrum/s2/src/CardView.tsx +++ b/packages/@react-spectrum/s2/src/CardView.tsx @@ -25,6 +25,7 @@ import {focusRing, style} from '../style' with {type: 'macro'}; import {getAllowedOverrides, StylesPropWithHeight, UnsafeStyles} from './style-utils' with {type: 'macro'}; import {ImageCoordinator} from './ImageCoordinator'; import {InvalidationContext, Layout, LayoutInfo, Rect, Size} from '@react-stately/virtualizer'; +import {mergeStyles} from '../style/runtime'; import {useActionBarContainer} from './ActionBar'; import {useDOMRef} from '@react-spectrum/utils'; import {useEffectEvent, useLayoutEffect, useLoadMore, useResizeObserver} from '@react-aria/utils'; @@ -531,6 +532,12 @@ const cardViewStyles = style({ outlineOffset: -2 }, getAllowedOverrides({height: true})); +// let wrapper = style({ +// position: 'relative', +// overflow: 'clip', +// size: 'fit' +// }, getAllowedOverrides({height: true})) + export const CardViewContext = createContext>, DOMRefValue>>(null); export const CardView = /*#__PURE__*/ (forwardRef as forwardRefType)(function CardView(props: CardViewProps, ref: DOMRef) { @@ -616,7 +623,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 ( -
+
{cardView} {actionBar}
From c8edfb4ef2ef405cd128061d1c248a70e6386942 Mon Sep 17 00:00:00 2001 From: Yihui Liao <44729383+yihuiliao@users.noreply.github.com> Date: Wed, 19 Feb 2025 17:25:59 -0800 Subject: [PATCH 2/5] remove comment --- packages/@react-spectrum/s2/src/CardView.tsx | 6 ------ 1 file changed, 6 deletions(-) diff --git a/packages/@react-spectrum/s2/src/CardView.tsx b/packages/@react-spectrum/s2/src/CardView.tsx index 1aee50c4634..2c8c345b79e 100644 --- a/packages/@react-spectrum/s2/src/CardView.tsx +++ b/packages/@react-spectrum/s2/src/CardView.tsx @@ -532,12 +532,6 @@ const cardViewStyles = style({ outlineOffset: -2 }, getAllowedOverrides({height: true})); -// let wrapper = style({ -// position: 'relative', -// overflow: 'clip', -// size: 'fit' -// }, getAllowedOverrides({height: true})) - export const CardViewContext = createContext>, DOMRefValue>>(null); export const CardView = /*#__PURE__*/ (forwardRef as forwardRefType)(function CardView(props: CardViewProps, ref: DOMRef) { From bc3567526cae8d9e2bb07ac7d763131ef038fd79 Mon Sep 17 00:00:00 2001 From: Yihui Liao <44729383+yihuiliao@users.noreply.github.com> Date: Fri, 21 Feb 2025 12:46:54 -0800 Subject: [PATCH 3/5] don't apply styles twice --- packages/@react-spectrum/s2/src/CardView.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@react-spectrum/s2/src/CardView.tsx b/packages/@react-spectrum/s2/src/CardView.tsx index 8a6768c32be..5c980863621 100644 --- a/packages/@react-spectrum/s2/src/CardView.tsx +++ b/packages/@react-spectrum/s2/src/CardView.tsx @@ -243,14 +243,14 @@ export const CardView = /*#__PURE__*/ (forwardRef as forwardRefType)(function Ca defaultSelectedKeys={undefined} onSelectionChange={onSelectionChange} style={{ - ...UNSAFE_style, + ...(!props.renderActionBar ? UNSAFE_style : {}), // 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'}) : '')}> {children} From 3eed5d8ae15fe95e1c706fd07fef594d88701b9c Mon Sep 17 00:00:00 2001 From: Yihui Liao <44729383+yihuiliao@users.noreply.github.com> Date: Fri, 21 Feb 2025 14:38:05 -0800 Subject: [PATCH 4/5] add styles back in --- packages/@react-spectrum/s2/src/CardView.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-spectrum/s2/src/CardView.tsx b/packages/@react-spectrum/s2/src/CardView.tsx index 5c980863621..fdfa6127c28 100644 --- a/packages/@react-spectrum/s2/src/CardView.tsx +++ b/packages/@react-spectrum/s2/src/CardView.tsx @@ -250,7 +250,7 @@ export const CardView = /*#__PURE__*/ (forwardRef as forwardRefType)(function Ca scrollPadding: options.minSpace.height, scrollPaddingBottom: actionBarHeight + options.minSpace.height }} - className={renderProps => (!props.renderActionBar ? UNSAFE_className + cardViewStyles({...renderProps, isLoading: props.loadingState === 'loading'}) : '')}> + className={renderProps => (!props.renderActionBar ? UNSAFE_className + cardViewStyles({...renderProps, isLoading: props.loadingState === 'loading'}, styles) : '')}> {children} From 7494911fa5bbc7ffbd4603747c38bec9c8fcc652 Mon Sep 17 00:00:00 2001 From: Yihui Liao <44729383+yihuiliao@users.noreply.github.com> Date: Wed, 26 Feb 2025 15:45:28 -0800 Subject: [PATCH 5/5] fixes from review --- packages/@react-spectrum/s2/src/CardView.tsx | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/@react-spectrum/s2/src/CardView.tsx b/packages/@react-spectrum/s2/src/CardView.tsx index fdfa6127c28..208166342a7 100644 --- a/packages/@react-spectrum/s2/src/CardView.tsx +++ b/packages/@react-spectrum/s2/src/CardView.tsx @@ -25,7 +25,6 @@ import {DOMRef, DOMRefValue, forwardRefType, Key, LoadingState} from '@react-typ import {focusRing, style} from '../style' with {type: 'macro'}; import {getAllowedOverrides, StylesPropWithHeight, UnsafeStyles} from './style-utils' with {type: 'macro'}; import {ImageCoordinator} from './ImageCoordinator'; -import {mergeStyles} from '../style/runtime'; import {Size} from '@react-stately/virtualizer'; import {useActionBarContainer} from './ActionBar'; import {useDOMRef} from '@react-spectrum/utils'; @@ -180,6 +179,12 @@ const cardViewStyles = style({ outlineOffset: -2 }, getAllowedOverrides({height: true})); +const wrapperStyles = style({ + position: 'relative', + overflow: 'clip', + size: 'fit' +}, getAllowedOverrides({height: true})); + export const CardViewContext = createContext>, DOMRefValue>>(null); export const CardView = /*#__PURE__*/ (forwardRef as forwardRefType)(function CardView(props: CardViewProps, ref: DOMRef) { @@ -250,7 +255,7 @@ export const CardView = /*#__PURE__*/ (forwardRef as forwardRefType)(function Ca scrollPadding: options.minSpace.height, scrollPaddingBottom: actionBarHeight + options.minSpace.height }} - 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'}, !props.renderActionBar ? styles : undefined)}> {children} @@ -263,7 +268,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 ( -
+
{cardView} {actionBar}