fix(Spinner): use forwardRef and aria-label for consistency (#2682)

- Use `aria-label` for accessible spinner label to be consistent with other components - Use `forwardRef` on Spinner --------- Co-authored-by: Tobias Barsnes <[email protected]>
digdir · Oct 25, 2024 · d3c58b0 · d3c58b0
1 parent 326671a
commit d3c58b0
Show file tree

Hide file tree

Showing 10 changed files with 54 additions and 41 deletions.
diff --git a/.changeset/wet-scissors-tickle.md b/.changeset/wet-scissors-tickle.md
@@ -0,0 +1,5 @@
+---
+"@digdir/designsystemet-react": patch
+---
+
+Spinner: `aria-label` required instead of `title` prop
diff --git a/apps/theme/components/Previews/Components/Components.tsx b/apps/theme/components/Previews/Components/Components.tsx
@@ -415,8 +415,8 @@ export const Components = () => {
         <Skeleton variant='text' />
         <Skeleton variant='text' />
         <div>
-          <Spinner title='laster innhold' size='md' />
-          <Spinner title='laster innhold' size='md' color='accent' />
+          <Spinner aria-label='laster innhold' size='md' />
+          <Spinner aria-label='laster innhold' size='md' color='accent' />
         </div>
       </div>
     </div>

diff --git a/packages/react/src/components/Button/Button.tsx b/packages/react/src/components/Button/Button.tsx
@@ -78,7 +78,7 @@ export const Button = forwardRef<HTMLButtonElement, ButtonProps>(
         {...rest}
       >
         {loading === true ? (
-          <Spinner aria-hidden='true' color={spinnerColor} size='sm' title='' />
+          <Spinner aria-hidden='true' color={spinnerColor} size='sm' />
         ) : (
           loading // Allow custom loading spinner
         )}

diff --git a/packages/react/src/components/form/Combobox/Combobox.tsx b/packages/react/src/components/form/Combobox/Combobox.tsx
@@ -459,7 +459,7 @@ export const ComboboxComponent = forwardRef<HTMLInputElement, ComboboxProps>(
 
                 {loading ? (
                   <ComboboxCustom className={'ds-combobox__loading'}>
-                    <Spinner title='Laster' size='sm' />
+                    <Spinner aria-label='Laster' size='sm' />
                     {loadingLabel}
                   </ComboboxCustom>
                 ) : (

diff --git a/packages/react/src/components/loaders/Spinner/Spinner.stories.tsx b/packages/react/src/components/loaders/Spinner/Spinner.stories.tsx
@@ -20,25 +20,25 @@ export default {
 export const Preview: Story = (args) => <Spinner {...args} />;
 
 Preview.args = {
-  title: 'Henter kaffi',
+  'aria-label': 'Henter kaffi',
   size: 'md',
   color: 'neutral',
 };
 
 export const Variants: Story = () => (
   <>
-    <Spinner title='Henter kaffi' color='neutral' size='xl' />
-    <Spinner title='Henter kaffi' color='accent' size='xl' />
+    <Spinner aria-label='Henter kaffi' color='neutral' size='xl' />
+    <Spinner aria-label='Henter kaffi' color='accent' size='xl' />
   </>
 );
 
 export const Sizes: Story = () => (
   <>
-    <Spinner title='Henter kaffi' color='neutral' size='2xs' />
-    <Spinner title='Henter kaffi' color='neutral' size='xs' />
-    <Spinner title='Henter kaffi' color='neutral' size='sm' />
-    <Spinner title='Henter kaffi' color='neutral' size='md' />
-    <Spinner title='Henter kaffi' color='neutral' size='lg' />
-    <Spinner title='Henter kaffi' color='neutral' size='xl' />
+    <Spinner aria-label='Henter kaffi' color='neutral' size='2xs' />
+    <Spinner aria-label='Henter kaffi' color='neutral' size='xs' />
+    <Spinner aria-label='Henter kaffi' color='neutral' size='sm' />
+    <Spinner aria-label='Henter kaffi' color='neutral' size='md' />
+    <Spinner aria-label='Henter kaffi' color='neutral' size='lg' />
+    <Spinner aria-label='Henter kaffi' color='neutral' size='xl' />
   </>
 );
diff --git a/packages/react/src/components/loaders/Spinner/Spinner.test.tsx b/packages/react/src/components/loaders/Spinner/Spinner.test.tsx
@@ -8,7 +8,7 @@ beforeAll(() => {
 
 describe('spinner', (): void => {
   it('should render with title "loading', (): void => {
-    render(<Spinner title='Loading' />);
-    expect(screen.getByTitle('Loading')).toBeInTheDocument();
+    render(<Spinner aria-label='Loading' />);
+    expect(screen.getByLabelText('Loading')).toBeInTheDocument();
   });
 });
diff --git a/packages/react/src/components/loaders/Spinner/Spinner.tsx b/packages/react/src/components/loaders/Spinner/Spinner.tsx
@@ -1,11 +1,12 @@
+import { useMergeRefs } from '@floating-ui/react';
 import cl from 'clsx/lite';
-import type { ComponentPropsWithoutRef } from 'react';
+import { type ComponentPropsWithoutRef, forwardRef } from 'react';
 
 import { useSynchronizedAnimation } from '../../../utilities';
 
 export type SpinnerProps = {
-  /** Spinner title  */
-  title: string;
+  /** Accessibile label  */
+  'aria-label'?: string;
   /**
    * Spinner size
    */
@@ -15,16 +16,23 @@ export type SpinnerProps = {
    * @default neutral
    */
   color?: 'neutral' | 'accent';
-} & ComponentPropsWithoutRef<'svg'>;
+} & ComponentPropsWithoutRef<'svg'> &
+  (
+    | { 'aria-label': string; 'aria-hidden'?: never }
+    | { 'aria-label'?: string; 'aria-hidden': true | 'true' } // Make aria-label optional when aria-hidden is true
+  );
 
 /**  Spinner component used for indicating busy or indeterminate loading */
-export const Spinner = ({
-  title,
-  color = 'neutral',
-  size,
-  className,
-  ...rest
-}: SpinnerProps): JSX.Element => {
+export const Spinner = forwardRef<SVGSVGElement, SpinnerProps>(function Spinner(
+  {
+    'aria-label': ariaLabel,
+    color = 'neutral',
+    size,
+    className,
+    ...rest
+  }: SpinnerProps,
+  ref,
+) {
   const svgRef = useSynchronizedAnimation<SVGSVGElement>(
     'ds-spinner-rotate-animation',
   );
@@ -33,24 +41,27 @@ export const Spinner = ({
     'ds-spinner-stroke-animation',
   );
 
+  const mergedRefs = useMergeRefs([svgRef, ref]);
+
   return (
     <svg
+      aria-label={ariaLabel}
       className={cl('ds-spinner', className)}
-      viewBox='0 0 50 50'
-      ref={svgRef}
       data-color={color}
       data-size={size}
+      ref={mergedRefs}
+      role='img'
+      viewBox='0 0 50 50'
       {...rest}
     >
-      <title>{title}</title>
       <circle
         className={cl('ds-spinner__background')}
         cx='25'
         cy='25'
         r='20'
         fill='none'
         strokeWidth='5'
-      ></circle>
+      />
       <circle
         className={cl(`ds-spinner__circle`)}
         cx='25'
@@ -59,9 +70,7 @@ export const Spinner = ({
         fill='none'
         strokeWidth='5'
         ref={strokeRef}
-      ></circle>
+      />
     </svg>
   );
-};
-
-Spinner.displayName = 'Spinner';
+});
diff --git a/packages/react/src/components/loaders/Spinner/index.ts b/packages/react/src/components/loaders/Spinner/index.ts
@@ -1,2 +1 @@
-export type { SpinnerProps } from './Spinner';
-export { Spinner } from './Spinner';
+export * from './Spinner';
diff --git a/packages/react/stories/testing.stories.tsx b/packages/react/stories/testing.stories.tsx
@@ -693,10 +693,10 @@ export const Sizes: StoryFn = () => {
         <List.Item>List Item 2</List.Item>
         <List.Item>List Item 3</List.Item>
       </List.Unordered>
-      <Spinner size='xs' title='Loading' />
-      <Spinner size='sm' title='Loading' />
-      <Spinner size='md' title='Loading' />
-      <Spinner size='xl' title='Loading' />
+      <Spinner size='xs' aria-label='Loading' />
+      <Spinner size='sm' aria-label='Loading' />
+      <Spinner size='md' aria-label='Loading' />
+      <Spinner size='xl' aria-label='Loading' />
       <div data-size='xs'>
         <Modal.Context>
           <Modal.Trigger>Open Modal</Modal.Trigger>

diff --git a/plugins/figma-plugin/src/ui/components/Toast/Toast.tsx b/plugins/figma-plugin/src/ui/components/Toast/Toast.tsx
@@ -34,7 +34,7 @@ export const Toast = () => {
       )}
       {!success && (
         <>
-          <Spinner color='accent' title='loading' size='sm' />
+          <Spinner color='accent' aria-label='loading' size='sm' />
           Oppdaterer variabler...
         </>
       )}