Skip to content

Commit d36b926

Browse files
committed
fix: keep detecting mode changes the class filter cannot see
Watch childList on body and class on the html element, so element moves and modes applied above body are still detected. Keep isMotionDisabled reading the live DOM, since it is public and may be called while the flush cache is populated.
1 parent a72722c commit d36b926

2 files changed

Lines changed: 125 additions & 23 deletions

File tree

src/internal/visual-mode/__tests__/use-visual-mode-performance.test.tsx

Lines changed: 84 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
22
// SPDX-License-Identifier: Apache-2.0
33

4-
import React, { useRef } from 'react';
5-
import { useCurrentMode, useDensityMode } from '../index';
4+
import React, { useLayoutEffect, useRef } from 'react';
5+
import { isMotionDisabled, useCurrentMode, useDensityMode, useReducedMotion } from '../index';
66
import { render, screen } from '@testing-library/react';
77
import { mutate } from './utils';
88

@@ -44,6 +44,11 @@ function spyOnClassListReads() {
4444

4545
afterEach(() => {
4646
jest.restoreAllMocks();
47+
// Reset shared state explicitly rather than in each test, so that one failing assertion
48+
// cannot leave a mode class behind and cascade into unrelated failures.
49+
document.documentElement.className = '';
50+
document.body.className = '';
51+
document.body.replaceChildren();
4752
});
4853

4954
describe('mode detection cost', () => {
@@ -59,8 +64,9 @@ describe('mode detection cost', () => {
5964
});
6065

6166
test('shares ancestor lookups between subscribers within a single flush', async () => {
67+
const depth = 20;
6268
async function countReadsForOneFlush(detectorCount: number) {
63-
const { container, unmount } = render(<ManyDetectors count={detectorCount} depth={20} />);
69+
const { container, unmount } = render(<ManyDetectors count={detectorCount} depth={depth} />);
6470
const reads = spyOnClassListReads();
6571
await mutate(() => container.classList.add('unrelated-class'));
6672
const total = reads.mock.calls.length;
@@ -69,14 +75,16 @@ describe('mode detection cost', () => {
6975
return total;
7076
}
7177

72-
const withTen = await countReadsForOneFlush(10);
7378
const withForty = await countReadsForOneFlush(40);
74-
expect(withTen).toBeGreaterThan(0);
75-
76-
// Each subscriber walks to the document root, so without memoization 4x the subscribers
77-
// costs ~4x the class reads. Sharing the resolved path within a flush means only each
78-
// subscriber's own element is uncached, making the growth far sublinear in chain depth.
79-
expect(withForty).toBeLessThan(withTen * 2);
79+
const withEighty = await countReadsForOneFlush(80);
80+
81+
// Assert the marginal cost of one more subscriber, which is what memoization bounds.
82+
// Each additional subscriber should only read its own element's classList, once per
83+
// detected mode, because its ancestors were already resolved by an earlier subscriber.
84+
// Without memoization each one re-walks the shared chain instead, so the marginal cost
85+
// would scale with `depth`.
86+
const marginalReadsPerSubscriber = (withEighty - withForty) / 40;
87+
expect(marginalReadsPerSubscriber).toBeLessThan(depth / 2);
8088
});
8189

8290
test('does not reuse cached lookups across separate flushes', async () => {
@@ -111,6 +119,72 @@ describe('mode detection cost', () => {
111119
expect(screen.getByTestId('in-neither')).toHaveTextContent('light-comfortable');
112120
});
113121

122+
test('detects a mode class applied above body', async () => {
123+
render(<ModeRender testId="detector" />);
124+
expect(screen.getByTestId('detector')).toHaveTextContent('light-comfortable');
125+
126+
await mutate(() => document.documentElement.classList.add('awsui-dark-mode'));
127+
expect(screen.getByTestId('detector')).toHaveTextContent('dark-comfortable');
128+
129+
await mutate(() => document.documentElement.classList.remove('awsui-dark-mode'));
130+
expect(screen.getByTestId('detector')).toHaveTextContent('light-comfortable');
131+
});
132+
133+
test('detects a move into a subtree with a different mode', async () => {
134+
const host = document.createElement('div');
135+
document.body.appendChild(host);
136+
const darkSubtree = document.createElement('div');
137+
darkSubtree.className = 'awsui-dark-mode';
138+
document.body.appendChild(darkSubtree);
139+
140+
render(<ModeRender testId="detector" />, { container: host });
141+
expect(screen.getByTestId('detector')).toHaveTextContent('light-comfortable');
142+
143+
// No class changes here: only the ancestor chain does.
144+
await mutate(() => darkSubtree.appendChild(host));
145+
expect(screen.getByTestId('detector')).toHaveTextContent('dark-comfortable');
146+
147+
await mutate(() => document.body.appendChild(host));
148+
expect(screen.getByTestId('detector')).toHaveTextContent('light-comfortable');
149+
});
150+
151+
test('isMotionDisabled reads the live DOM when called during a fan-out', async () => {
152+
const wrapper = document.createElement('div');
153+
document.body.appendChild(wrapper);
154+
let observedDuringFlush: boolean | undefined = undefined;
155+
156+
function MotionSubscriber() {
157+
const ref = useRef(null);
158+
useReducedMotion(ref);
159+
return <div ref={ref} />;
160+
}
161+
162+
// This effect runs inside the fan-out, after MotionSubscriber has already resolved (and
163+
// cached) the motion lookup for the shared ancestor chain.
164+
function MutatingDuringFlush() {
165+
const ref = useRef<HTMLDivElement>(null);
166+
const colorMode = useCurrentMode(ref);
167+
useLayoutEffect(() => {
168+
if (colorMode === 'dark' && ref.current) {
169+
wrapper.classList.add('awsui-motion-disabled');
170+
observedDuringFlush = isMotionDisabled(ref.current);
171+
}
172+
}, [colorMode]);
173+
return <div ref={ref} />;
174+
}
175+
176+
render(
177+
<>
178+
<MotionSubscriber />
179+
<MutatingDuringFlush />
180+
</>,
181+
{ container: wrapper }
182+
);
183+
await mutate(() => wrapper.classList.add('awsui-dark-mode'));
184+
185+
expect(observedDuringFlush).toBe(true);
186+
});
187+
114188
test('detects a mode applied to an intermediate ancestor rather than body', async () => {
115189
const { container } = render(
116190
<div className="outer">

src/internal/visual-mode/index.ts

Lines changed: 41 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,14 @@ import { safeMatchMedia } from '../utils/safe-match-media.js';
1313

1414
/**
1515
* Ancestor-chain lookups resolved during the current mutation flush, keyed by mode and then
16-
* by element. Only populated while the singleton observer is fanning out (see
17-
* `useMutationSingleton`); `null` at all other times, so a stale entry can never be read.
16+
* by element. Only populated while the singleton observer is fanning out to its subscribers
17+
* (see `useMutationSingleton`), and `null` at all other times.
18+
*
19+
* Only the detectors below read it. Callers outside the fan-out must not, because React can
20+
* run effects while it is populated: an effect that mutates a mode class and then queries
21+
* synchronously has to see the DOM as it is, not as the flush found it. The detectors
22+
* themselves are unaffected, since they all run before any effect in the batch and any
23+
* mutation an effect makes schedules a fresh flush.
1824
*/
1925
let flushCache: null | Map<string, Map<HTMLElement, boolean>> = null;
2026

@@ -73,7 +79,17 @@ function hasMatchingAncestor(mode: string, element: HTMLElement, test: (node: HT
7379
return resolved;
7480
}
7581

82+
function hasMotionDisabledAncestor(element: HTMLElement): boolean {
83+
return !!findUpUntil(element, node => node.classList.contains('awsui-motion-disabled'));
84+
}
85+
86+
// Public API, callable at any time, so it always reads the live DOM rather than the flush cache.
7687
export function isMotionDisabled(element: HTMLElement): boolean {
88+
return hasMotionDisabledAncestor(element) || safeMatchMedia(element, '(prefers-reduced-motion: reduce)');
89+
}
90+
91+
// Equivalent to `isMotionDisabled`, but shares ancestor lookups across subscribers in a flush.
92+
function detectReducedMotion(element: HTMLElement): boolean {
7793
return (
7894
hasMatchingAncestor('motion', element, node => node.classList.contains('awsui-motion-disabled')) ||
7995
safeMatchMedia(element, '(prefers-reduced-motion: reduce)')
@@ -135,27 +151,39 @@ export function useDensityMode(elementRef: React.RefObject<HTMLElement>) {
135151
}
136152

137153
export function useReducedMotion(elementRef: React.RefObject<HTMLElement>) {
138-
return useModeDetector(elementRef, isMotionDisabled, false);
154+
return useModeDetector(elementRef, detectReducedMotion, false);
139155
}
140156

141157
const useMutationSingleton = createSingletonHandler<void>(handler => {
142-
const observer = new MutationObserver(() => {
158+
const fanOut = () => {
143159
// Memoize ancestor lookups for the duration of this fan-out only. Every subscriber runs
144-
// synchronously inside handler(), so the cache is scoped to a single consistent view of
145-
// the DOM and is discarded before any further mutations can be observed.
160+
// synchronously inside handler(), before React processes any effect in the batch, so all
161+
// of them see one consistent view of the DOM.
146162
flushCache = new Map();
147163
try {
148164
handler();
149165
} finally {
150166
flushCache = null;
151167
}
152-
});
153-
// Every mode is expressed as a class name, so class is the only attribute whose change can
154-
// affect a detected mode. Filtering here avoids waking all subscribers for unrelated
155-
// attribute changes anywhere in the document, such as the `data-awsui-focus-visible`
156-
// toggle that focus-visible writes to `<body>` on every keydown and mousedown.
157-
observer.observe(document.body, { attributes: true, subtree: true, attributeFilter: ['class'] });
158-
return () => observer.disconnect();
168+
};
169+
const observer = new MutationObserver(fanOut);
170+
const htmlObserver = new MutationObserver(fanOut);
171+
// A mode is only ever expressed as a class name, so watching `class` is what detects a mode
172+
// change. Filtering to it avoids waking every subscriber for unrelated attribute changes
173+
// anywhere in the document, such as the `data-awsui-focus-visible` toggle that
174+
// focus-visible writes to `<body>` on every keydown and mousedown.
175+
//
176+
// `childList` is watched as well because moving an element between subtrees changes its
177+
// ancestor chain, and therefore its mode, without any class changing. Previously such
178+
// moves were only picked up incidentally, by whatever unrelated attribute mutation
179+
// happened to follow.
180+
observer.observe(document.body, { attributes: true, subtree: true, childList: true, attributeFilter: ['class'] });
181+
// Modes are also honoured above `<body>`, which the observer above does not cover.
182+
htmlObserver.observe(document.documentElement, { attributes: true, attributeFilter: ['class'] });
183+
return () => {
184+
observer.disconnect();
185+
htmlObserver.disconnect();
186+
};
159187
});
160188

161189
function useMutationObserver(elementRef: React.RefObject<HTMLElement>, onChange: (element: HTMLElement) => void) {

0 commit comments

Comments
 (0)