feat(ui-spinner,ui-avatar): refactor spinner

Author: HerrTopi

docs/guides/upgrade-guide.md

@@ -41,6 +41,11 @@ The new icons automatically sync with theme changes, support all InstUI color to
 - Setting `readonly` does not set the low level `<input>` to disabled, but to `readonly`. This also means that the input is still focusable when `readonly`
 - its DOM structure has been significantly simplified
 
+### Spinner
+
+- `as` prop has been removed, `Spinner` will always render as a `<div>` element.
+- `elementRef` prop has been removed, use the `ref` prop instead.
+
 ## Codemods
 
 To ease the upgrade, we provide codemods that will automate most of the changes. Pay close attention to its output, it cannot refactor complex code! The codemod scripts can be run via the following commands:

packages/ui-avatar/src/Avatar/README.md

@@ -11,11 +11,11 @@ The `size` prop allows you to select from `x-small`, `small`, `medium` and `larg
 ---
 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" />
+<div style={{ display: 'flex', alignItems: 'center', gap: '1rem' }}>
+  <Spinner renderTitle="Loading" size="x-small" />
+  <Spinner renderTitle="Loading" size="small" />
+  <Spinner renderTitle="Loading" />
+  <Spinner renderTitle="Loading" size="large" />
 </div>
 ```
 
@@ -28,8 +28,8 @@ dark backgrounds.
 ---
 type: example
 ---
-<View background="primary-inverse" as="div">
-  <Spinner renderTitle="Loading" variant="inverse" />
+<View background="primary-inverse" as="div" margin="spacing.spaceXs" padding="spacing.spaceLg">
+  <Spinner renderTitle="Loading" variant="inverse" margin="spacing.spaceXs" />
 </View>
 ```
 
@@ -41,11 +41,11 @@ The `delay` prop allows you to delay the rendering of the spinner a desired time
 ---
 type: example
 ---
-<div>
+<div style={{ display: 'flex', alignItems: 'center', gap: '1rem' }}>
   <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" delay={2000} />
+  <Spinner renderTitle="Loading" delay={3000} />
+  <Spinner renderTitle="Loading" size="large" delay={4000} />
 </div>
 ```
 
@@ -57,5 +57,7 @@ The `renderTitle` prop is read to screen readers.
 ---
 type: example
 ---
-<Spinner renderTitle={() => "Hello world"} />
+<div>
+  <Spinner renderTitle={() => "Hello world"} margin="spacing.spaceXs" />
+</div>
 ```

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

@@ -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,41 +81,6 @@ 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('with the delay prop', () => {
     it('should delay rendering', async () => {
       render(<Spinner renderTitle="Loading" delay={300} />)

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

@@ -22,150 +22,113 @@
  * SOFTWARE.
  */
 
-import { Component } from 'react'
+import { useState, useEffect, forwardRef } from 'react'
 
-import { View } from '@instructure/ui-view'
+import { useStyle } from '@instructure/emotion'
 import {
   callRenderProp,
   omitProps,
-  withDeterministicId
+  useDeterministicId
 } from '@instructure/ui-react-utils'
 import { logError as error } from '@instructure/console'
 
-import { withStyleRework as 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,
+    themeOverride
+  } = props
+
+  const [shouldRender, setShouldRender] = useState(!delay)
+  // Deterministic ID generation
+  const [titleId, setTitleId] = useState<string | undefined>()
+  const getId = useDeterministicId('Spinner')
+  useEffect(() => {
+    setTitleId(getId())
+  }, [])
+
+  const styles = useStyle({
+    generateStyle,
+    themeOverride,
+    params: {
+      size,
+      variant,
+      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 = () => {
     error(
-      !!hasTitle,
-      '[Spinner] The renderTitle prop is necessary for screen reader support.'
+      !!renderTitle,
+      '[Spinner] The `renderTitle` prop is necessary for screen reader support.'
     )
 
+    const passthroughProps = omitProps(props, allowedProps)
+
     return (
-      <View
+      <div
         {...passthroughProps}
-        as={this.props.as}
-        elementRef={this.handleRef}
-        css={this.props.styles?.spinner}
-        margin={this.props.margin}
+        css={styles?.spinner}
+        ref={ref}
         data-cid="Spinner"
       >
         <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 }