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
25 changes: 25 additions & 0 deletions docs/KNOWN_ISSUES.md
Original file line number Diff line number Diff line change
Expand Up @@ -95,3 +95,28 @@
HTTPS + system trust store + code-signature verification of the downloaded artifact. The blast
radius of a stale or broken pin exceeds the risk it would remove.
- Affected files: electron-builder.json (update feed config); no source changes made.

## `prefers-reduced-motion` diverges across CI runners — `getComputedStyle` transition assertions are platform-flaky

- Status: Confirmed (GitHub-hosted `windows-latest`/`macos-latest` runners, PR #3443 and #3449
CI runs; local dev Macs and the `ubuntu-latest`/xvfb runner unaffected).
- Symptom: A renderer spec that asserts on a CSS transition/animation property via
`getComputedStyle(element).transitionDuration` (or similar) passes locally and on Linux CI but
fails or reports a different value on GitHub-hosted Windows and macOS runners.
- Root cause: `getComputedStyle` resolves the live cascade, which means it evaluates the
`prefers-reduced-motion` media query against the runner's actual OS-level accessibility
setting. GitHub-hosted Windows and macOS runner images report `prefers-reduced-motion: reduce`
at the OS level, while the `ubuntu-latest` + xvfb runner and local developer Macs report
`no-preference`. A component that opts out of animation under reduced motion therefore renders
with different computed transition values depending solely on which CI platform is running the
test — the assertion is checking the runner's OS setting, not the component's logic.
- Workaround: Do not assert transition/animation values via `getComputedStyle` in specs. Instead,
read the parsed CSSOM directly — walk `document.styleSheets`, find the plain style rule and the
`@media (prefers-reduced-motion: reduce)` override rule that target the element's own
emotion-generated class. Both rules exist in the CSSOM regardless of which value the media
query currently resolves to on that runner, so the assertion is deterministic on every
platform. See `findTransitionRulesForElement` in
`src/ui/components/TopBar/DownloadsIndicator.spec.tsx` (~lines 80-138) for the pattern.
- Affected files: src/ui/components/TopBar/DownloadsIndicator.spec.tsx.
- Reference: PR #3443 (introduced the animated percentage slot), PR #3449 (documented after CI
runs surfaced the divergence).
44 changes: 44 additions & 0 deletions src/ui/components/Shell/index.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,16 @@ jest.mock('../TopBar', () => ({
__esModule: true,
TopBar: ({
leadingSlot,
centerSlot,
trailingSlot,
}: {
leadingSlot?: React.ReactNode;
centerSlot?: React.ReactNode;
trailingSlot?: React.ReactNode;
}) => (
<div data-testid='top-bar'>
{leadingSlot}
{centerSlot}
{trailingSlot}
</div>
),
Expand Down Expand Up @@ -410,6 +413,47 @@ describe('Shell', () => {
});
});

describe('linux chrome', () => {
let restorePlatform: () => void;

afterEach(() => {
restorePlatform?.();
});

it('mounts the TopBar with the downloads indicator, and no window controls, when navigationLayout is sidebar', () => {
restorePlatform = setPlatform('linux');

renderWithStore(<Shell />, {
preloadedState: buildState({ navigationLayout: 'sidebar' }),
});

expect(screen.getByTestId('top-bar')).toBeInTheDocument();
expect(screen.getByTestId('downloads-indicator')).toBeInTheDocument();
expect(screen.queryByTestId('window-controls')).not.toBeInTheDocument();
expect(screen.getByTestId('tab-bar')).toHaveAttribute(
'data-orientation',
'vertical'
);
expect(screen.getByTestId('meatball-menu-button')).toBeInTheDocument();
});

it('mounts the TopBar with the downloads indicator and server switcher, and no window controls or TabBar, when navigationLayout is hidden', () => {
restorePlatform = setPlatform('linux');

renderWithStore(<Shell />, {
preloadedState: buildState({ navigationLayout: 'hidden' }),
});

expect(screen.getByTestId('top-bar')).toBeInTheDocument();
expect(screen.getByTestId('downloads-indicator')).toBeInTheDocument();
expect(screen.queryByTestId('window-controls')).not.toBeInTheDocument();
expect(screen.queryByTestId('tab-bar')).not.toBeInTheDocument();
expect(
screen.getByRole('button', { name: 'tabBar.workspaces' })
).toBeInTheDocument();
});
});

describe('downloads indicator: exactly one instance, compact in the thin TopBar', () => {
it('renders exactly one instance on the darwin TopBar (sidebar layout), compact', () => {
const restorePlatform = setPlatform('darwin');
Expand Down
27 changes: 14 additions & 13 deletions src/ui/components/Shell/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -95,19 +95,20 @@ export const Shell = () => {
}
/>
)}
{navigationLayout !== 'tabs' && process.platform === 'darwin' && (
<TopBar
centerSlot={
navigationLayout === 'hidden' ? <ServerSwitcher /> : undefined
}
trailingSlot={
<>
<UpdateLabel />
<DownloadsIndicator compact />
</>
}
/>
)}
{navigationLayout !== 'tabs' &&
['darwin', 'linux'].includes(process.platform) && (
<TopBar
centerSlot={
navigationLayout === 'hidden' ? <ServerSwitcher /> : undefined
}
trailingSlot={
<>
<UpdateLabel />
<DownloadsIndicator compact />
</>
}
/>
)}
{navigationLayout !== 'tabs' && process.platform === 'win32' && (
<TopBar
leadingSlot={
Expand Down
64 changes: 39 additions & 25 deletions src/ui/components/TopBar/DownloadsIndicator.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,9 @@ describe('DownloadsIndicator', () => {
}),
});

expect(screen.getByTestId('downloads-unseen-dot')).toBeInTheDocument();
expect(
await screen.findByTestId('downloads-unseen-dot')
).toBeInTheDocument();

await user.click(
screen.getByRole('button', { name: 'tabBar.downloads.title' })
Expand Down Expand Up @@ -710,6 +712,8 @@ describe('DownloadsIndicator', () => {
}),
});

screen.getByTestId('downloads-unseen-dot');

const glyph = screen.getByTestId('downloads-glyph');

const paths = glyph.querySelectorAll('path');
Expand All @@ -731,34 +735,44 @@ describe('DownloadsIndicator', () => {
// seenAt is initialized to the mount-time Date.now(); endTime must sit
// between that mount time and the later click's own Date.now() call
// for the download to start unseen and end up seen after the click.
const mountTime = Date.now();
renderWithStore(<DownloadsIndicator />, {
preloadedState: buildState({
1: {
...baseDownload,
state: 'completed',
receivedBytes: 1000,
startTime: SESSION_START + 1000,
endTime: mountTime + 1,
},
}),
});
// Date.now is pinned for the duration of this test to make that
// ordering deterministic instead of racing the wall clock.
const NOW = Date.now();
const spy = jest.spyOn(Date, 'now').mockReturnValue(NOW);

try {
renderWithStore(<DownloadsIndicator />, {
preloadedState: buildState({
1: {
...baseDownload,
state: 'completed',
receivedBytes: 1000,
startTime: SESSION_START + 1000,
endTime: NOW + 1,
},
}),
});

expect(screen.getByTestId('downloads-unseen-dot')).toBeInTheDocument();
expect(screen.getByTestId('downloads-unseen-dot')).toBeInTheDocument();

await user.click(
screen.getByRole('button', { name: 'tabBar.downloads.title' })
);
await user.click(screen.getByTestId('downloads-panel-backdrop'));
spy.mockReturnValue(NOW + 60_000);

const glyph = screen.getByTestId('downloads-glyph');
const paths = glyph.querySelectorAll('path');
expect(paths).toHaveLength(1);
expect(paths[0].getAttribute('d')).toContain(TRACK_CIRCLE_D);
expect(paths[0].getAttribute('d')).toContain(OUTER_CIRCLE_D);
expect(paths[0].getAttribute('d')).toContain(ARROW_PATH);
await user.click(
screen.getByRole('button', { name: 'tabBar.downloads.title' })
);
await user.click(screen.getByTestId('downloads-panel-backdrop'));

expect(glyph.querySelectorAll('circle')).toHaveLength(0);
const glyph = screen.getByTestId('downloads-glyph');
const paths = glyph.querySelectorAll('path');
expect(paths).toHaveLength(1);
expect(paths[0].getAttribute('d')).toContain(TRACK_CIRCLE_D);
expect(paths[0].getAttribute('d')).toContain(OUTER_CIRCLE_D);
expect(paths[0].getAttribute('d')).toContain(ARROW_PATH);

expect(glyph.querySelectorAll('circle')).toHaveLength(0);
} finally {
spy.mockRestore();
}
});

it('does not spin the unseen-completed arc (indeterminate spin only applies while actually downloading)', () => {
Expand Down
Loading