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
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,15 @@ import type { LodyConnectionUiState } from '@/atoms/control-connection';
* - The whole pill springs in / out on appear / disappear; while it stays
* mounted, its inner content cross-fades between states so e.g. the
* loader → ✓ swap reads as a smooth transition rather than a jump.
*
* Every leading glyph carries `shrink-0`. The pill is a capped-width flex
* row whose label truncates, so without it a long label compresses the
* spinner to a non-square box (measured 14×14 → 13.55×15.43 on the
* reconnecting label) and `animate-spin` then sweeps an ellipse — the
* glyph visibly wobbles instead of turning in place. `index.css` also
* pins `transform-box`/`transform-origin` globally for spinners; both
* are needed, since that rule fixes the pivot but cannot restore a
* squished box.
*/

export type MobileConnectionStatusLabels = {
Expand Down Expand Up @@ -114,7 +123,7 @@ function contentFor(
both at once would be visual noise. */
return {
key: 'refreshing',
icon: <Loader2 className="h-3.5 w-3.5 animate-spin" aria-hidden="true" />,
icon: <Loader2 className="h-3.5 w-3.5 shrink-0 animate-spin" aria-hidden="true" />,
label: labels.refreshing ?? '刷新中…',
textColor: 'text-muted-foreground',
};
Expand All @@ -130,15 +139,15 @@ function contentFor(
if (state === 'reconnecting') {
return {
key: 'reconnecting',
icon: <Loader2 className="h-3.5 w-3.5 animate-spin" aria-hidden="true" />,
icon: <Loader2 className="h-3.5 w-3.5 shrink-0 animate-spin" aria-hidden="true" />,
label: labels.reconnecting ?? '正在重连…',
textColor: 'text-muted-foreground',
};
}
if (state === 'loading') {
return {
key: 'loading',
icon: <Loader2 className="h-3.5 w-3.5 animate-spin" aria-hidden="true" />,
icon: <Loader2 className="h-3.5 w-3.5 shrink-0 animate-spin" aria-hidden="true" />,
label: labels.loading ?? '连接中…',
textColor: 'text-muted-foreground',
};
Expand Down
50 changes: 50 additions & 0 deletions packages/components/src/tailwind/index.css
Original file line number Diff line number Diff line change
Expand Up @@ -1791,6 +1791,56 @@ pre,
}
}

/*
---break---
*/

/* Spinners must rotate about their OWN center, not orbit a point.
`animate-spin` is `rotate(360deg)` with the default
`transform-origin: 50% 50%`, and `transform-box`'s initial value is
`view-box` — for an SVG that percentage resolves against the nearest
SVG viewport rather than the element's CSS box. A root `<svg>` has a
CSS layout box, so `view-box` is specified to act as `border-box`
there and modern engines land on the box center. But nothing in the
markup states that intent, so any element that loses its CSS layout
box, is sized asymmetrically, or is rotated by an engine resolving
the percentage against the 24-unit `viewBox` pivots off-center — the
glyph then sweeps a circle around a point beside it (measured: a
14px icon pivoting on the viewBox center orbits with a ~14px
diameter, i.e. wider than the icon itself) instead of turning in
place. Pinning both properties states the intent once, globally.

`flex-shrink: 0` belongs here for the same reason: in a tight flex
row (the mobile home status pill, whose label truncates) a spinner
without it is compressed to a non-square box, so the rotation
sweeps an ellipse and the glyph visibly wobbles. Measured 14×14 →
13.55×15.43 in the reconnecting pill before this rule.

`@layer base` keeps this below utilities, so an explicit
`origin-*` / `shrink` utility at a call site still wins. */
@layer base {
svg.animate-spin {
transform-box: border-box;
transform-origin: 50% 50%;
flex-shrink: 0;
}

/* Two sites animate an HTML wrapper instead of the glyph, to keep an
always-on spinner on the compositor (see `sidebar-row-shared.tsx`
and `session-syncing-indicator.tsx`). A wrapper is only equivalent
while its box matches the glyph's: the moment anything else lands
inside it (a label, a gap) its box grows, its center moves off the
glyph, and the glyph orbits — measured 64px of travel for a
wrapper that also contained its "Syncing" label. These wrappers
must therefore stay icon-only and explicitly sized; centering the
origin here makes that failure mode a wobble-free no-op rather
than an orbit. */
span.animate-spin {
transform-box: border-box;
transform-origin: 50% 50%;
}
}

/*
---break---
*/
Expand Down
124 changes: 124 additions & 0 deletions packages/components/tests/spinner-rotates-in-place.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
// @vitest-environment jsdom

/**
* A spinner must rotate about its own center. `animate-spin` is
* `rotate(360deg)` about `transform-origin: 50% 50%`, so the glyph turns in
* place only while the animated element's box is square AND centered on the
* glyph. Two ways that breaks, both of which shipped as visible bugs:
*
* 1. The animated element is an HTML *wrapper* that contains more than the
* glyph (a label, a gap). Its box grows, its center moves off the glyph,
* and the glyph orbits that offset center — a wrapper that also held its
* "Syncing" label measured 64px of travel in WebKit.
* 2. The glyph sits in a tight flex row with a truncating label and no
* `shrink-0`, so it is compressed to a non-square box and the rotation
* sweeps an ellipse (measured 14×14 → 13.55×15.43).
*
* jsdom has no layout, so these assert the *structural* invariants that make
* the geometry correct rather than re-measuring pixels: an animated wrapper
* is icon-only and explicitly sized, and a spinner in a flex row cannot be
* squished. The `transform-box`/`transform-origin` half of the fix lives in
* `src/tailwind/index.css` and is not observable here.
*/

import React from 'react';
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
import { flushSync } from 'react-dom';
import { createRoot, type Root } from 'react-dom/client';
import { SessionRowLeadingSlot } from '../src/components/sidebar-row-shared';
import { SessionSyncingIndicator } from '../src/components/sessions/session-syncing-indicator';
import { MobileConnectionStatus } from '../src/components/mobile/mobile-connection-status';
import { initI18n } from '../src/i18n';

/** A sizing utility that pins BOTH axes, e.g. `h-3 w-3` / `size-4`. */
function hasExplicitSquareSize(el: Element): boolean {
const classes = [...el.classList];
const has = (prefix: string) =>
classes.some((c) => new RegExp(String.raw`^-?${prefix}-\[?[\d./]`).test(c));
return (has('h') && has('w')) || has('size');
}

let container: HTMLDivElement;
let root: Root | undefined;

beforeEach(async () => {
await initI18n();
container = document.createElement('div');
document.body.appendChild(container);
root = createRoot(container);
});

afterEach(() => {
flushSync(() => root?.unmount());
root = undefined;
container.remove();
});

function render(node: React.ReactElement) {
flushSync(() => root?.render(node));
}

describe('spinners rotate in place', () => {
/* The compositor-friendly pattern: animate an HTML wrapper, not the SVG.
Only equivalent while the wrapper's box IS the glyph's box. */
const wrapperCases: ReadonlyArray<readonly [string, () => React.ReactElement]> = [
[
'session row working indicator',
() =>
React.createElement(SessionRowLeadingSlot, { isWorking: true, menuLabel: 'More actions' }),
],
['session syncing indicator', () => React.createElement(SessionSyncingIndicator, {})],
];

for (const [name, element] of wrapperCases) {
it(`${name}: animated wrapper is icon-only and explicitly sized`, () => {
render(element());

const wrappers = [...container.querySelectorAll('span.animate-spin')];
expect(wrappers.length).toBe(1);
const wrapper = wrappers[0]!;

// Square, explicitly sized: the wrapper's center is the glyph's center.
expect(hasExplicitSquareSize(wrapper)).toBe(true);

// Icon-only. Any extra content (a label, a sibling) shifts the box
// center off the glyph and turns the spin into an orbit.
expect(wrapper.childElementCount).toBe(1);
expect(wrapper.firstElementChild?.tagName).toBe('svg');
expect(wrapper.textContent).toBe('');

// The glyph itself must NOT also spin, or the two rotations compound.
expect(wrapper.querySelector('svg')?.classList.contains('animate-spin')).toBe(false);
});
}

/* The mobile home status pill: a capped-width flex row with a truncating
label, i.e. exactly the layout that squishes an unprotected spinner. */
const pillStates = [
{ label: 'refreshing', props: { state: 'online', refreshing: true } },
{ label: 'reconnecting', props: { state: 'reconnecting' } },
{ label: 'loading', props: { state: 'loading' } },
] as const;

for (const { label, props } of pillStates) {
it(`mobile status pill (${label}): spinner cannot be squished by the label`, () => {
render(
React.createElement(MobileConnectionStatus, {
...props,
labels: {
refreshing: '正在刷新工作区,请稍候等待同步完成',
reconnecting: '正在重新连接到工作区,请稍候等待',
loading: '正在连接到工作区,请稍候等待',
},
} as React.ComponentProps<typeof MobileConnectionStatus>)
);

const spinner = container.querySelector('svg.animate-spin');
expect(spinner).not.toBeNull();
// Without shrink-0 the flex row compresses the glyph to a non-square
// box and the rotation sweeps an ellipse.
expect(spinner!.classList.contains('shrink-0')).toBe(true);
expect(hasExplicitSquareSize(spinner!)).toBe(true);
});
}
});