Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions packages/react-spinner/etc/react-spinner.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,30 +7,30 @@
import type { ComponentProps } from '@fluentui/react-utilities';
import type { ComponentState } from '@fluentui/react-utilities';
import type { ForwardRefComponent } from '@fluentui/react-utilities';
import { Label } from '@fluentui/react-label';
import * as React_2 from 'react';
import type { Slot } from '@fluentui/react-utilities';
import { SlotClassNames } from '@fluentui/react-utilities';
import type { SlotClassNames } from '@fluentui/react-utilities';

// @public
export const renderSpinner_unstable: (state: SpinnerState) => JSX.Element;

// @public
export const Spinner: ForwardRefComponent<SpinnerProps>;

// @public @deprecated (undocumented)
export const spinnerClassName = "fui-Spinner";

// @public (undocumented)
export const spinnerClassNames: SlotClassNames<SpinnerSlots>;

// Warning: (ae-forgotten-export) The symbol "SpinnerCommons" needs to be exported by the entry point index.d.ts
//
// @public
export type SpinnerProps = ComponentProps<SpinnerSlots> & SpinnerCommons;
export type SpinnerProps = Omit<ComponentProps<SpinnerSlots>, 'size'> & Partial<SpinnerCommons>;

// @public (undocumented)
export type SpinnerSlots = {
root: Slot<'div'>;
root: NonNullable<Slot<'div'>>;
spinner?: Slot<'span'>;
label?: Slot<typeof Label>;
};

// @public
Expand Down
1 change: 1 addition & 0 deletions packages/react-spinner/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
},
"dependencies": {
"@fluentui/react-theme": "9.0.0-rc.4",
"@fluentui/react-label": "9.0.0-beta.9",
"@fluentui/react-utilities": "9.0.0-rc.5",
"@griffel/react": "1.0.0",
"tslib": "^2.1.0"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import * as React from 'react';

export const DefaultSvg = () => (
<svg role="progressbar" className="fui-Spinner__Progressbar">
<circle className="fui-Spinner__Track" />
<circle className="fui-Spinner__Tail" />
</svg>
);
10 changes: 10 additions & 0 deletions packages/react-spinner/src/components/Spinner/Spinner.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,16 @@ describe('Spinner', () => {
isConformant({
Component: Spinner,
displayName: 'Spinner',
testOptions: {
'has-static-classnames': [
{
props: {
label: 'Test Label',
},
},
],
},
disabledTests: ['component-has-static-classname', 'component-has-static-classname-exported'],
});

// TODO add more tests here, and create visual regression tests in /apps/vr-tests

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding the it('renders') test maybe with and without the label.

Expand Down
44 changes: 41 additions & 3 deletions packages/react-spinner/src/components/Spinner/Spinner.types.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,55 @@
import type { ComponentProps, ComponentState, Slot } from '@fluentui/react-utilities';
import { Label } from '@fluentui/react-label';

export type SpinnerSlots = {
root: Slot<'div'>;
/**
* The root of the Spinner.
* The root slot receives the `className` and `style` specified directly on the `<Spinner>`.
*/
root: NonNullable<Slot<'div'>>;
/**
* The slot for the animated svg.
* The spinner slot receives the `className` and `style` that handles the spinning animation.
* An svg is also rendered as a child of this slot
*/
spinner?: Slot<'span'>;
/**
* The label of the Slider.
* The label slot receives the styling related to the text associated with the Spinner.
*/
label?: Slot<typeof Label>;
};

type SpinnerCommons = {
// TODO Add things shared between props and state here
/**
* The appearance of the Spinner.
* @default 'primary'
*/
appearance?: 'primary' | 'inverted';

/**
* Where the label is positioned relative to the Spinner
* @default 'after'
*/
labelPosition?: 'above' | 'below' | 'before' | 'after';

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

above and below feel positional - like top and bottom. Consider only before and after and then a separate property about horizontal or vertical layout.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was discussed in the Spec PR to keep it this way to align with react-positioning: #21336 (comment)


/**
* The size of the spinner.
* @default 'medium'
*/
size?: 'tiny' | 'extra-small' | 'small' | 'medium' | 'large' | 'extra-large' | 'huge';

/**
* The status of the Spinner.
* @default 'active'
*/
status?: 'active' | 'inactive';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is status different from disabled?

};

/**
* Spinner Props
*/
export type SpinnerProps = ComponentProps<SpinnerSlots> & SpinnerCommons;
export type SpinnerProps = Omit<ComponentProps<SpinnerSlots>, 'size'> & Partial<SpinnerCommons>;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is better to have commons have the required/optional based on its usage with SpinnerProps. Then have SpinnerState make required/optional changes. This way a caller reading the code doesn't have to do mental gymnastics to figure out what is required/option in props.


/**
* State used in rendering Spinner
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,21 @@ exports[`Spinner renders a default state 1`] = `
<div
class="fui-Spinner"
>
Default Spinner
<span
class="fui-Spinner__spinner"
>
<svg
class="fui-Spinner__Progressbar"
role="progressbar"
>
<circle
class="fui-Spinner__Track"
/>
<circle
class="fui-Spinner__Tail"
/>
</svg>
</span>
</div>
</div>
`;
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,12 @@ import type { SpinnerState, SpinnerSlots } from './Spinner.types';
*/
export const renderSpinner_unstable = (state: SpinnerState) => {
const { slots, slotProps } = getSlots<SpinnerSlots>(state);

// TODO Add additional slots in the appropriate place
return <slots.root {...slotProps.root} />;
const { labelPosition, status } = state;
return (
<slots.root {...slotProps.root}>
{slots.label && (labelPosition === 'above' || labelPosition === 'before') && <slots.label {...slotProps.label} />}
{slots.spinner && status === 'active' && <slots.spinner {...slotProps.spinner} />}
{slots.label && (labelPosition === 'below' || labelPosition === 'after') && <slots.label {...slotProps.label} />}
</slots.root>
);
};
28 changes: 0 additions & 28 deletions packages/react-spinner/src/components/Spinner/useSpinner.ts

This file was deleted.

42 changes: 42 additions & 0 deletions packages/react-spinner/src/components/Spinner/useSpinner.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import * as React from 'react';
import { getNativeElementProps, resolveShorthand } from '@fluentui/react-utilities';
import type { SpinnerProps, SpinnerState } from './Spinner.types';
import { Label } from '@fluentui/react-label';
import { DefaultSvg } from './DefaultSvg';

/**
* Create the state required to render Spinner.
*
* The returned state can be modified with hooks such as useSpinnerStyles_unstable,
* before being passed to renderSpinner_unstable.
*
* @param props - props from this instance of Spinner
* @param ref - reference to root HTMLElement of Spinner
*/
export const useSpinner_unstable = (props: SpinnerProps, ref: React.Ref<HTMLElement>): SpinnerState => {
// Props
const { appearance = 'primary', labelPosition = 'after', size = 'medium', status = 'active' } = props;

const state: SpinnerState = {
appearance,
labelPosition,
size,
status,
components: {
root: 'div',
spinner: 'span',
label: Label,
},
root: getNativeElementProps('div', { ref, ...props }, ['size']),
spinner: resolveShorthand(props.spinner, {
required: true,
defaultProps: {
children: <DefaultSvg />,
},
}),
label: resolveShorthand(props.label, {
required: false,
}),
};
return state;
};
Loading