[v12] Rework spinner

packages/ui-avatar/src/Avatar/props.ts

@@ -76,9 +76,8 @@ type AvatarOwnProps = {
   shape?: 'circle' | 'rectangle'
   display?: 'inline' | 'block'
   /**
-   * Valid values are `0`, `none`, `auto`, `xxx-small`, `xx-small`, `x-small`,
-   * `small`, `medium`, `large`, `x-large`, `xx-large`. Apply these values via
-   * familiar CSS-like shorthand. For example: `margin="small auto large"`.
+   * Valid values are from themes. See theme.semantics.spacing. Apply these values via
+   * familiar CSS-like shorthand. For example: `margin="spaceLg gap.cards.sm 20px padding.container.sm"`.
    */
   margin?: Spacing
   /**

packages/ui-spinner/src/Spinner/README.md

@@ -13,9 +13,9 @@ type: example
 ---
 <div>
   <Spinner renderTitle="Loading" size="x-small"/>
-  <Spinner renderTitle="Loading" size="small" margin="0 0 0 medium" />
-  <Spinner renderTitle="Loading" margin="0 0 0 medium" />
-  <Spinner renderTitle="Loading" size="large" margin="0 0 0 medium" />
+  <Spinner renderTitle="Loading" size="small" margin="0 0 0 spaceMd" />
+  <Spinner renderTitle="Loading" margin="0 0 0 spaceMd" />
+  <Spinner renderTitle="Loading" size="large" margin="0 0 0 spaceMd" />
 </div>
 ```
 
@@ -43,9 +43,9 @@ type: example
 ---
 <div>
   <Spinner renderTitle="Loading" size="x-small" delay={1000} />
-  <Spinner renderTitle="Loading" size="small" margin="0 0 0 medium" delay={2000} />
-  <Spinner renderTitle="Loading" margin="0 0 0 medium"  delay={3000} />
-  <Spinner renderTitle="Loading" size="large" margin="0 0 0 medium"  delay={4000} />
+  <Spinner renderTitle="Loading" size="small" margin="0 0 0 spaceMd" delay={2000} />
+  <Spinner renderTitle="Loading" margin="0 0 0 spaceMd"  delay={3000} />
+  <Spinner renderTitle="Loading" size="large" margin="0 0 0 spaceMd"  delay={4000} />
 </div>
 ```
 

packages/ui-spinner/src/Spinner/__tests__/Spinner.test.tsx

@@ -28,7 +28,6 @@ import { vi, expect } from 'vitest'
 import type { MockInstance } from 'vitest'
 
 import '@testing-library/jest-dom'
-import { View } from '@instructure/ui-view'
 import Spinner from '../index'
 import type { SpinnerProps } from '../props'
 
@@ -82,39 +81,35 @@ describe('<Spinner />', () => {
     expect(spinner).toHaveTextContent('I have translated Loading')
   })
 
-  describe('when passing down props to View', () => {
-    const allowedProps: { [key: string]: any } = {
-      margin: 'small',
-      elementRef: () => {},
-      as: 'div'
-    }
-
-    View.allowedProps
-      .filter((prop) => prop !== 'children')
-      .forEach((prop) => {
-        if (Object.keys(allowedProps).indexOf(prop) < 0) {
-          it(`should NOT allow the '${prop}' prop`, async () => {
-            const props = {
-              [prop]: 'foo'
-            }
-            const expectedErrorMessage = `prop '${prop}' is not allowed.`
-
-            render(<Spinner renderTitle="Loading" {...props} />)
-
-            expect(consoleErrorMock).toHaveBeenCalledWith(
-              expect.stringContaining(expectedErrorMessage),
-              expect.any(String)
-            )
-          })
-        } else {
-          it(`should allow the '${prop}' prop`, async () => {
-            const props = { [prop]: allowedProps[prop] }
-            render(<Spinner renderTitle="Loading" {...props} />)
-
-            expect(consoleErrorMock).not.toHaveBeenCalled()
-          })
-        }
-      })
+  describe('when passing down props', () => {
+    it('should allow the "margin" prop', async () => {
+      render(<Spinner renderTitle="Loading" margin="small" />)
+      expect(consoleErrorMock).not.toHaveBeenCalled()
+    })
+
+    it('should allow the "elementRef" prop', async () => {
+      const ref = vi.fn()
+      render(<Spinner renderTitle="Loading" ref={ref} />)
+      expect(consoleErrorMock).not.toHaveBeenCalled()
+      expect(ref).toHaveBeenCalledWith(expect.any(Element))
+    })
+
+    it('should pass through DOM props to the div element', async () => {
+      const { container } = render(
+        <Spinner renderTitle="Loading" data-testid="spinner" id="spinner-id" />
+      )
+      const spinner = container.querySelector('div')
+      expect(spinner).toHaveAttribute('data-testid', 'spinner')
+      expect(spinner).toHaveAttribute('id', 'spinner-id')
+    })
+
+    it('should not pass through className as it is automatically excluded by omitProps', async () => {
+      const { container } = render(
+        <Spinner renderTitle="Loading" className="custom-class" />
+      )
+      const spinner = container.querySelector('div')
+      expect(spinner).not.toHaveClass('custom-class')
+    })
   })
 
   describe('with the delay prop', () => {

packages/ui-spinner/src/Spinner/index.tsx

@@ -22,150 +22,103 @@
  * SOFTWARE.
  */
 
-import { Component } from 'react'
+import { useState, useEffect, useId, forwardRef } from 'react'
 
-import { View } from '@instructure/ui-view'
-import {
-  callRenderProp,
-  omitProps,
-  withDeterministicId
-} from '@instructure/ui-react-utils'
+import { useStyle } from '@instructure/emotion'
+import { callRenderProp, omitProps } from '@instructure/ui-react-utils'
 import { logError as error } from '@instructure/console'
 
-import { withStyle } from '@instructure/emotion'
-
 import generateStyle from './styles'
 import generateComponentTheme from './theme'
-import type { SpinnerProps, SpinnerState } from './props'
+import type { SpinnerProps } from './props'
 import { allowedProps } from './props'
 
 /**
 ---
 category: components
 ---
 **/
-@withDeterministicId()
-@withStyle(generateStyle, generateComponentTheme)
-class Spinner extends Component<SpinnerProps, SpinnerState> {
-  static readonly componentId = 'Spinner'
-  static allowedProps = allowedProps
-  static defaultProps = {
-    as: 'div',
-    size: 'medium',
-    variant: 'default'
-  }
-
-  ref: Element | null = null
-  private readonly titleId?: string
-  private delayTimeout?: NodeJS.Timeout
-
-  handleRef = (el: Element | null) => {
-    const { elementRef } = this.props
-
-    this.ref = el
-
-    if (typeof elementRef === 'function') {
-      elementRef(el)
-    }
-  }
-
-  constructor(props: SpinnerProps) {
-    super(props)
-
-    this.titleId = props.deterministicId!()
-
-    this.state = {
-      shouldRender: !props.delay
-    }
-  }
-
-  componentDidMount() {
-    this.props.makeStyles?.()
-    const { delay } = this.props
-
+const Spinner = forwardRef<HTMLDivElement, SpinnerProps>((props, ref) => {
+  const {
+    size = 'medium',
+    variant = 'default',
+    delay,
+    renderTitle,
+    margin,
+    // elementRef,
+    themeOverride
+  } = props
+
+  const [shouldRender, setShouldRender] = useState(!delay)
+  const titleId = useId()
+
+  const styles = useStyle({
+    generateStyle,
+    generateComponentTheme,
+    params: {
+      size,
+      variant,
+      themeOverride,
+      margin
+    },
+    componentId: 'Spinner',
+    displayName: 'Spinner'
+  })
+
+  useEffect(() => {
     if (delay) {
-      this.delayTimeout = setTimeout(() => {
-        this.setState({ shouldRender: true })
+      const delayTimeout = setTimeout(() => {
+        setShouldRender(true)
       }, delay)
-    }
-  }
-
-  componentDidUpdate() {
-    this.props.makeStyles?.()
-  }
-
-  componentWillUnmount() {
-    clearTimeout(this.delayTimeout)
-  }
 
-  radius() {
-    switch (this.props.size) {
-      case 'x-small':
-        return '0.5em'
-      case 'small':
-        return '1em'
-      case 'large':
-        return '2.25em'
-      default:
-        return '1.75em'
+      return () => clearTimeout(delayTimeout)
     }
-  }
-
-  renderSpinner() {
-    const passthroughProps = View.omitViewProps(
-      omitProps(this.props, Spinner.allowedProps),
-      Spinner
-    )
+    return undefined
+  }, [delay])
 
-    const hasTitle = this.props.renderTitle
+  const renderSpinner = () => {
+    const hasTitle = renderTitle
     error(
       !!hasTitle,
       '[Spinner] The renderTitle prop is necessary for screen reader support.'
     )
 
+    const passthroughProps = omitProps(props, allowedProps)
+
     return (
-      <View
-        {...passthroughProps}
-        as={this.props.as}
-        elementRef={this.handleRef}
-        css={this.props.styles?.spinner}
-        margin={this.props.margin}
-        data-cid="Spinner"
-      >
+      <div {...passthroughProps} css={styles?.spinner} ref={ref}>
         <svg
-          css={this.props.styles?.circle}
+          css={styles?.circle}
           role="img"
-          aria-labelledby={this.titleId}
+          aria-labelledby={titleId}
           focusable="false"
         >
-          <title id={this.titleId}>
-            {callRenderProp(this.props.renderTitle)}
-          </title>
+          <title id={titleId}>{callRenderProp(renderTitle)}</title>
           <g role="presentation">
-            {this.props.variant !== 'inverse' && (
+            {variant !== 'inverse' && (
               <circle
-                css={this.props.styles?.circleTrack}
+                css={styles?.circleTrack}
                 cx="50%"
                 cy="50%"
-                r={this.radius()}
+                r={styles?.radius as string}
               />
             )}
             <circle
-              css={this.props.styles?.circleSpin}
+              css={styles?.circleSpin}
               cx="50%"
               cy="50%"
-              r={this.radius()}
+              r={styles?.radius as string}
             />
           </g>
         </svg>
-      </View>
+      </div>
     )
   }
 
-  render() {
-    return this.state.shouldRender ? this.renderSpinner() : null
-  }
-}
+  return shouldRender ? renderSpinner() : null
+})
+
+Spinner.displayName = 'Spinner'
 
 export default Spinner
 export { Spinner }

packages/ui-spinner/src/Spinner/props.ts

@@ -28,30 +28,20 @@ import type {
   ComponentStyle
 } from '@instructure/emotion'
 import type {
-  AsElementType,
   OtherHTMLAttributes,
   SpinnerTheme
 } from '@instructure/shared-types'
 import type { WithDeterministicIdProps } from '@instructure/ui-react-utils'
 import { Renderable } from '@instructure/shared-types'
 
 type SpinnerOwnProps = {
-  /**
-   * Render Spinner "as" another HTML element
-   */
-  as?: AsElementType
   /**
    * delay spinner rendering for a time (in ms). Used to prevent flickering in case of very fast load times
    */
   delay?: number
   /**
-   * provides a reference to the underlying html root element
-   */
-  elementRef?: (element: Element | null) => void
-  /**
-   * Valid values are `0`, `none`, `auto`, `xxx-small`, `xx-small`, `x-small`,
-   * `small`, `medium`, `large`, `x-large`, `xx-large`. Apply these values via
-   * familiar CSS-like shorthand. For example: `margin="small auto large"`.
+   * Valid values are from themes. See theme.semantics.spacing. Apply these values via
+   * familiar CSS-like shorthand. For example: `margin="spaceLg gap.cards.sm 20px padding.container.sm"`.
    */
   margin?: Spacing
   /**
@@ -82,16 +72,15 @@ type SpinnerState = {
 }
 
 type SpinnerStyle = ComponentStyle<
-  'spinner' | 'circle' | 'circleTrack' | 'circleSpin'
+  'spinner' | 'circle' | 'circleTrack' | 'circleSpin' | 'radius'
 >
+
 const allowedProps: AllowedPropKeys = [
   'delay',
   'renderTitle',
   'size',
   'variant',
-  'margin',
-  'elementRef',
-  'as'
+  'margin'
 ]
 
 export type { SpinnerProps, SpinnerState, SpinnerStyle }