feat: Tag remove-button focus ring (#8194)

snowystinger · dannify · web-flow · commit acecfe2d2733 · 2025-05-09T22:46:48.000Z
* feat: Tag remove button focus ring

* always add focus ring to clear button

* fix styles

* fix safari

* add focus ring to react aria

* fix animation and emphasized

---------

Co-authored-by: Danni &lt;darobins@adobe.com&gt;
diff --git a/packages/@adobe/spectrum-css-temp/components/button/index.css b/packages/@adobe/spectrum-css-temp/components/button/index.css
@@ -372,6 +372,24 @@ a.spectrum-ActionButton {
     margin-block: 0;
     margin-inline: auto;
   }
+
+  &.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 {
+        box-shadow: inset 0 0 0 var(--spectrum-focus-ring-size) var(--spectrum-focus-ring-color);
+      }
+    }
+  }
 }
 
 @media screen and (-ms-high-contrast: active), (-ms-high-contrast: none) {
diff --git a/packages/@react-aria/tag/docs/useTagGroup.mdx b/packages/@react-aria/tag/docs/useTagGroup.mdx
@@ -134,14 +134,14 @@ interface TagProps<T> extends AriaTagProps<T> {
 function Tag<T>(props: TagProps<T>) {
   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 (
     <div ref={ref} {...rowProps} {...focusProps} data-focus-visible={isFocusVisible}>
       <div {...gridCellProps}>
         {item.rendered}
-        {allowsRemoving && <Button {...removeButtonProps}>❎</Button>}
+        {allowsRemoving && <Button {...removeButtonProps}>ⓧ</Button>}
       </div>
     </div>
   );
@@ -172,13 +172,16 @@ function Tag<T>(props: TagProps<T>) {
 }
 
 .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<T>(props: TagProps<T>) {
 }
 
 .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 <button {...buttonProps} ref={ref}>{props.children}</button>;
+  let {focusProps, isFocusVisible} = useFocusRing({within: false});
+  return <button {...mergeProps(buttonProps, focusProps)} ref={ref} data-focus-visible={isFocusVisible}>{props.children}</button>;
 }
 ```
 
diff --git a/packages/@react-spectrum/button/src/ClearButton.tsx b/packages/@react-spectrum/button/src/ClearButton.tsx
@@ -25,7 +25,8 @@ interface ClearButtonProps<T extends ElementType = 'button'> 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<HTMLButtonElement>) {
@@ -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
@@ -18,47 +18,61 @@ import {
 import {controlSize} from './style-utils' with {type: 'macro'};
 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 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<ClearButtonRenderProps>({
+  ...focusRingStyles,
+  display: 'flex',
+  alignItems: 'center',
+  justifyContent: 'center',
+  height: 'full',
+  width: controlSize(),
+  flexShrink: 0,
+  borderRadius: 'full',
+  borderStyle: 'none',
+  backgroundColor: 'transparent',
+  boxSizing: 'border-box',
+  padding: 0,
+  outlineOffset: -4,
+  outlineColor: {
+    default: focusRingStyles.outlineColor,
+    isStaticColor: 'white'
+  },
+  color: 'inherit',
+  '--iconPrimary': {
+    type: 'fill',
+    value: 'currentColor'
+  }
+});
+
 export const ClearButton = forwardRef(function ClearButton(props: ClearButtonProps, ref: FocusableRef<HTMLButtonElement>) {
+  let {size = 'M', isStaticColor = false, ...rest} = props;
   let domRef = useFocusableRef(ref);
-
   return (
     <Button
-      {...props}
+      {...rest}
       ref={domRef}
-      className={renderProps => style<ClearButtonRenderProps>({
-        display: 'flex',
-        alignItems: 'center',
-        justifyContent: 'center',
-        height: 'full',
-        width: controlSize(),
-        flexShrink: 0,
-        borderStyle: 'none',
-        outlineStyle: 'none',
-        backgroundColor: 'transparent',
-        padding: 0,
-        color: 'inherit',
-        '--iconPrimary': {
-          type: 'fill',
-          value: 'currentColor'
-        }
-      })({...renderProps, size: props.size || 'M'})}>
-      <CrossIcon size={props.size || 'M'} />
+      style={pressScale(domRef)}
+      className={renderProps => visibleClearButton({...renderProps, size, isStaticColor})}>
+      <CrossIcon size={props.size} />
     </Button>
   );
 });
diff --git 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) => (
-        <TagWrapper isInRealDOM={isInRealDOM} {...renderProps}>{typeof children === 'string' ? <Text>{children}</Text> : children}</TagWrapper>
+        <TagWrapper isInRealDOM={isInRealDOM} isEmphasized={isEmphasized} {...renderProps}>{typeof children === 'string' ? <Text>{children}</Text> : children}</TagWrapper>
       ))}
     </AriaTag>
   );
 });
 
-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}) {
         <ClearButton
           slot="remove"
           size={size}
+          isStaticColor={isEmphasized && isSelected}
           isDisabled={isDisabled} />
       )}
     </>
diff --git 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 (
       <div style={{width: 320, resize: 'horizontal', overflow: 'hidden', padding: 4}}>
-        <TagGroup {...args}>
+        <TagGroup {...props}>
           <Tag id="chocolate">Chocolate</Tag>
           <Tag>Mint</Tag>
           <Tag>Strawberry</Tag>
@@ -102,12 +103,13 @@ let items: Array<ITagItem> = [
 ];
 export let Dynamic = {
   render: (args: any) => {
+    let props = {...args};
     if (args.onRemove) {
-      args.onRemove = action('remove');
+      props.onRemove = action('remove');
     }
     return (
       <div style={{width: 320, resize: 'horizontal', overflow: 'hidden', padding: 4}}>
-        <TagGroup {...args} items={items}>
+        <TagGroup {...props} items={items}>
           {(item: ITagItem) => <Tag>{item.name}</Tag>}
         </TagGroup>
       </div>
@@ -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 (
-      <TagGroup {...args} disabledKeys={new Set(['mint', 'vanilla'])} styles={style({width: 320})}>
+      <TagGroup {...props} disabledKeys={new Set(['mint', 'vanilla'])} styles={style({width: 320})}>
         <Tag id="chocolate" textValue="chocolate"><NewIcon /><Text>Chocolate</Text></Tag>
         <Tag id="mint">Mint</Tag>
         <Tag id="strawberry">
@@ -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 (
-      <TagGroup {...args} renderEmptyState={renderEmptyState} />
+      <TagGroup {...props} renderEmptyState={renderEmptyState} />
     );
   },
   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 (
-      <TagGroup {...args} />
+      <TagGroup {...props} />
     );
   },
   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 (
-      <TagGroup {...args} disabledKeys={new Set(['google'])}>
+      <TagGroup {...props} disabledKeys={new Set(['google'])}>
         <Tag id="adobe" href="https://adobe.com">Adobe</Tag>
         <Tag id="google">Google</Tag>
         <Tag id="apple" href="https://apple.com">Apple</Tag>
@@ -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 (
       <TagGroup
-        {...args}
+        {...props}
         contextualHelp={
           <ContextualHelp>
             <Heading>What is a ice cream?</Heading>
diff --git a/packages/@react-spectrum/tag/src/Tag.tsx b/packages/@react-spectrum/tag/src/Tag.tsx
@@ -35,7 +35,7 @@ export function Tag<T>(props: SpectrumTagProps<T>): 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 (
     <span {...styleProps}>
-      <ClearButton {...props} />
+      <ClearButton {...props} inset />
     </span>
   );
 }
diff --git 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] {
diff --git a/packages/react-aria-components/src/TagGroup.tsx b/packages/react-aria-components/src/TagGroup.tsx
