Skip to content

Commit d1a7d76

Browse files
ryancogswelloliviertassinari
ryancogswell
authored and
oliviertassinari
committed
[MenuList] Convert to a function component (#14865)
* [MenuList] Convert to a function component * Provide access to MenuList internals via actions property * Changed Menu to interact with MenuList only via exposed actions * Adjusted Menu and MenuList tests accordingly * [MenuList] Fix MenuList test expected results to account for varying scrollbar size * scrollbar size is 0px with jsdom, but various sizes in Karma tests with different browsers * [MenuList] Address lint errors * Moved resetTabIndex out of the component so it doesn't need to be an effect dependency * Removed no-longer-used (due to moving functionality to MenuList) imports from Menu * [MenuList] Incorporate code review feedback * Deconstruct props in function body * Remove redundant findDOMNode calls * Document StrictMode status of findDOMNode calls * [MenuList] Fix Karma test for Safari (hopefully) * Unable to test this locally * [MenuList] Fix Karma test for Mac OS Chrome (hopefully) * Unable to test this locally * [MenuList] Use getScrollbarSize from utils instead of dom-helpers * [MenuList] Prettier * [MenuList] Fix Karma test for Chrome 41 (hopefully) * [MenuList] Fix Karma test for Chrome 41 * Stub clientHeight by stubbing the getter (stubbing the value does not work on Chrome 41) * [MenuList] Support ref using forwardRef * Change test to use ref instead of wrapper.getDOMNode() * [MenuList] Prettier test * [test] Remove wrapper.setProps calls for specifying onEnteringSpy * [MenuList] Make intent of condition clearer * [MenuList] Change useLayoutEffect to useEffect
1 parent a2a3771 commit d1a7d76

File tree

5 files changed

+286
-245
lines changed

5 files changed

+286
-245
lines changed

‎packages/material-ui/src/Menu/Menu.js‎

Lines changed: 11 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
22

33
import React from 'react';
44
import PropTypes from 'prop-types';
5-
import ReactDOM from 'react-dom';
6-
import getScrollbarSize from '../utils/getScrollbarSize';
75
import withStyles from '../styles/withStyles';
86
import withForwardedRef from '../utils/withForwardedRef';
97
import Popover from '../Popover';
@@ -32,51 +30,31 @@ export const styles = {
3230
};
3331

3432
class Menu extends React.Component {
33+
menuListActionsRef = React.createRef();
34+
3535
componentDidMount() {
3636
if (this.props.open && this.props.disableAutoFocusItem !== true) {
3737
this.focus();
3838
}
3939
}
4040

4141
getContentAnchorEl = () => {
42-
if (this.menuListRef.selectedItemRef) {
43-
return ReactDOM.findDOMNode(this.menuListRef.selectedItemRef);
44-
}
45-
46-
return ReactDOM.findDOMNode(this.menuListRef).firstChild;
42+
return this.menuListActionsRef.current.getContentAnchorEl();
4743
};
4844

4945
focus = () => {
50-
if (this.menuListRef && this.menuListRef.selectedItemRef) {
51-
ReactDOM.findDOMNode(this.menuListRef.selectedItemRef).focus();
52-
return;
53-
}
54-
55-
const menuList = ReactDOM.findDOMNode(this.menuListRef);
56-
if (menuList && menuList.firstChild) {
57-
menuList.firstChild.focus();
58-
}
59-
};
60-
61-
handleMenuListRef = ref => {
62-
this.menuListRef = ref;
46+
return this.menuListActionsRef.current && this.menuListActionsRef.current.focus();
6347
};
6448

6549
handleEntering = element => {
6650
const { disableAutoFocusItem, theme } = this.props;
67-
const menuList = ReactDOM.findDOMNode(this.menuListRef);
68-
69-
// Focus so the scroll computation of the Popover works as expected.
70-
if (disableAutoFocusItem !== true) {
71-
this.focus();
72-
}
7351

74-
// Let's ignore that piece of logic if users are already overriding the width
75-
// of the menu.
76-
if (menuList && element.clientHeight < menuList.clientHeight && !menuList.style.width) {
77-
const scrollbarSize = `${getScrollbarSize()}px`;
78-
menuList.style[theme.direction === 'rtl' ? 'paddingLeft' : 'paddingRight'] = scrollbarSize;
79-
menuList.style.width = `calc(100% + ${scrollbarSize})`;
52+
if (this.menuListActionsRef.current) {
53+
// Focus so the scroll computation of the Popover works as expected.
54+
if (disableAutoFocusItem !== true) {
55+
this.menuListActionsRef.current.focus();
56+
}
57+
this.menuListActionsRef.current.adjustStyleForScrollbar(element, theme);
8058
}
8159

8260
if (this.props.onEntering) {
@@ -129,7 +107,7 @@ class Menu extends React.Component {
129107
data-mui-test="Menu"
130108
onKeyDown={this.handleListKeyDown}
131109
{...MenuListProps}
132-
ref={this.handleMenuListRef}
110+
actions={this.menuListActionsRef}
133111
>
134112
{children}
135113
</MenuList>

‎packages/material-ui/src/Menu/Menu.test.js‎

Lines changed: 27 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
import React from 'react';
2-
import { spy, stub } from 'sinon';
2+
import { spy } from 'sinon';
33
import { assert } from 'chai';
4-
import ReactDOM from 'react-dom';
54
import { createShallow, createMount, getClasses } from '@material-ui/core/test-utils';
65
import Popover from '../Popover';
76
import Menu from './Menu';
87
import MenuList from '../MenuList';
98

9+
const MENU_LIST_HEIGHT = 100;
10+
1011
describe('<Menu />', () => {
1112
let shallow;
1213
let classes;
@@ -126,108 +127,35 @@ describe('<Menu />', () => {
126127
assert.strictEqual(document.activeElement, menuEl && menuEl.firstChild);
127128
});
128129

129-
describe('mount', () => {
130-
let wrapper;
131-
let instance;
132-
133-
let selectedItemFocusSpy;
134-
let menuListSpy;
135-
let menuListFocusSpy;
136-
137-
let elementForHandleEnter;
138-
139-
const SELECTED_ITEM_KEY = 111111;
140-
const MENU_LIST_HEIGHT = 100;
130+
it('should call props.onEntering with element if exists', () => {
131+
const onEnteringSpy = spy();
132+
const wrapper = mount(<Menu {...defaultProps} classes={classes} onEntering={onEnteringSpy} />);
133+
const instance = wrapper.find('Menu').instance();
141134

142-
let findDOMNodeStub;
143-
144-
before(() => {
145-
wrapper = mount(<Menu {...defaultProps} classes={classes} />);
146-
instance = wrapper.find('Menu').instance();
147-
148-
selectedItemFocusSpy = spy();
149-
menuListFocusSpy = spy();
150-
menuListSpy = {};
151-
menuListSpy.clientHeight = MENU_LIST_HEIGHT;
152-
menuListSpy.style = {};
153-
menuListSpy.firstChild = { focus: menuListFocusSpy };
154-
155-
findDOMNodeStub = stub(ReactDOM, 'findDOMNode').callsFake(arg => {
156-
if (arg === SELECTED_ITEM_KEY) {
157-
return { focus: selectedItemFocusSpy };
158-
}
159-
return menuListSpy;
160-
});
161-
162-
elementForHandleEnter = { clientHeight: MENU_LIST_HEIGHT };
163-
});
164-
165-
after(() => {
166-
findDOMNodeStub.restore();
167-
});
135+
const elementForHandleEnter = { clientHeight: MENU_LIST_HEIGHT };
168136

169-
beforeEach(() => {
170-
menuListFocusSpy.resetHistory();
171-
selectedItemFocusSpy.resetHistory();
172-
});
137+
instance.handleEntering(elementForHandleEnter);
138+
assert.strictEqual(onEnteringSpy.callCount, 1);
139+
assert.strictEqual(onEnteringSpy.calledWith(elementForHandleEnter), true);
140+
});
173141

174-
it('should call props.onEntering with element if exists', () => {
175-
const onEnteringSpy = spy();
176-
wrapper.setProps({ onEntering: onEnteringSpy });
177-
instance.handleEntering(elementForHandleEnter);
178-
assert.strictEqual(onEnteringSpy.callCount, 1);
179-
assert.strictEqual(onEnteringSpy.calledWith(elementForHandleEnter), true);
180-
});
142+
it('should call props.onEntering, disableAutoFocusItem', () => {
143+
const onEnteringSpy = spy();
144+
const wrapper = mount(
145+
<Menu disableAutoFocusItem {...defaultProps} classes={classes} onEntering={onEnteringSpy} />,
146+
);
147+
const instance = wrapper.find('Menu').instance();
181148

182-
it('should call menuList focus when no menuList', () => {
183-
delete instance.menuListRef;
184-
instance.handleEntering(elementForHandleEnter);
185-
assert.strictEqual(selectedItemFocusSpy.callCount, 0);
186-
assert.strictEqual(menuListFocusSpy.callCount, 1);
187-
});
149+
const elementForHandleEnter = { clientHeight: MENU_LIST_HEIGHT };
188150

189-
it('should call menuList focus when menuList but no menuList.selectedItemRef ', () => {
190-
instance.menuListRef = {};
191-
delete instance.menuListRef.selectedItemRef;
192-
instance.handleEntering(elementForHandleEnter);
193-
assert.strictEqual(selectedItemFocusSpy.callCount, 0);
194-
assert.strictEqual(menuListFocusSpy.callCount, 1);
195-
});
151+
instance.handleEntering(elementForHandleEnter);
152+
assert.strictEqual(onEnteringSpy.callCount, 1);
153+
assert.strictEqual(onEnteringSpy.calledWith(elementForHandleEnter), true);
154+
});
196155

197-
describe('menuList.selectedItemRef exists', () => {
198-
before(() => {
199-
instance.menuListRef = {};
200-
instance.menuListRef.selectedItemRef = SELECTED_ITEM_KEY;
201-
});
202-
203-
it('should call selectedItem focus when there is a menuList.selectedItemRef', () => {
204-
instance.handleEntering(elementForHandleEnter);
205-
assert.strictEqual(selectedItemFocusSpy.callCount, 1);
206-
assert.strictEqual(menuListFocusSpy.callCount, 0);
207-
});
208-
209-
it('should not set style on list when element.clientHeight > list.clientHeight', () => {
210-
elementForHandleEnter.clientHeight = MENU_LIST_HEIGHT + 1;
211-
instance.handleEntering(elementForHandleEnter);
212-
assert.strictEqual(menuListSpy.style.paddingRight, undefined);
213-
assert.strictEqual(menuListSpy.style.width, undefined);
214-
});
215-
216-
it('should not set style on list when element.clientHeight == list.clientHeight', () => {
217-
elementForHandleEnter.clientHeight = MENU_LIST_HEIGHT;
218-
instance.handleEntering(elementForHandleEnter);
219-
assert.strictEqual(menuListSpy.style.paddingRight, undefined);
220-
assert.strictEqual(menuListSpy.style.width, undefined);
221-
});
222-
223-
it('should not set style on list when element.clientHeight < list.clientHeight', () => {
224-
assert.strictEqual(menuListSpy.style.paddingRight, undefined);
225-
assert.strictEqual(menuListSpy.style.width, undefined);
226-
elementForHandleEnter.clientHeight = MENU_LIST_HEIGHT - 1;
227-
instance.handleEntering(elementForHandleEnter);
228-
assert.notStrictEqual(menuListSpy.style.paddingRight, undefined);
229-
assert.notStrictEqual(menuListSpy.style.width, undefined);
230-
});
231-
});
156+
it('call handleListKeyDown without onClose prop', () => {
157+
const wrapper = mount(<Menu {...defaultProps} />);
158+
const instance = wrapper.find('Menu').instance();
159+
instance.handleListKeyDown({ key: 'Tab', preventDefault: () => {} });
232160
});
233161
});

0 commit comments

Comments
 (0)