React Spinner Spec by tomi-msft · Pull Request #21336 · microsoft/fluentui

tomi-msft · 2022-01-19T15:58:13Z

This PR is to get the Spinner spec into the repo for review. We still need to decide where Spinner will live(in its own package, or under react-progress), or if it will be called Spinner. This will be moved into the right spot once it has been decided

codesandbox-ci · 2022-01-19T16:03:34Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit ff4c714:

Sandbox	Source
@fluentui/react 8 starter	Configuration
@fluentui/react-components 9 starter	Configuration

size-auditor · 2022-01-19T16:11:30Z

Asset size changes

Size Auditor did not detect a change in bundle size for any component!

Baseline commit: 0b97e0a99abaa9370fe339b62945a258d64473a7 (build)

fabricteam · 2022-01-19T16:17:17Z

📊 Bundle size report

Unchanged fixtures

Package & Exports	Size (minified/GZIP)
`react-spinner` Spinner	`16.459 kB` `5.549 kB`

_{🤖 This report was generated against 0b97e0a99abaa9370fe339b62945a258d64473a7}

fabricteam · 2022-01-19T16:24:13Z

Perf Analysis (`@fluentui/react`)

Scenario	Render type	Master Ticks	PR Ticks	Iterations	Status
ContextualMenu	mount	8451	16335	1000	Possible regression

All results

Scenario	Render type	Master Ticks	PR Ticks	Iterations	Status
Avatar	mount	1002	976	5000
BaseButton	mount	1016	1033	5000
Breadcrumb	mount	2694	2688	1000
ButtonNext	mount	524	510	5000
Checkbox	mount	1760	1707	5000
CheckboxBase	mount	1484	1521	5000
ChoiceGroup	mount	5278	5224	5000
ComboBox	mount	1106	1122	1000
CommandBar	mount	10374	10475	1000
ContextualMenu	mount	8451	16335	1000	Possible regression
DefaultButton	mount	1239	1224	5000
DetailsRow	mount	4037	3975	5000
DetailsRowFast	mount	3985	3937	5000
DetailsRowNoStyles	mount	3834	3786	5000
Dialog	mount	2411	2416	1000
DocumentCardTitle	mount	211	213	1000
Dropdown	mount	3420	3415	5000
FluentProviderNext	mount	1829	1908	5000
FluentProviderWithTheme	mount	164	183	10
FluentProviderWithTheme	virtual-rerender	109	120	10
FluentProviderWithTheme	virtual-rerender-with-unmount	194	204	10
FocusTrapZone	mount	1952	1887	5000
FocusZone	mount	1875	1836	5000
IconButton	mount	1895	1938	5000
Label	mount	406	407	5000
Layer	mount	3237	3226	5000
Link	mount	550	536	5000
MakeStyles	mount	1711	1754	50000
MenuButton	mount	1703	1620	5000
MessageBar	mount	2070	2062	5000
Nav	mount	3480	3506	1000
OverflowSet	mount	1199	1166	5000
Panel	mount	2240	2331	1000
Persona	mount	904	915	1000
Pivot	mount	1555	1562	1000
PrimaryButton	mount	1418	1380	5000
Rating	mount	8470	8469	5000
SearchBox	mount	1501	1529	5000
Shimmer	mount	2759	2731	5000
Slider	mount	2122	2140	5000
SpinButton	mount	5338	5468	5000
Spinner	mount	467	481	5000
SplitButton	mount	3398	3385	5000
Stack	mount	582	596	5000
StackWithIntrinsicChildren	mount	2575	2643	5000
StackWithTextChildren	mount	5925	5896	5000
SwatchColorPicker	mount	12267	12202	5000
TagPicker	mount	2797	2828	5000
TeachingBubble	mount	13306	13246	5000
Text	mount	508	548	5000
TextField	mount	1550	1596	5000
ThemeProvider	mount	1246	1263	5000
ThemeProvider	virtual-rerender	664	653	5000
ThemeProvider	virtual-rerender-with-unmount	2052	2052	5000
Toggle	mount	873	943	5000
buttonNative	mount	160	160	5000

Perf Analysis (`@fluentui/react-northstar`)

Perf tests with no regressions

Scenario	Current PR Ticks	Baseline Ticks	Ratio
ButtonMinimalPerf.default	209	192	1.09:1
ListWith60ListItems.default	779	716	1.09:1
TreeWith60ListItems.default	204	189	1.08:1
VideoMinimalPerf.default	736	682	1.08:1
FlexMinimalPerf.default	338	317	1.07:1
TableMinimalPerf.default	475	442	1.07:1
AttachmentMinimalPerf.default	188	178	1.06:1
GridMinimalPerf.default	412	388	1.06:1
HeaderMinimalPerf.default	430	407	1.06:1
PortalMinimalPerf.default	192	182	1.05:1
AnimationMinimalPerf.default	596	575	1.04:1
ChatDuplicateMessagesPerf.default	359	344	1.04:1
InputMinimalPerf.default	1427	1370	1.04:1
ToolbarMinimalPerf.default	1068	1028	1.04:1
AccordionMinimalPerf.default	176	171	1.03:1
AttachmentSlotsPerf.default	1200	1169	1.03:1
BoxMinimalPerf.default	411	398	1.03:1
CardMinimalPerf.default	637	618	1.03:1
LoaderMinimalPerf.default	767	744	1.03:1
MenuMinimalPerf.default	978	945	1.03:1
SliderMinimalPerf.default	1847	1789	1.03:1
TableManyItemsPerf.default	2137	2081	1.03:1
AlertMinimalPerf.default	313	307	1.02:1
DropdownMinimalPerf.default	3219	3160	1.02:1
HeaderSlotsPerf.default	915	896	1.02:1
ImageMinimalPerf.default	434	427	1.02:1
RefMinimalPerf.default	256	250	1.02:1
SplitButtonMinimalPerf.default	4855	4745	1.02:1
ChatMinimalPerf.default	826	820	1.01:1
ItemLayoutMinimalPerf.default	1327	1316	1.01:1
LabelMinimalPerf.default	448	442	1.01:1
RosterPerf.default	1408	1391	1.01:1
PopupMinimalPerf.default	675	670	1.01:1
SegmentMinimalPerf.default	401	398	1.01:1
IconMinimalPerf.default	696	687	1.01:1
TooltipMinimalPerf.default	1114	1106	1.01:1
TreeMinimalPerf.default	887	876	1.01:1
AvatarMinimalPerf.default	235	236	1:1
ButtonOverridesMissPerf.default	1815	1808	1:1
ChatWithPopoverPerf.default	416	416	1:1
CheckboxMinimalPerf.default	2842	2835	1:1
DialogMinimalPerf.default	836	837	1:1
LayoutMinimalPerf.default	413	412	1:1
ListMinimalPerf.default	586	584	1:1
ProviderMergeThemesPerf.default	1771	1764	1:1
ProviderMinimalPerf.default	1234	1234	1:1
RadioGroupMinimalPerf.default	501	502	1:1
SkeletonMinimalPerf.default	406	404	1:1
StatusMinimalPerf.default	794	793	1:1
ButtonSlotsPerf.default	589	595	0.99:1
CarouselMinimalPerf.default	529	533	0.99:1
DropdownManyItemsPerf.default	763	769	0.99:1
EmbedMinimalPerf.default	4433	4458	0.99:1
FormMinimalPerf.default	478	482	0.99:1
CustomToolbarPrototype.default	4278	4314	0.99:1
DatepickerMinimalPerf.default	5814	5916	0.98:1
ListCommonPerf.default	730	748	0.98:1
ListNestedPerf.default	646	658	0.98:1
ReactionMinimalPerf.default	434	442	0.98:1
MenuButtonMinimalPerf.default	1895	1950	0.97:1
TextAreaMinimalPerf.default	570	587	0.97:1
TextMinimalPerf.default	382	396	0.96:1
DividerMinimalPerf.default	430	455	0.95:1

tomi-msft · 2022-01-24T21:33:31Z

@@ -0,0 +1,147 @@
+# Spinner
+
+**GitHub Epic issue** - [Spinner Convergence #]()


Please add this to the document itself

behowell

Adding some comments, and waiting for the spec to be finished, including the Accessibility section.

behowell · 2022-01-25T20:10:04Z

+  /* The label prop allows user to add text. Defaults to "Loading..." */
+  label?: <Text />;


I don't think we want to include default label text, since that needs to be localized to different languages, and FluentUI doesn't include any strings built-in. Presumably it should be fine to have no label by default, and let the user supply it if they need it?

If we have a precedent for this or if the design mandates a spinner should always be accompanied by text, I'd say we should maintain a default. However, it is true we still have no localization/built-in strings but it's a need, check #19258

In v8 it has no text by default, and in this case I think that's the right pattern to carry forward

I agree, creating a spinner with a text label should be fairly simple. Without the label slot we could then leverage the children of the component for the svg

I think it makes sense for Spinner to have an optional label slot of type Label that can be conditionally rendered as needed

behowell · 2022-01-25T20:12:48Z

+  vertical?: boolean;
+  /* The size prop sets the size of the Spinner
+   * @defaultValue "medium"*/
+  size?: "tiny" | "x-small" | "small" | "medium" | "large" | "x-large" | "huge";


Spell out extra in 'extra-small' and 'extra-large' to align with other components like Badge.

theerebuss · 2022-01-26T17:59:26Z

@@ -0,0 +1,147 @@
+# Spinner
+
+**GitHub Epic issue** - [Spinner Convergence #]()


Please add this to the document itself

theerebuss · 2022-01-26T18:15:43Z

+  /* The label prop allows user to add text. Defaults to "Loading..." */
+  label?: <Text />;


If we have a precedent for this or if the design mandates a spinner should always be accompanied by text, I'd say we should maintain a default. However, it is true we still have no localization/built-in strings but it's a need, check #19258

theerebuss · 2022-01-26T18:33:07Z

Regarding naming, I'm pro Loader as it's more resilient to future design changes and it's more universally searchable.
Ideally, we should have OpenUI research for this and work with them to define and follow the standard.

ecraig12345 · 2022-01-26T20:32:20Z

Regarding naming, I'm pro Loader as it's more resilient to future design changes and it's more universally searchable. Ideally, we should have OpenUI research for this and work with them to define and follow the standard.

To me the issue with Loader is that it can imply that the component includes some kind of async loading functionality, which isn't the case. Though if that name is widely used by other libraries it might be okay. LoadingSpinner is a possible compromise but more verbose.

msft-fluent-ui-bot · 2022-02-05T00:00:02Z