wip: get majority of component updated

Parkreiner · Parkreiner · commit 2f356bc783df · 2025-02-07T16:28:21.000Z
diff --git a/site/src/components/Spinner/Spinner.tsx b/site/src/components/Spinner/Spinner.tsx
@@ -42,8 +42,60 @@ export const Spinner: FC<SpinnerProps> = ({
 	unmountedWhileLoading = false,
 	...delegatedProps
 }) => {
-	// Doing some mid-render state syncs to minimize re-renders and risks of
-	// contradictory states. It's ugly, but it's what the React team recommends
+	const showSpinner = useShowSpinner(loading, spinnerStartDelayMs);
+	if (!showSpinner) {
+		return children;
+	}
+
+	return (
+		<>
+			<svg
+				// Fill is the only prop that should be allowed to be
+				// overridden; all other props must come after destructuring
+				fill="currentColor"
+				{...delegatedProps}
+				viewBox="0 0 24 24"
+				xmlns="http://www.w3.org/2000/svg"
+				className={cn(className, spinnerVariants({ size }))}
+			>
+				<title>Loading&hellip;</title>
+				{leavesIterable.map((leafIndex) => (
+					<rect
+						key={leafIndex}
+						x="10.9"
+						y="2"
+						width="2"
+						height="5.5"
+						rx="1"
+						className={
+							// 0.8 is hard-coded because of Tailwind; the value
+							// should always be (0.1 * SPINNER_LEAF_COUNT)
+							isChromatic() ? "" : "animate-[loading_0.8s_ease-in-out_infinite]"
+						}
+						style={{
+							transform: `rotate(${leafIndex * (360 / SPINNER_LEAF_COUNT)}deg)`,
+							transformOrigin: "center",
+							animationDelay: `${-leafIndex * 0.1}s`,
+						}}
+					/>
+				))}
+			</svg>
+			{!unmountedWhileLoading && <div className="sr-only">{children}</div>}
+		</>
+	);
+};
+
+// Not a big fan of one-time custom hooks, but it helps insulate the main
+// component from the chaos of handling all these states, when the ultimate
+// result is a simple boolean. V8 will be able to inline the function definition
+// in some cases anyway
+function useShowSpinner(
+	loading: boolean,
+	spinnerStartDelayMs: number,
+): boolean {
+	// Doing a bunch of mid-render state syncs to minimize risks of
+	// contradictory states during re-renders. It's ugly, but it's what the
+	// React team officially recommends
 	const noDelay = spinnerStartDelayMs === 0;
 	const [mountSpinner, setMountSpinner] = useState(noDelay);
 	const unmountImmediatelyOnRerender = mountSpinner && !loading;
@@ -65,42 +117,5 @@ export const Spinner: FC<SpinnerProps> = ({
 		return () => window.clearTimeout(delayId);
 	}, [spinnerStartDelayMs]);
 
-	// Past this point, only showSpinner should need to be referenced
-	const showSpinner = loading && mountSpinner;
-	if (!showSpinner) {
-		return children;
-	}
-
-	return (
-		<svg
-			// Fill is the only prop that should be allowed to be overridden;
-			// all other props must come after destructuring
-			fill="currentColor"
-			{...delegatedProps}
-			viewBox="0 0 24 24"
-			xmlns="http://www.w3.org/2000/svg"
-			className={cn(className, spinnerVariants({ size }))}
-		>
-			<title>Loading&hellip;</title>
-			{leavesIterable.map((leafIndex) => (
-				<rect
-					key={leafIndex}
-					x="10.9"
-					y="2"
-					width="2"
-					height="5.5"
-					rx="1"
-					// 0.8 = leaves * 0.1
-					className={
-						isChromatic() ? "" : "animate-[loading_0.8s_ease-in-out_infinite]"
-					}
-					style={{
-						transform: `rotate(${leafIndex * (360 / SPINNER_LEAF_COUNT)}deg)`,
-						transformOrigin: "center",
-						animationDelay: `${-leafIndex * 0.1}s`,
-					}}
-				/>
-			))}
-		</svg>
-	);
-};
+	return loading && mountSpinner;
+}
