Skip to content

Commit 2af7ca4

Browse files
hoxyqhoxyq
committed
refactor[react-devtools]: rewrite context menus
1 parent 9d76c95 commit 2af7ca4

File tree

19 files changed

+824
-661
lines changed

19 files changed

+824
-661
lines changed

‎packages/react-devtools-inline/__tests__/__e2e__/components.test.js‎

Lines changed: 50 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ const devToolsUtils = require('./devtools-utils');
88
const {test, expect} = require('@playwright/test');
99
const config = require('../../playwright.config');
1010
const semver = require('semver');
11+
1112
test.use(config);
1213
test.describe('Components', () => {
1314
let page;
@@ -59,43 +60,54 @@ test.describe('Components', () => {
5960
const isEditableValue = semver.gte(config.use.react_version, '16.8.0');
6061

6162
// Then read the inspected values.
62-
const [propName, propValue] = await page.evaluate(
63-
isEditable => {
64-
const {createTestNameSelector, findAllNodes} =
65-
window.REACT_DOM_DEVTOOLS;
66-
const container = document.getElementById('devtools');
67-
68-
// Get name of first prop
69-
const selectorName = isEditable.name
70-
? 'EditableName'
71-
: 'NonEditableName';
72-
const nameElement = findAllNodes(container, [
73-
createTestNameSelector('InspectedElementPropsTree'),
74-
createTestNameSelector(selectorName),
75-
])[0];
76-
const name = isEditable.name
77-
? nameElement.value
78-
: nameElement.innerText;
79-
80-
// Get value of first prop
81-
const selectorValue = isEditable.value
82-
? 'EditableValue'
83-
: 'NonEditableValue';
84-
const valueElement = findAllNodes(container, [
85-
createTestNameSelector('InspectedElementPropsTree'),
86-
createTestNameSelector(selectorValue),
87-
])[0];
88-
const value = isEditable.value
89-
? valueElement.value
90-
: valueElement.innerText;
91-
92-
return [name, value];
93-
},
94-
{name: isEditableName, value: isEditableValue}
95-
);
96-
97-
expect(propName).toBe('label');
98-
expect(propValue).toBe('"one"');
63+
const {name, value, existingNameElementsSize, existingValueElementsSize} =
64+
await page.evaluate(
65+
isEditable => {
66+
const {createTestNameSelector, findAllNodes} =
67+
window.REACT_DOM_DEVTOOLS;
68+
const container = document.getElementById('devtools');
69+
70+
// Get name of first prop
71+
const nameSelector = isEditable.name
72+
? 'EditableName'
73+
: 'NonEditableName';
74+
// Get value of first prop
75+
const valueSelector = isEditable.value
76+
? 'EditableValue'
77+
: 'NonEditableValue';
78+
79+
const existingNameElements = findAllNodes(container, [
80+
createTestNameSelector('InspectedElementPropsTree'),
81+
createTestNameSelector('KeyValue'),
82+
createTestNameSelector(nameSelector),
83+
]);
84+
const existingValueElements = findAllNodes(container, [
85+
createTestNameSelector('InspectedElementPropsTree'),
86+
createTestNameSelector('KeyValue'),
87+
createTestNameSelector(valueSelector),
88+
]);
89+
90+
const name = isEditable.name
91+
? existingNameElements[0].value
92+
: existingNameElements[0].innerText;
93+
const value = isEditable.value
94+
? existingValueElements[0].value
95+
: existingValueElements[0].innerText;
96+
97+
return {
98+
name,
99+
value,
100+
existingNameElementsSize: existingNameElements.length,
101+
existingValueElementsSize: existingValueElements.length,
102+
};
103+
},
104+
{name: isEditableName, value: isEditableValue}
105+
);
106+
107+
expect(existingNameElementsSize).toBe(1);
108+
expect(existingValueElementsSize).toBe(1);
109+
expect(name).toBe('label');
110+
expect(value).toBe('"one"');
99111
});
100112

101113
test('Should allow inspecting source of the element', async () => {
@@ -135,6 +147,7 @@ test.describe('Components', () => {
135147

136148
focusWithin(container, [
137149
createTestNameSelector('InspectedElementPropsTree'),
150+
createTestNameSelector('KeyValue'),
138151
createTestNameSelector('EditableValue'),
139152
]);
140153
});

‎packages/react-devtools-shared/src/backend/utils.js‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,11 +140,15 @@ export function serializeToString(data: any): string {
140140
return 'undefined';
141141
}
142142

143+
if (typeof data === 'function') {
144+
return data.toString();
145+
}
146+
143147
const cache = new Set<mixed>();
144148
// Use a custom replacer function to protect against circular references.
145149
return JSON.stringify(
146150
data,
147-
(key, value) => {
151+
(key: string, value: any) => {
148152
if (typeof value === 'object' && value !== null) {
149153
if (cache.has(value)) {
150154
return;

‎packages/react-devtools-shared/src/devtools/ContextMenu/ContextMenu.css‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,4 @@
66
overflow: hidden;
77
z-index: 10000002;
88
user-select: none;
9-
}
9+
}

‎packages/react-devtools-shared/src/devtools/ContextMenu/ContextMenu.js‎

Lines changed: 80 additions & 111 deletions
Original file line numberDiff line numberDiff line change
@@ -8,141 +8,110 @@
88
*/
99

1010
import * as React from 'react';
11-
import {useContext, useEffect, useLayoutEffect, useRef, useState} from 'react';
11+
import {useLayoutEffect, createRef} from 'react';
1212
import {createPortal} from 'react-dom';
13-
import {RegistryContext} from './Contexts';
1413

15-
import styles from './ContextMenu.css';
14+
import ContextMenuItem from './ContextMenuItem';
15+
16+
import type {
17+
ContextMenuItem as ContextMenuItemType,
18+
ContextMenuPosition,
19+
ContextMenuRef,
20+
} from './types';
1621

17-
import type {RegistryContextType} from './Contexts';
22+
import styles from './ContextMenu.css';
1823

19-
function repositionToFit(element: HTMLElement, pageX: number, pageY: number) {
24+
function repositionToFit(element: HTMLElement, x: number, y: number) {
2025
const ownerWindow = element.ownerDocument.defaultView;
21-
if (element !== null) {
22-
if (pageY + element.offsetHeight >= ownerWindow.innerHeight) {
23-
if (pageY - element.offsetHeight > 0) {
24-
element.style.top = `${pageY - element.offsetHeight}px`;
25-
} else {
26-
element.style.top = '0px';
27-
}
26+
if (y + element.offsetHeight >= ownerWindow.innerHeight) {
27+
if (y - element.offsetHeight > 0) {
28+
element.style.top = `${y - element.offsetHeight}px`;
2829
} else {
29-
element.style.top = `${pageY}px`;
30+
element.style.top = '0px';
3031
}
32+
} else {
33+
element.style.top = `${y}px`;
34+
}
3135

32-
if (pageX + element.offsetWidth >= ownerWindow.innerWidth) {
33-
if (pageX - element.offsetWidth > 0) {
34-
element.style.left = `${pageX - element.offsetWidth}px`;
35-
} else {
36-
element.style.left = '0px';
37-
}
36+
if (x + element.offsetWidth >= ownerWindow.innerWidth) {
37+
if (x - element.offsetWidth > 0) {
38+
element.style.left = `${x - element.offsetWidth}px`;
3839
} else {
39-
element.style.left = `${pageX}px`;
40+
element.style.left = '0px';
4041
}
42+
} else {
43+
element.style.left = `${x}px`;
4144
}
4245
}
4346

44-
const HIDDEN_STATE = {
45-
data: null,
46-
isVisible: false,
47-
pageX: 0,
48-
pageY: 0,
49-
};
50-
5147
type Props = {
52-
children: (data: Object) => React$Node,
53-
id: string,
48+
anchorElementRef: {current: React.ElementRef<any> | null},
49+
items: ContextMenuItemType[],
50+
position: ContextMenuPosition,
51+
hide: () => void,
52+
ref?: ContextMenuRef,
5453
};
5554

56-
export default function ContextMenu({children, id}: Props): React.Node {
57-
const {hideMenu, registerMenu} =
58-
useContext<RegistryContextType>(RegistryContext);
59-
60-
const [state, setState] = useState(HIDDEN_STATE);
55+
export default function ContextMenu({
56+
anchorElementRef,
57+
position,
58+
items,
59+
hide,
60+
ref = createRef(),
61+
}: Props): React.Node {
62+
// This works on the assumption that ContextMenu component is only rendered when it should be shown
63+
const anchor = anchorElementRef.current;
64+
65+
if (anchor == null) {
66+
throw new Error(
67+
'Attempted to open a context menu for an element, which is not mounted',
68+
);
69+
}
6170

62-
const bodyAccessorRef = useRef(null);
63-
const containerRef = useRef(null);
64-
const menuRef = useRef(null);
71+
const ownerDocument = anchor.ownerDocument;
72+
const portalContainer = ownerDocument.querySelector(
73+
'[data-react-devtools-portal-root]',
74+
);
6575

66-
useEffect(() => {
67-
const element = bodyAccessorRef.current;
68-
if (element !== null) {
69-
const ownerDocument = element.ownerDocument;
70-
containerRef.current = ownerDocument.querySelector(
71-
'[data-react-devtools-portal-root]',
72-
);
76+
useLayoutEffect(() => {
77+
const menu = ((ref.current: any): HTMLElement);
7378

74-
if (containerRef.current == null) {
75-
console.warn(
76-
'DevTools tooltip root node not found; context menus will be disabled.',
77-
);
79+
function hideUnlessContains(event: Event) {
80+
if (!menu.contains(((event.target: any): Node))) {
81+
hide();
7882
}
7983
}
80-
}, []);
8184

82-
useEffect(() => {
83-
const showMenuFn = ({
84-
data,
85-
pageX,
86-
pageY,
87-
}: {
88-
data: any,
89-
pageX: number,
90-
pageY: number,
91-
}) => {
92-
setState({data, isVisible: true, pageX, pageY});
93-
};
94-
const hideMenuFn = () => setState(HIDDEN_STATE);
95-
return registerMenu(id, showMenuFn, hideMenuFn);
96-
}, [id]);
85+
ownerDocument.addEventListener('mousedown', hideUnlessContains);
86+
ownerDocument.addEventListener('touchstart', hideUnlessContains);
87+
ownerDocument.addEventListener('keydown', hideUnlessContains);
9788

98-
useLayoutEffect(() => {
99-
if (!state.isVisible) {
100-
return;
101-
}
89+
const ownerWindow = ownerDocument.defaultView;
90+
ownerWindow.addEventListener('resize', hide);
10291

103-
const menu = ((menuRef.current: any): HTMLElement);
104-
const container = containerRef.current;
105-
if (container !== null) {
106-
// $FlowFixMe[missing-local-annot]
107-
const hideUnlessContains = event => {
108-
if (!menu.contains(event.target)) {
109-
hideMenu();
110-
}
111-
};
112-
113-
const ownerDocument = container.ownerDocument;
114-
ownerDocument.addEventListener('mousedown', hideUnlessContains);
115-
ownerDocument.addEventListener('touchstart', hideUnlessContains);
116-
ownerDocument.addEventListener('keydown', hideUnlessContains);
117-
118-
const ownerWindow = ownerDocument.defaultView;
119-
ownerWindow.addEventListener('resize', hideMenu);
120-
121-
repositionToFit(menu, state.pageX, state.pageY);
122-
123-
return () => {
124-
ownerDocument.removeEventListener('mousedown', hideUnlessContains);
125-
ownerDocument.removeEventListener('touchstart', hideUnlessContains);
126-
ownerDocument.removeEventListener('keydown', hideUnlessContains);
127-
128-
ownerWindow.removeEventListener('resize', hideMenu);
129-
};
130-
}
131-
}, [state]);
92+
repositionToFit(menu, position.x, position.y);
13293

133-
if (!state.isVisible) {
134-
return <div ref={bodyAccessorRef} />;
135-
} else {
136-
const container = containerRef.current;
137-
if (container !== null) {
138-
return createPortal(
139-
<div ref={menuRef} className={styles.ContextMenu}>
140-
{children(state.data)}
141-
</div>,
142-
container,
143-
);
144-
} else {
145-
return null;
146-
}
94+
return () => {
95+
ownerDocument.removeEventListener('mousedown', hideUnlessContains);
96+
ownerDocument.removeEventListener('touchstart', hideUnlessContains);
97+
ownerDocument.removeEventListener('keydown', hideUnlessContains);
98+
99+
ownerWindow.removeEventListener('resize', hide);
100+
};
101+
}, []);
102+
103+
if (portalContainer == null || items.length === 0) {
104+
return null;
147105
}
106+
107+
return createPortal(
108+
<div className={styles.ContextMenu} ref={ref}>
109+
{items.map(({onClick, content}, index) => (
110+
<ContextMenuItem key={index} onClick={onClick} hide={hide}>
111+
{content}
112+
</ContextMenuItem>
113+
))}
114+
</div>,
115+
portalContainer,
116+
);
148117
}

0 commit comments

Comments
 (0)