diff --git a/packages/ui-avatar/src/Avatar/props.ts b/packages/ui-avatar/src/Avatar/props.ts index cb2178f7c2..fd5f870653 100644 --- a/packages/ui-avatar/src/Avatar/props.ts +++ b/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 /** diff --git a/packages/ui-spinner/src/Spinner/README.md b/packages/ui-spinner/src/Spinner/README.md index ec2e9ec3c9..fc63a27a64 100644 --- a/packages/ui-spinner/src/Spinner/README.md +++ b/packages/ui-spinner/src/Spinner/README.md @@ -13,9 +13,9 @@ type: example ---
- - - + + +
``` @@ -43,9 +43,9 @@ type: example ---
- - - + + +
``` diff --git a/packages/ui-spinner/src/Spinner/__tests__/Spinner.test.tsx b/packages/ui-spinner/src/Spinner/__tests__/Spinner.test.tsx index ef8a8fbd11..31cfb9dcdc 100644 --- a/packages/ui-spinner/src/Spinner/__tests__/Spinner.test.tsx +++ b/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('', () => { 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() - - expect(consoleErrorMock).toHaveBeenCalledWith( - expect.stringContaining(expectedErrorMessage), - expect.any(String) - ) - }) - } else { - it(`should allow the '${prop}' prop`, async () => { - const props = { [prop]: allowedProps[prop] } - render() - - expect(consoleErrorMock).not.toHaveBeenCalled() - }) - } - }) + describe('when passing down props', () => { + it('should allow the "margin" prop', async () => { + render() + expect(consoleErrorMock).not.toHaveBeenCalled() + }) + + it('should allow the "elementRef" prop', async () => { + const ref = vi.fn() + render() + expect(consoleErrorMock).not.toHaveBeenCalled() + expect(ref).toHaveBeenCalledWith(expect.any(Element)) + }) + + it('should pass through DOM props to the div element', async () => { + const { container } = render( + + ) + 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( + + ) + const spinner = container.querySelector('div') + expect(spinner).not.toHaveClass('custom-class') + }) }) describe('with the delay prop', () => { diff --git a/packages/ui-spinner/src/Spinner/index.tsx b/packages/ui-spinner/src/Spinner/index.tsx index 979c041286..d9c6036add 100644 --- a/packages/ui-spinner/src/Spinner/index.tsx +++ b/packages/ui-spinner/src/Spinner/index.tsx @@ -22,21 +22,15 @@ * 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' /** @@ -44,128 +38,87 @@ import { allowedProps } from './props' category: components --- **/ -@withDeterministicId() -@withStyle(generateStyle, generateComponentTheme) -class Spinner extends Component { - 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((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 ( - +
- - {callRenderProp(this.props.renderTitle)} - + {callRenderProp(renderTitle)} - {this.props.variant !== 'inverse' && ( + {variant !== 'inverse' && ( )} - +
) } - render() { - return this.state.shouldRender ? this.renderSpinner() : null - } -} + return shouldRender ? renderSpinner() : null +}) + +Spinner.displayName = 'Spinner' export default Spinner export { Spinner } diff --git a/packages/ui-spinner/src/Spinner/props.ts b/packages/ui-spinner/src/Spinner/props.ts index 5c7d51e1d9..4df2d25ca1 100644 --- a/packages/ui-spinner/src/Spinner/props.ts +++ b/packages/ui-spinner/src/Spinner/props.ts @@ -28,7 +28,6 @@ import type { ComponentStyle } from '@instructure/emotion' import type { - AsElementType, OtherHTMLAttributes, SpinnerTheme } from '@instructure/shared-types' @@ -36,22 +35,13 @@ 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 } diff --git a/packages/ui-spinner/src/Spinner/styles.ts b/packages/ui-spinner/src/Spinner/styles.ts index a2abc5482d..758b611d1e 100644 --- a/packages/ui-spinner/src/Spinner/styles.ts +++ b/packages/ui-spinner/src/Spinner/styles.ts @@ -22,11 +22,18 @@ * SOFTWARE. */ -import { keyframes } from '@instructure/emotion' +import { keyframes, mapSpacingToShorthand } from '@instructure/emotion' +import type { NewComponentTypes } from '@instructure/ui-themes' -import type { SpinnerTheme } from '@instructure/shared-types' import type { SpinnerProps, SpinnerStyle } from './props' +type StyleParams = { + size: SpinnerProps['size'] + variant: SpinnerProps['variant'] + themeOverride: SpinnerProps['themeOverride'] + margin: SpinnerProps['margin'] +} + // keyframes have to be outside of 'generateStyle', // since it is causing problems in style recalculation const rotate = keyframes` @@ -40,7 +47,7 @@ const morph = keyframes` } 50% { - stroke-dashoffset: 50%; + stroke-dashoffset: 00%; transform: rotate(90deg); } @@ -60,84 +67,89 @@ const morph = keyframes` * @return {Object} The final style object, which will be used in the component */ const generateStyle = ( - componentTheme: SpinnerTheme, - props: SpinnerProps + componentTheme: NewComponentTypes['Spinner'], + params: StyleParams, + //TODO type themes properly + theme: any ): SpinnerStyle => { - const { size, variant } = props + const { size, variant, margin } = params const spinnerSizes = { 'x-small': { - width: componentTheme.xSmallSize, - height: componentTheme.xSmallSize + width: componentTheme.containerSizeXs, + height: componentTheme.containerSizeXs }, small: { - width: componentTheme.smallSize, - height: componentTheme.smallSize + width: componentTheme.containerSizeSm, + height: componentTheme.containerSizeSm }, medium: { - width: componentTheme.mediumSize, - height: componentTheme.mediumSize + width: componentTheme.containerSizeMd, + height: componentTheme.containerSizeMd }, large: { - width: componentTheme.largeSize, - height: componentTheme.largeSize + width: componentTheme.containerSizeLg, + height: componentTheme.containerSizeLg } } const circleSizes = { 'x-small': { - width: componentTheme.xSmallSize, - height: componentTheme.xSmallSize + width: componentTheme.containerSizeXs, + height: componentTheme.containerSizeXs }, small: { - width: componentTheme.smallSize, - height: componentTheme.smallSize + width: componentTheme.containerSizeSm, + height: componentTheme.containerSizeSm }, medium: { - width: componentTheme.mediumSize, - height: componentTheme.mediumSize + width: componentTheme.containerSizeMd, + height: componentTheme.containerSizeMd }, large: { - width: componentTheme.largeSize, - height: componentTheme.largeSize + width: componentTheme.containerSizeLg, + height: componentTheme.containerSizeLg } } const circleTrackSizes = { 'x-small': { - strokeWidth: componentTheme.xSmallBorderWidth + strokeWidth: componentTheme.strokeWidthXs }, small: { - strokeWidth: componentTheme.smallBorderWidth + strokeWidth: componentTheme.strokeWidthSm }, medium: { - strokeWidth: componentTheme.mediumBorderWidth + strokeWidth: componentTheme.strokeWidthMd }, large: { - strokeWidth: componentTheme.largeBorderWidth + strokeWidth: componentTheme.strokeWidthLg } } + const radii = { + 'x-small': `calc(50% - ${componentTheme.strokeWidthXs} / 2)`, + small: `calc(50% - ${componentTheme.strokeWidthSm} / 2)`, + medium: `calc(50% - ${componentTheme.strokeWidthMd} / 2)`, + large: `calc(50% - ${componentTheme.strokeWidthLg} / 2)` + } + const circleSpinSizes = { 'x-small': { - strokeWidth: componentTheme.xSmallBorderWidth, - strokeDasharray: '3em', - transformOrigin: `calc(${componentTheme.xSmallSize} / 2) calc(${componentTheme.xSmallSize} / 2)` + strokeWidth: componentTheme.strokeWidthXs, + transformOrigin: `calc(${componentTheme.containerSizeXs} / 2) calc(${componentTheme.containerSizeXs} / 2)` }, small: { - strokeWidth: componentTheme.smallBorderWidth, - strokeDasharray: '6em', - transformOrigin: `calc(${componentTheme.smallSize} / 2) calc(${componentTheme.smallSize} / 2)` + strokeWidth: componentTheme.strokeWidthSm, + transformOrigin: `calc(${componentTheme.containerSizeSm} / 2) calc(${componentTheme.containerSizeSm} / 2)` }, medium: { - strokeWidth: componentTheme.mediumBorderWidth, - strokeDasharray: '10.5em', - transformOrigin: `calc(${componentTheme.mediumSize} / 2) calc(${componentTheme.mediumSize} / 2)` + strokeWidth: componentTheme.strokeWidthMd, + transformOrigin: `calc(${componentTheme.containerSizeMd} / 2) calc(${componentTheme.containerSizeMd} / 2)` }, large: { - strokeWidth: componentTheme.largeBorderWidth, - strokeDasharray: '14em', - transformOrigin: `calc(${componentTheme.largeSize} / 2) calc(${componentTheme.largeSize} / 2)` + strokeWidth: componentTheme.strokeWidthLg, + transformOrigin: `calc(${componentTheme.containerSizeLg} / 2) calc(${componentTheme.containerSizeLg} / 2)` //7em } } @@ -154,7 +166,8 @@ const generateStyle = ( position: 'relative', boxSizing: 'border-box', overflow: 'hidden', - ...spinnerSizes[size!] + ...spinnerSizes[size!], + margin: mapSpacingToShorthand(margin, theme.semantics.spacing) }, circle: { label: 'spinner__circle', @@ -172,6 +185,8 @@ const generateStyle = ( label: 'spinner__circleTrack', stroke: componentTheme.trackColor, fill: 'none', + width: '100%', + height: '100%', ...circleTrackSizes[size!] }, circleSpin: { @@ -182,9 +197,11 @@ const generateStyle = ( animationDuration: '1.75s', animationIterationCount: 'infinite', animationTimingFunction: 'ease', + strokeDasharray: `calc(${radii[size!]} * 2 * 3.14159 * 0.75) 1000px`, ...circleSpinSizes[size!], ...circleSpinVariant[variant!] - } + }, + radius: radii[size!] } }