From 601ee0613070664c94c159619d9cf968035b8cc7 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Tue, 6 May 2025 14:56:10 +1000 Subject: [PATCH 1/6] feat: Tag remove button focus ring --- .../components/button/index.css | 11 +++ .../button/src/ClearButton.tsx | 7 +- .../@react-spectrum/s2/src/ClearButton.tsx | 90 +++++++++++++------ packages/@react-spectrum/s2/src/TagGroup.tsx | 1 + .../s2/stories/TagGroup.stories.tsx | 36 +++++--- packages/@react-spectrum/tag/src/Tag.tsx | 4 +- .../react-aria-components/src/TagGroup.tsx | 2 +- 7 files changed, 106 insertions(+), 45 deletions(-) diff --git a/packages/@adobe/spectrum-css-temp/components/button/index.css b/packages/@adobe/spectrum-css-temp/components/button/index.css index 806ead8e29d..151c52dd452 100644 --- a/packages/@adobe/spectrum-css-temp/components/button/index.css +++ b/packages/@adobe/spectrum-css-temp/components/button/index.css @@ -372,6 +372,17 @@ a.spectrum-ActionButton { margin-block: 0; margin-inline: auto; } + + &.spectrum-ClearButton--inset { + &:focus-visible { + &:after { + left: 4px; + right: 4px; + bottom: 4px; + top: 4px; + } + } + } } @media screen and (-ms-high-contrast: active), (-ms-high-contrast: none) { diff --git a/packages/@react-spectrum/button/src/ClearButton.tsx b/packages/@react-spectrum/button/src/ClearButton.tsx index 08dc3bad77b..aed43c43639 100644 --- a/packages/@react-spectrum/button/src/ClearButton.tsx +++ b/packages/@react-spectrum/button/src/ClearButton.tsx @@ -25,7 +25,8 @@ interface ClearButtonProps extends ButtonProps focusClassName?: string, variant?: 'overBackground', excludeFromTabOrder?: boolean, - preventFocus?: boolean + preventFocus?: boolean, + inset?: boolean } export const ClearButton = React.forwardRef(function ClearButton(props: ClearButtonProps, ref: FocusableRef) { @@ -37,6 +38,7 @@ export const ClearButton = React.forwardRef(function ClearButton(props: ClearBut isDisabled, preventFocus, elementType = preventFocus ? 'div' : 'button' as ElementType, + inset = false, ...otherProps } = props; let domRef = useFocusableRef(ref); @@ -66,7 +68,8 @@ export const ClearButton = React.forwardRef(function ClearButton(props: ClearBut [`spectrum-ClearButton--${variant}`]: variant, 'is-disabled': isDisabled, 'is-active': isPressed, - 'is-hovered': isHovered + 'is-hovered': isHovered, + 'spectrum-ClearButton--inset': inset }, styleProps.className ) diff --git a/packages/@react-spectrum/s2/src/ClearButton.tsx b/packages/@react-spectrum/s2/src/ClearButton.tsx index 11c06ee674a..740d4ee4079 100644 --- a/packages/@react-spectrum/s2/src/ClearButton.tsx +++ b/packages/@react-spectrum/s2/src/ClearButton.tsx @@ -17,47 +17,83 @@ import { } from 'react-aria-components'; import CrossIcon from '../ui-icons/Cross'; import {FocusableRef} from '@react-types/shared'; +import {focusRing, style} from '../style' with {type: 'macro'}; import {forwardRef} from 'react'; -import {style} from '../style' with {type: 'macro'}; +import {pressScale} from './pressScale'; import {useFocusableRef} from '@react-spectrum/utils'; - interface ClearButtonStyleProps { /** * The size of the ClearButton. * * @default 'M' */ - size?: 'S' | 'M' | 'L' | 'XL' + size?: 'S' | 'M' | 'L' | 'XL', + /** Whether to show a focus ring. */ + showFocusRing?: boolean } interface ClearButtonRenderProps extends ButtonRenderProps, ClearButtonStyleProps {} interface ClearButtonProps extends ButtonProps, ClearButtonStyleProps {} +const visibleClearButton = style({ + ...focusRing(), + display: 'flex', + alignItems: 'center', + justifyContent: 'center', + height: 'full', + width: 'control', + flexShrink: 0, + borderRadius: 'full', + borderStyle: 'none', + backgroundColor: 'transparent', + boxSizing: 'border-box', + padding: 0, + outlineOffset: -4, + color: '[inherit]', + '--iconPrimary': { + type: 'fill', + value: 'currentColor' + } +}); + export const ClearButton = forwardRef(function ClearButton(props: ClearButtonProps, ref: FocusableRef) { + let {showFocusRing = false, size = 'M', ...rest} = props; let domRef = useFocusableRef(ref); - return ( - - ); + if (showFocusRing) { + return ( + + ); + } else { + return ( + + ); + } }); diff --git a/packages/@react-spectrum/s2/src/TagGroup.tsx b/packages/@react-spectrum/s2/src/TagGroup.tsx index b2aa19e19be..ee734c54782 100644 --- a/packages/@react-spectrum/s2/src/TagGroup.tsx +++ b/packages/@react-spectrum/s2/src/TagGroup.tsx @@ -585,6 +585,7 @@ function TagWrapper({children, isDisabled, allowsRemoving, isInRealDOM}) { {!isInRealDOM && children} {allowsRemoving && isInRealDOM && ( diff --git a/packages/@react-spectrum/s2/stories/TagGroup.stories.tsx b/packages/@react-spectrum/s2/stories/TagGroup.stories.tsx index 650ae340a56..89dc92f873e 100644 --- a/packages/@react-spectrum/s2/stories/TagGroup.stories.tsx +++ b/packages/@react-spectrum/s2/stories/TagGroup.stories.tsx @@ -50,12 +50,13 @@ export default meta; export let Example = { render: (args: any) => { + let props = {...args}; if (args.onRemove) { - args.onRemove = action('remove'); + props.onRemove = action('remove'); } return (
- + Chocolate Mint Strawberry @@ -102,12 +103,13 @@ let items: Array = [ ]; export let Dynamic = { render: (args: any) => { + let props = {...args}; if (args.onRemove) { - args.onRemove = action('remove'); + props.onRemove = action('remove'); } return (
- + {(item: ITagItem) => {item.name}}
@@ -125,12 +127,13 @@ const SRC_URL_1 = export let Disabled = { render: (args: any) => { + let props = {...args}; if (args.onRemove) { - args.onRemove = action('remove'); + props.onRemove = action('remove'); } return ( - + Chocolate Mint @@ -165,12 +168,13 @@ function renderEmptyState() { } export let Empty = { render: (args: any) => { + let props = {...args}; if (args.onRemove) { - args.onRemove = action('remove'); + props.onRemove = action('remove'); } return ( - + ); }, args: { @@ -179,12 +183,13 @@ export let Empty = { }; export let DefaultEmpty = { render: (args: any) => { + let props = {...args}; if (args.onRemove) { - args.onRemove = action('remove'); + props.onRemove = action('remove'); } return ( - + ); }, args: { @@ -194,8 +199,12 @@ export let DefaultEmpty = { export let Links = { render: (args: any) => { + let props = {...args}; + if (args.onRemove) { + props.onRemove = action('remove'); + } return ( - + Adobe Google Apple @@ -210,12 +219,13 @@ export let Links = { export const ContextualHelpExample = { render: (args: any) => { + let props = {...args}; if (args.onRemove) { - args.onRemove = action('remove'); + props.onRemove = action('remove'); } return ( What is a ice cream? diff --git a/packages/@react-spectrum/tag/src/Tag.tsx b/packages/@react-spectrum/tag/src/Tag.tsx index b42fe140ea9..9ce8af187e3 100644 --- a/packages/@react-spectrum/tag/src/Tag.tsx +++ b/packages/@react-spectrum/tag/src/Tag.tsx @@ -35,7 +35,7 @@ export function Tag(props: SpectrumTagProps): ReactNode { // @ts-ignore let {styleProps} = useStyleProps(otherProps); let {hoverProps, isHovered} = useHover({}); - let {isFocused, isFocusVisible, focusProps} = useFocusRing({within: true}); + let {isFocused, isFocusVisible, focusProps} = useFocusRing({within: false}); let ref = useRef(null); let {removeButtonProps, gridCellProps, rowProps, allowsRemoving} = useTag({ ...props, @@ -81,7 +81,7 @@ function TagRemoveButton(props) { return ( - + ); } diff --git a/packages/react-aria-components/src/TagGroup.tsx b/packages/react-aria-components/src/TagGroup.tsx index 0f7d1ff4827..eccd36bd58e 100644 --- a/packages/react-aria-components/src/TagGroup.tsx +++ b/packages/react-aria-components/src/TagGroup.tsx @@ -202,7 +202,7 @@ export interface TagProps extends RenderProps, LinkDOMProps, Hov export const Tag = /*#__PURE__*/ createLeafComponent('item', (props: TagProps, forwardedRef: ForwardedRef, item: Node) => { let state = useContext(ListStateContext)!; let ref = useObjectRef(forwardedRef); - let {focusProps, isFocusVisible} = useFocusRing({within: true}); + let {focusProps, isFocusVisible} = useFocusRing({within: false}); let {rowProps, gridCellProps, removeButtonProps, ...states} = useTag({item}, state, ref); let {hoverProps, isHovered} = useHover({ From 7bb86302f4a5e11689224fed5fae7e17b42045f9 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Thu, 8 May 2025 06:47:16 +1000 Subject: [PATCH 2/6] always add focus ring to clear button --- .../@react-spectrum/s2/src/ClearButton.tsx | 52 ++++--------------- packages/@react-spectrum/s2/src/TagGroup.tsx | 1 - 2 files changed, 11 insertions(+), 42 deletions(-) diff --git a/packages/@react-spectrum/s2/src/ClearButton.tsx b/packages/@react-spectrum/s2/src/ClearButton.tsx index 740d4ee4079..1f7ebb92a8c 100644 --- a/packages/@react-spectrum/s2/src/ClearButton.tsx +++ b/packages/@react-spectrum/s2/src/ClearButton.tsx @@ -27,9 +27,7 @@ interface ClearButtonStyleProps { * * @default 'M' */ - size?: 'S' | 'M' | 'L' | 'XL', - /** Whether to show a focus ring. */ - showFocusRing?: boolean + size?: 'S' | 'M' | 'L' | 'XL' } interface ClearButtonRenderProps extends ButtonRenderProps, ClearButtonStyleProps {} @@ -57,43 +55,15 @@ const visibleClearButton = style({ }); export const ClearButton = forwardRef(function ClearButton(props: ClearButtonProps, ref: FocusableRef) { - let {showFocusRing = false, size = 'M', ...rest} = props; + let {size = 'M', ...rest} = props; let domRef = useFocusableRef(ref); - - if (showFocusRing) { - return ( - - ); - } else { - return ( - - ); - } + return ( + + ); }); diff --git a/packages/@react-spectrum/s2/src/TagGroup.tsx b/packages/@react-spectrum/s2/src/TagGroup.tsx index ee734c54782..b2aa19e19be 100644 --- a/packages/@react-spectrum/s2/src/TagGroup.tsx +++ b/packages/@react-spectrum/s2/src/TagGroup.tsx @@ -585,7 +585,6 @@ function TagWrapper({children, isDisabled, allowsRemoving, isInRealDOM}) { {!isInRealDOM && children} {allowsRemoving && isInRealDOM && ( From 0229540d61cc3a2abe781ec997e4d51eaa8c847c Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Thu, 8 May 2025 09:51:03 +1000 Subject: [PATCH 3/6] fix styles --- packages/@react-spectrum/s2/src/ClearButton.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-spectrum/s2/src/ClearButton.tsx b/packages/@react-spectrum/s2/src/ClearButton.tsx index 3ff2beef1a7..73f690128e6 100644 --- a/packages/@react-spectrum/s2/src/ClearButton.tsx +++ b/packages/@react-spectrum/s2/src/ClearButton.tsx @@ -40,7 +40,7 @@ const visibleClearButton = style({ alignItems: 'center', justifyContent: 'center', height: 'full', - width: 'control', + width: controlSize(), flexShrink: 0, borderRadius: 'full', borderStyle: 'none', From e5d847e86f9852becfb4648f690a1c01c6f8bbc1 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Thu, 8 May 2025 11:40:49 +1000 Subject: [PATCH 4/6] fix safari --- packages/@adobe/spectrum-css-temp/components/button/index.css | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/@adobe/spectrum-css-temp/components/button/index.css b/packages/@adobe/spectrum-css-temp/components/button/index.css index 151c52dd452..1aa076dff14 100644 --- a/packages/@adobe/spectrum-css-temp/components/button/index.css +++ b/packages/@adobe/spectrum-css-temp/components/button/index.css @@ -380,6 +380,7 @@ a.spectrum-ActionButton { right: 4px; bottom: 4px; top: 4px; + box-shadow: inset 0 0 0 var(--spectrum-focus-ring-size) var(--spectrum-focus-ring-color); } } } From d583cca2e2a94326aa875d40a3247d94aca3ebad Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Fri, 9 May 2025 11:21:55 +1000 Subject: [PATCH 5/6] add focus ring to react aria --- packages/@react-aria/tag/docs/useTagGroup.mdx | 34 ++++++++++++++----- .../react-aria-components/docs/TagGroup.mdx | 10 +++++- 2 files changed, 34 insertions(+), 10 deletions(-) diff --git a/packages/@react-aria/tag/docs/useTagGroup.mdx b/packages/@react-aria/tag/docs/useTagGroup.mdx index 38701fd308c..5fdc29e2251 100644 --- a/packages/@react-aria/tag/docs/useTagGroup.mdx +++ b/packages/@react-aria/tag/docs/useTagGroup.mdx @@ -134,14 +134,14 @@ interface TagProps extends AriaTagProps { function Tag(props: TagProps) { let {item, state} = props; let ref = React.useRef(null); - let {focusProps, isFocusVisible} = useFocusRing({within: true}); + let {focusProps, isFocusVisible} = useFocusRing({within: false}); let {rowProps, gridCellProps, removeButtonProps, allowsRemoving} = useTag(props, state, ref); return (
{item.rendered} - {allowsRemoving && } + {allowsRemoving && }
); @@ -172,13 +172,16 @@ function Tag(props: TagProps) { } .tag-group [role="row"] { - display: flex; - align-items: center; border: 1px solid gray; + forced-color-adjust: none; border-radius: 4px; - padding: 2px 5px; - cursor: default; + padding: 2px 8px; + font-size: 0.929rem; outline: none; + cursor: default; + display: flex; + align-items: center; + transition: border-color 200ms; &[data-focus-visible=true] { outline: 2px solid slateblue; @@ -197,13 +200,24 @@ function Tag(props: TagProps) { } .tag-group [role="gridcell"] { - margin: 0 5px; + display: contents; } .tag-group [role="row"] button { background: none; border: none; - padding-right: 0; + padding: 0; + margin-left: 4px; + outline: none; + font-size: 0.95em; + border-radius: 100%; + aspect-ratio: 1/1; + height: 100%; + + &[data-focus-visible=true] { + outline: 2px solid slateblue; + outline-offset: -1px; + } } .tag-group .description { @@ -227,11 +241,13 @@ The `Button` component is used in the above example to remove a tag. It is built ```tsx example export=true render=false import {useButton} from '@react-aria/button'; +import {mergeProps} from '@react-aria/utils'; function Button(props) { let ref = React.useRef(null); let {buttonProps} = useButton(props, ref); - return ; + let {focusProps, isFocusVisible} = useFocusRing({within: false}); + return ; } ``` diff --git a/packages/react-aria-components/docs/TagGroup.mdx b/packages/react-aria-components/docs/TagGroup.mdx index 79d8e5765cb..c36036c7494 100644 --- a/packages/react-aria-components/docs/TagGroup.mdx +++ b/packages/react-aria-components/docs/TagGroup.mdx @@ -288,15 +288,23 @@ function Example() { background: none; border: none; padding: 0; - margin-left: 8px; + margin-left: 2px; color: var(--text-color-base); transition: color 200ms; outline: none; font-size: 0.95em; + border-radius: 100%; + aspect-ratio: 1/1; + height: 100%; &[data-hovered] { color: var(--text-color-hover); } + + &[data-focus-visible] { + outline: 2px solid var(--focus-ring-color); + outline-offset: -1px; + } } &[data-selected] { From 18c1ab4381f34c912e57408fd4e86e9cffd6c8d3 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Fri, 9 May 2025 11:51:09 +1000 Subject: [PATCH 6/6] fix animation and emphasized --- .../components/button/index.css | 16 +++++++++++----- packages/@react-spectrum/s2/src/ClearButton.tsx | 16 ++++++++++++---- packages/@react-spectrum/s2/src/TagGroup.tsx | 5 +++-- 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/packages/@adobe/spectrum-css-temp/components/button/index.css b/packages/@adobe/spectrum-css-temp/components/button/index.css index 1aa076dff14..3815307497f 100644 --- a/packages/@adobe/spectrum-css-temp/components/button/index.css +++ b/packages/@adobe/spectrum-css-temp/components/button/index.css @@ -373,13 +373,19 @@ a.spectrum-ActionButton { margin-inline: auto; } - &.spectrum-ClearButton--inset { + &.spectrum-ClearButton--inset.spectrum-ClearButton--inset { + transition: unset; + box-sizing: border-box; + &:after { + transition: unset; + left: 4px; + right: 4px; + bottom: 4px; + top: 4px; + } + &:focus-visible { &:after { - left: 4px; - right: 4px; - bottom: 4px; - top: 4px; box-shadow: inset 0 0 0 var(--spectrum-focus-ring-size) var(--spectrum-focus-ring-color); } } diff --git a/packages/@react-spectrum/s2/src/ClearButton.tsx b/packages/@react-spectrum/s2/src/ClearButton.tsx index 73f690128e6..f562f9e2d84 100644 --- a/packages/@react-spectrum/s2/src/ClearButton.tsx +++ b/packages/@react-spectrum/s2/src/ClearButton.tsx @@ -28,14 +28,18 @@ interface ClearButtonStyleProps { * * @default 'M' */ - size?: 'S' | 'M' | 'L' | 'XL' + size?: 'S' | 'M' | 'L' | 'XL', + /** Whether the ClearButton should be displayed with a static color. */ + isStaticColor?: boolean } interface ClearButtonRenderProps extends ButtonRenderProps, ClearButtonStyleProps {} interface ClearButtonProps extends ButtonProps, ClearButtonStyleProps {} +const focusRingStyles = focusRing(); + const visibleClearButton = style({ - ...focusRing(), + ...focusRingStyles, display: 'flex', alignItems: 'center', justifyContent: 'center', @@ -48,6 +52,10 @@ const visibleClearButton = style({ boxSizing: 'border-box', padding: 0, outlineOffset: -4, + outlineColor: { + default: focusRingStyles.outlineColor, + isStaticColor: 'white' + }, color: 'inherit', '--iconPrimary': { type: 'fill', @@ -56,14 +64,14 @@ const visibleClearButton = style({ }); export const ClearButton = forwardRef(function ClearButton(props: ClearButtonProps, ref: FocusableRef) { - let {size = 'M', ...rest} = props; + let {size = 'M', isStaticColor = false, ...rest} = props; let domRef = useFocusableRef(ref); return ( ); diff --git a/packages/@react-spectrum/s2/src/TagGroup.tsx b/packages/@react-spectrum/s2/src/TagGroup.tsx index 358173bf912..10319f4911e 100644 --- a/packages/@react-spectrum/s2/src/TagGroup.tsx +++ b/packages/@react-spectrum/s2/src/TagGroup.tsx @@ -502,13 +502,13 @@ export const Tag = /*#__PURE__*/ (forwardRef as forwardRefType)(function Tag({ch style={pressScale(domRef)} className={renderProps => tagStyles({size, isEmphasized, isLink, ...renderProps})} > {composeRenderProps(children, (children, renderProps) => ( - {typeof children === 'string' ? {children} : children} + {typeof children === 'string' ? {children} : children} ))} ); }); -function TagWrapper({children, isDisabled, allowsRemoving, isInRealDOM}) { +function TagWrapper({children, isDisabled, allowsRemoving, isInRealDOM, isEmphasized, isSelected}) { let {size = 'M'} = useSlottedContext(TagGroupContext) ?? {}; return ( <> @@ -553,6 +553,7 @@ function TagWrapper({children, isDisabled, allowsRemoving, isInRealDOM}) { )}