Skip to content

Commit 5de569c

Browse files
cee-chencee-chen
committed
Move CurrentEuiBreakpointProvider render from EuiProvider to EuiThemeProvider
- in the previous logic, 2 resize listeners would instantiate for a top-level `<EuiProvider modify={{ breakpoint }} />` - this logic is simpler + rewrite tests for this PR to be their own describe block
1 parent 71cc628 commit 5de569c

3 files changed

Lines changed: 72 additions & 28 deletions

File tree

‎packages/eui/src/components/provider/provider.tsx‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import {
1313
EuiThemeProvider,
1414
EuiThemeProviderProps,
1515
EuiThemeSystem,
16-
CurrentEuiBreakpointProvider,
1716
} from '../../services';
1817
import { emitEuiProviderWarning } from '../../services/theme/warning';
1918
import { cache as fallbackCache } from '../../services/emotion/css';
@@ -146,9 +145,7 @@ export const EuiProvider = <T extends {} = {}>({
146145
</>
147146
)}
148147
<EuiComponentDefaultsProvider componentDefaults={componentDefaults}>
149-
<CurrentEuiBreakpointProvider>
150-
{children}
151-
</CurrentEuiBreakpointProvider>
148+
{children}
152149
</EuiComponentDefaultsProvider>
153150
</EuiThemeProvider>
154151
</EuiCacheProvider>

‎packages/eui/src/services/theme/provider.test.tsx‎

Lines changed: 65 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -74,32 +74,76 @@ describe('EuiThemeProvider', () => {
7474

7575
expect(getByText('Modified')).toHaveStyleRule('color', 'hotpink');
7676
});
77+
});
7778

78-
it('sets a conditional CurrentEuiBreakpointProvider if modify.breakpoint is passed', () => {
79-
window.innerWidth = 2500;
80-
const customBreakpoints = { xxl: 2000 };
81-
const PrintCurrentBreakpoint = () => <>{useCurrentEuiBreakpoint()}</>;
79+
describe('conditional CurrentEuiBreakpointProvider', () => {
80+
const PrintCurrentBreakpoint = () => <>{useCurrentEuiBreakpoint()}</>;
81+
let resizeListenerCount = 0;
8282

83-
const { container, rerender } = render(
84-
<EuiThemeProvider modify={{ breakpoint: customBreakpoints }}>
85-
<PrintCurrentBreakpoint />
86-
</EuiThemeProvider>
87-
);
83+
beforeAll(() => {
84+
// React 16 and 17 register a bunch of error listeners which we don't need to capture
85+
window.addEventListener = jest.fn((event: string) => {
86+
if (event === 'resize') resizeListenerCount++;
87+
});
88+
});
8889

89-
expect(container.textContent).toEqual('xxl');
90+
beforeEach(() => {
91+
// Reset counts
92+
resizeListenerCount = 0;
93+
});
9094

91-
// Does nothing if no modify.breakpoint is passed
92-
const eventListenerSpy = jest.spyOn(window, 'addEventListener');
93-
rerender(
94-
<EuiThemeProvider>
95-
<PrintCurrentBreakpoint />
96-
</EuiThemeProvider>
97-
);
98-
expect(eventListenerSpy).not.toHaveBeenCalledWith('resize');
99-
expect(container.textContent).toEqual('xl');
95+
afterAll(() => {
96+
jest.restoreAllMocks();
97+
});
10098

101-
// Reset window width to jsdom's default
102-
window.innerWidth = 1024;
99+
describe('in the top-level global theme provider', () => {
100+
it('is always rendered regardless of modified breakpoints', () => {
101+
const { container } = render(
102+
<EuiProvider>
103+
<PrintCurrentBreakpoint />
104+
</EuiProvider>
105+
);
106+
expect(container.textContent).toEqual('l');
107+
expect(resizeListenerCount).toEqual(1);
108+
});
109+
});
110+
111+
describe('in nested child theme providers', () => {
112+
beforeAll(() => {
113+
window.innerWidth = 2500;
114+
});
115+
116+
afterAll(() => {
117+
// Reset window width to jsdom's default
118+
window.innerWidth = 1024;
119+
});
120+
121+
it('is rendered if modify.breakpoint is passed', () => {
122+
const customBreakpoints = { xxl: 2000 };
123+
124+
const { container } = render(
125+
<EuiProvider>
126+
<EuiThemeProvider modify={{ breakpoint: customBreakpoints }}>
127+
<PrintCurrentBreakpoint />
128+
</EuiThemeProvider>
129+
</EuiProvider>
130+
);
131+
132+
expect(container.textContent).toEqual('xxl');
133+
expect(resizeListenerCount).toBeGreaterThanOrEqual(2); // Technically 3 due to EuiThemeProvider rerendering
134+
});
135+
136+
it('is not rendered if modify.breakpoint is not passed', () => {
137+
const { container } = render(
138+
<EuiProvider>
139+
<EuiThemeProvider>
140+
<PrintCurrentBreakpoint />
141+
</EuiThemeProvider>
142+
</EuiProvider>
143+
);
144+
expect(container.textContent).toEqual('xl');
145+
expect(resizeListenerCount).toEqual(1);
146+
});
103147
});
104148
});
105149

‎packages/eui/src/services/theme/provider.tsx‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,13 @@ export const EuiThemeProvider = <T extends {} = {}>({
8383
const prevSystemKey = useRef(system.key);
8484

8585
// To reduce the number of window resize listeners, only render a
86-
// CurrentEuiBreakpointProvider if modified breakpoint overrides are passed
86+
// CurrentEuiBreakpointProvider for the top level parent theme, or for
87+
// nested themes only if modified breakpoint overrides are passed
8788
const EuiConditionalBreakpointProvider = useMemo(() => {
88-
return _modifications?.breakpoint ? CurrentEuiBreakpointProvider : Fragment;
89-
}, [_modifications]);
89+
return isGlobalTheme || _modifications?.breakpoint
90+
? CurrentEuiBreakpointProvider
91+
: Fragment;
92+
}, [isGlobalTheme, _modifications]);
9093

9194
const [modifications, setModifications] = useState<EuiThemeModifications>(
9295
mergeDeep(parentModifications, _modifications)

0 commit comments

Comments
 (0)