Skip to content

Commit a2df03d

Browse files
authored
[React 18] Add full <StrictMode> support (#7007)
1 parent 66ff657 commit a2df03d

File tree

6 files changed

+67
-52
lines changed

6 files changed

+67
-52
lines changed

src/components/context_menu/context_menu_panel.spec.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ describe('EuiContextMenuPanel', () => {
7979
]}
8080
/>
8181
);
82+
cy.get('.euiContextMenuPanel').should('be.focused');
8283
cy.realPress('{downarrow}');
8384
cy.focused().should('have.attr', 'data-test-subj', 'itemA');
8485
});
@@ -100,6 +101,7 @@ describe('EuiContextMenuPanel', () => {
100101
);
101102
};
102103
cy.mount(<DynanicItemsTest />);
104+
cy.get('.euiContextMenuPanel').should('be.focused');
103105
cy.realPress('{downarrow}');
104106
cy.focused().should('have.attr', 'data-test-subj', 'itemA');
105107
cy.realPress('{downarrow}');
@@ -144,6 +146,7 @@ describe('EuiContextMenuPanel', () => {
144146

145147
it('focuses the back button panel title by default when no initialFocusedItemIndex is passed', () => {
146148
cy.mount(<EuiContextMenu panels={panels} initialPanelId="A" />);
149+
cy.get('.euiContextMenuPanel').should('be.focused');
147150
cy.realPress('{downarrow}');
148151
cy.realPress('{downarrow}');
149152
cy.focused().should('have.attr', 'data-test-subj', 'panelA');
@@ -155,6 +158,7 @@ describe('EuiContextMenuPanel', () => {
155158

156159
it('focuses the correct toggling item when using the left arrow key to navigate to the previous panel', () => {
157160
cy.mount(<EuiContextMenu panels={panels} initialPanelId="B" />);
161+
cy.get('[data-test-subj="panelB"]').should('be.focused');
158162
cy.realPress('{leftarrow}');
159163
cy.focused().should('have.attr', 'data-test-subj', 'panelA');
160164
});

src/components/datagrid/body/data_grid_cell_popover.spec.tsx

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,10 @@ describe('EuiDataGridCellPopover', () => {
6161
);
6262

6363
cy.realPress('Escape');
64-
cy.focused()
65-
.should('have.attr', 'data-gridcell-column-index', '0')
66-
.should('have.attr', 'data-gridcell-row-index', '0');
64+
65+
cy.get(
66+
'[data-gridcell-column-index="0"][data-gridcell-row-index="0"]'
67+
).should('be.focused');
6768
});
6869

6970
it('when the expand button is clicked and then F2 key is pressed', () => {

src/components/flyout/flyout.spec.tsx

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -53,15 +53,11 @@ describe('EuiFlyout', () => {
5353

5454
it('traps focus and cycles tabbable items', () => {
5555
cy.mount(<Flyout />);
56-
cy.wait(100); // wait for focus lib to focus the right element
56+
cy.get('[data-test-subj="flyoutSpec"]').should('be.focused');
5757
cy.repeatRealPress('Tab', 4);
58-
cy.focused().should('have.attr', 'data-test-subj', 'itemC');
58+
cy.get('[data-test-subj="itemC"]').should('be.focused');
5959
cy.repeatRealPress('Tab', 3);
60-
cy.focused().should(
61-
'have.attr',
62-
'data-test-subj',
63-
'euiFlyoutCloseButton'
64-
);
60+
cy.get('[data-test-subj="euiFlyoutCloseButton"]').should('be.focused');
6561
});
6662

6763
it('does not focus trap or scrollLock for push flyouts', () => {
@@ -130,7 +126,10 @@ describe('EuiFlyout', () => {
130126

131127
it('closes the flyout when the overlay mask is clicked', () => {
132128
cy.mount(<Flyout />);
133-
cy.get('.euiOverlayMask').should('be.visible').realClick();
129+
cy.get('[data-test-subj="flyoutSpec"]').should('be.visible');
130+
cy.get('.euiOverlayMask')
131+
.should('be.visible')
132+
.realClick({ position: 'left' });
134133
cy.get('[data-test-subj="flyoutSpec"]').should('not.exist');
135134
});
136135

src/components/popover/wrapping_popover.tsx

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,6 @@ export interface EuiWrappingPopoverProps
2222
*/
2323
export class EuiWrappingPopover extends Component<EuiWrappingPopoverProps> {
2424
private portal: HTMLElement | null = null;
25-
private anchor: HTMLElement | null = null;
26-
27-
componentDidMount() {
28-
if (this.anchor) {
29-
this.anchor.insertAdjacentElement('beforebegin', this.props.button);
30-
}
31-
}
3225

3326
componentWillUnmount() {
3427
if (this.props.button.parentNode) {
@@ -43,7 +36,7 @@ export class EuiWrappingPopover extends Component<EuiWrappingPopoverProps> {
4336
};
4437

4538
setAnchorRef = (node: HTMLElement | null) => {
46-
this.anchor = node;
39+
node?.insertAdjacentElement('beforebegin', this.props.button);
4740
};
4841

4942
render() {

src/components/portal/portal.tsx

Lines changed: 38 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
* into portals.
1212
*/
1313

14-
import React, { Component, ContextType, ReactNode } from 'react';
14+
import { Component, ContextType, ReactNode } from 'react';
1515
import { createPortal } from 'react-dom';
1616

1717
import { EuiNestedThemeContext } from '../../services';
@@ -41,63 +41,79 @@ export interface EuiPortalProps {
4141
portalRef?: (ref: HTMLDivElement | null) => void;
4242
}
4343

44-
export class EuiPortal extends Component<EuiPortalProps> {
44+
interface EuiPortalState {
45+
portalNode: HTMLDivElement | null;
46+
}
47+
48+
export class EuiPortal extends Component<EuiPortalProps, EuiPortalState> {
4549
static contextType = EuiNestedThemeContext;
4650
declare context: ContextType<typeof EuiNestedThemeContext>;
4751

48-
portalNode: HTMLDivElement | null = null;
49-
5052
constructor(props: EuiPortalProps) {
5153
super(props);
52-
if (typeof window === 'undefined') return; // Prevent SSR errors
5354

55+
this.state = {
56+
portalNode: null,
57+
};
58+
}
59+
60+
componentDidMount() {
5461
const { insert } = this.props;
5562

56-
this.portalNode = document.createElement('div');
57-
this.portalNode.dataset.euiportal = 'true';
63+
const portalNode = document.createElement('div');
64+
portalNode.dataset.euiportal = 'true';
5865

5966
if (insert == null) {
6067
// no insertion defined, append to body
61-
document.body.appendChild(this.portalNode);
68+
document.body.appendChild(portalNode);
6269
} else {
6370
// inserting before or after an element
6471
const { sibling, position } = insert;
65-
sibling.insertAdjacentElement(insertPositions[position], this.portalNode);
72+
sibling.insertAdjacentElement(insertPositions[position], portalNode);
6673
}
67-
}
6874

69-
componentDidMount() {
70-
this.setThemeColor();
71-
this.updatePortalRef(this.portalNode);
75+
this.setThemeColor(portalNode);
76+
this.updatePortalRef(portalNode);
77+
78+
// Update state with portalNode to intentionally trigger component rerender
79+
// and call createPortal with correct root element in render()
80+
this.setState({
81+
portalNode,
82+
});
7283
}
7384

7485
componentWillUnmount() {
75-
if (this.portalNode?.parentNode) {
76-
this.portalNode.parentNode.removeChild(this.portalNode);
86+
const { portalNode } = this.state;
87+
if (portalNode?.parentNode) {
88+
portalNode.parentNode.removeChild(portalNode);
7789
}
7890
this.updatePortalRef(null);
7991
}
8092

8193
// Set the inherited color of the portal based on the wrapping EuiThemeProvider
82-
setThemeColor() {
83-
if (this.portalNode && this.context) {
94+
private setThemeColor(portalNode: HTMLDivElement) {
95+
if (this.context) {
8496
const { hasDifferentColorFromGlobalTheme, colorClassName } = this.context;
8597

8698
if (hasDifferentColorFromGlobalTheme && this.props.insert == null) {
87-
this.portalNode.classList.add(colorClassName);
99+
portalNode.classList.add(colorClassName);
88100
}
89101
}
90102
}
91103

92-
updatePortalRef(ref: HTMLDivElement | null) {
104+
private updatePortalRef(ref: HTMLDivElement | null) {
93105
if (this.props.portalRef) {
94106
this.props.portalRef(ref);
95107
}
96108
}
97109

98110
render() {
99-
return this.portalNode ? (
100-
<>{createPortal(this.props.children, this.portalNode)}</>
101-
) : null;
111+
const { portalNode } = this.state;
112+
113+
if (!portalNode) {
114+
return null;
115+
}
116+
117+
return createPortal(this.props.children, portalNode);
102118
}
103119
}

src/components/tour/tour_step.tsx

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import React, {
1212
ReactElement,
1313
ReactNode,
1414
useEffect,
15-
useRef,
1615
useState,
1716
} from 'react';
1817
import classNames from 'classnames';
@@ -164,26 +163,29 @@ export const EuiTourStep: FunctionComponent<EuiTourStepProps> = ({
164163
);
165164
}
166165

167-
const [hasValidAnchor, setHasValidAnchor] = useState<boolean>(false);
168-
const animationFrameId = useRef<number>();
169-
const anchorNode = useRef<HTMLElement | null>(null);
166+
const [anchorNode, setAnchorNode] = useState<HTMLElement | null>(null);
170167
const [popoverPosition, setPopoverPosition] = useState<EuiPopoverPosition>();
171168

172169
const onPositionChange = (position: EuiPopoverPosition) => {
173170
setPopoverPosition(position);
174171
};
175172

176173
useEffect(() => {
174+
let timeout: number;
177175
if (anchor) {
178-
animationFrameId.current = window.requestAnimationFrame(() => {
179-
anchorNode.current = findElementBySelectorOrRef(anchor);
180-
setHasValidAnchor(anchorNode.current ? true : false);
176+
// Wait until next tick to find anchor node in case it's not already
177+
// in DOM requestAnimationFrame isn't used here because we don't need to
178+
// synchronize with repainting ticks and the updated value still
179+
// needs to go through a react DOM rerender which may take more than
180+
// 1 frame (16ms) of time.
181+
// TODO: It would be ideal to have some kind of intersection observer here instead
182+
timeout = window.setTimeout(() => {
183+
setAnchorNode(findElementBySelectorOrRef(anchor));
181184
});
182185
}
183186

184187
return () => {
185-
animationFrameId.current &&
186-
window.cancelAnimationFrame(animationFrameId.current);
188+
timeout && window.clearTimeout(timeout);
187189
};
188190
}, [anchor]);
189191

@@ -332,8 +334,8 @@ export const EuiTourStep: FunctionComponent<EuiTourStepProps> = ({
332334
);
333335
}
334336

335-
return hasValidAnchor && anchorNode.current ? (
336-
<EuiWrappingPopover button={anchorNode.current} {...popoverProps}>
337+
return anchorNode ? (
338+
<EuiWrappingPopover button={anchorNode} {...popoverProps}>
337339
{layout}
338340
</EuiWrappingPopover>
339341
) : null;

0 commit comments

Comments
 (0)