Skip to content

Commit

Permalink
[MenuList] Convert to a function component (#14865)
Browse files Browse the repository at this point in the history 
* [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
  • Loading branch information
@ryancogswell @oliviertassinari
ryancogswell authored and oliviertassinari committed Mar 21, 2019
1 parent a2a3771 commit d1a7d76
Show file tree
Hide file tree
Showing 5 changed files with 286 additions and 245 deletions.
44 changes: 11 additions & 33 deletions packages/material-ui/src/Menu/Menu.js
View file
Expand Up @@ -2,8 +2,6 @@

import React from 'react';
import PropTypes from 'prop-types';
import ReactDOM from 'react-dom';
import getScrollbarSize from '../utils/getScrollbarSize';
import withStyles from '../styles/withStyles';
import withForwardedRef from '../utils/withForwardedRef';
import Popover from '../Popover';
Expand Down Expand Up @@ -32,51 +30,31 @@ export const styles = {
};

class Menu extends React.Component {
menuListActionsRef = React.createRef();

componentDidMount() {
if (this.props.open && this.props.disableAutoFocusItem !== true) {
this.focus();
}
}

getContentAnchorEl = () => {
if (this.menuListRef.selectedItemRef) {
return ReactDOM.findDOMNode(this.menuListRef.selectedItemRef);
}

return ReactDOM.findDOMNode(this.menuListRef).firstChild;
return this.menuListActionsRef.current.getContentAnchorEl();
};

focus = () => {
if (this.menuListRef && this.menuListRef.selectedItemRef) {
ReactDOM.findDOMNode(this.menuListRef.selectedItemRef).focus();
return;
}

const menuList = ReactDOM.findDOMNode(this.menuListRef);
if (menuList && menuList.firstChild) {
menuList.firstChild.focus();
}
};

handleMenuListRef = ref => {
this.menuListRef = ref;
return this.menuListActionsRef.current && this.menuListActionsRef.current.focus();
};

handleEntering = element => {
const { disableAutoFocusItem, theme } = this.props;
const menuList = ReactDOM.findDOMNode(this.menuListRef);

// Focus so the scroll computation of the Popover works as expected.
if (disableAutoFocusItem !== true) {
this.focus();
}

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

if (this.props.onEntering) {
Expand Down Expand Up @@ -129,7 +107,7 @@ class Menu extends React.Component {
data-mui-test="Menu"
onKeyDown={this.handleListKeyDown}
{...MenuListProps}
ref={this.handleMenuListRef}
actions={this.menuListActionsRef}
>
{children}
</MenuList>
Expand Down
126 changes: 27 additions & 99 deletions packages/material-ui/src/Menu/Menu.test.js
View file
@@ -1,12 +1,13 @@
import React from 'react';
import { spy, stub } from 'sinon';
import { spy } from 'sinon';
import { assert } from 'chai';
import ReactDOM from 'react-dom';
import { createShallow, createMount, getClasses } from '@material-ui/core/test-utils';
import Popover from '../Popover';
import Menu from './Menu';
import MenuList from '../MenuList';

const MENU_LIST_HEIGHT = 100;

describe('<Menu />', () => {
let shallow;
let classes;
Expand Down Expand Up @@ -126,108 +127,35 @@ describe('<Menu />', () => {
assert.strictEqual(document.activeElement, menuEl && menuEl.firstChild);
});

describe('mount', () => {
let wrapper;
let instance;

let selectedItemFocusSpy;
let menuListSpy;
let menuListFocusSpy;

let elementForHandleEnter;

const SELECTED_ITEM_KEY = 111111;
const MENU_LIST_HEIGHT = 100;
it('should call props.onEntering with element if exists', () => {
const onEnteringSpy = spy();
const wrapper = mount(<Menu {...defaultProps} classes={classes} onEntering={onEnteringSpy} />);
const instance = wrapper.find('Menu').instance();

let findDOMNodeStub;

before(() => {
wrapper = mount(<Menu {...defaultProps} classes={classes} />);
instance = wrapper.find('Menu').instance();

selectedItemFocusSpy = spy();
menuListFocusSpy = spy();
menuListSpy = {};
menuListSpy.clientHeight = MENU_LIST_HEIGHT;
menuListSpy.style = {};
menuListSpy.firstChild = { focus: menuListFocusSpy };

findDOMNodeStub = stub(ReactDOM, 'findDOMNode').callsFake(arg => {
if (arg === SELECTED_ITEM_KEY) {
return { focus: selectedItemFocusSpy };
}
return menuListSpy;
});

elementForHandleEnter = { clientHeight: MENU_LIST_HEIGHT };
});

after(() => {
findDOMNodeStub.restore();
});
const elementForHandleEnter = { clientHeight: MENU_LIST_HEIGHT };

beforeEach(() => {
menuListFocusSpy.resetHistory();
selectedItemFocusSpy.resetHistory();
});
instance.handleEntering(elementForHandleEnter);
assert.strictEqual(onEnteringSpy.callCount, 1);
assert.strictEqual(onEnteringSpy.calledWith(elementForHandleEnter), true);
});

it('should call props.onEntering with element if exists', () => {
const onEnteringSpy = spy();
wrapper.setProps({ onEntering: onEnteringSpy });
instance.handleEntering(elementForHandleEnter);
assert.strictEqual(onEnteringSpy.callCount, 1);
assert.strictEqual(onEnteringSpy.calledWith(elementForHandleEnter), true);
});
it('should call props.onEntering, disableAutoFocusItem', () => {
const onEnteringSpy = spy();
const wrapper = mount(
<Menu disableAutoFocusItem {...defaultProps} classes={classes} onEntering={onEnteringSpy} />,
);
const instance = wrapper.find('Menu').instance();

it('should call menuList focus when no menuList', () => {
delete instance.menuListRef;
instance.handleEntering(elementForHandleEnter);
assert.strictEqual(selectedItemFocusSpy.callCount, 0);
assert.strictEqual(menuListFocusSpy.callCount, 1);
});
const elementForHandleEnter = { clientHeight: MENU_LIST_HEIGHT };

it('should call menuList focus when menuList but no menuList.selectedItemRef ', () => {
instance.menuListRef = {};
delete instance.menuListRef.selectedItemRef;
instance.handleEntering(elementForHandleEnter);
assert.strictEqual(selectedItemFocusSpy.callCount, 0);
assert.strictEqual(menuListFocusSpy.callCount, 1);
});
instance.handleEntering(elementForHandleEnter);
assert.strictEqual(onEnteringSpy.callCount, 1);
assert.strictEqual(onEnteringSpy.calledWith(elementForHandleEnter), true);
});

describe('menuList.selectedItemRef exists', () => {
before(() => {
instance.menuListRef = {};
instance.menuListRef.selectedItemRef = SELECTED_ITEM_KEY;
});

it('should call selectedItem focus when there is a menuList.selectedItemRef', () => {
instance.handleEntering(elementForHandleEnter);
assert.strictEqual(selectedItemFocusSpy.callCount, 1);
assert.strictEqual(menuListFocusSpy.callCount, 0);
});

it('should not set style on list when element.clientHeight > list.clientHeight', () => {
elementForHandleEnter.clientHeight = MENU_LIST_HEIGHT + 1;
instance.handleEntering(elementForHandleEnter);
assert.strictEqual(menuListSpy.style.paddingRight, undefined);
assert.strictEqual(menuListSpy.style.width, undefined);
});

it('should not set style on list when element.clientHeight == list.clientHeight', () => {
elementForHandleEnter.clientHeight = MENU_LIST_HEIGHT;
instance.handleEntering(elementForHandleEnter);
assert.strictEqual(menuListSpy.style.paddingRight, undefined);
assert.strictEqual(menuListSpy.style.width, undefined);
});

it('should not set style on list when element.clientHeight < list.clientHeight', () => {
assert.strictEqual(menuListSpy.style.paddingRight, undefined);
assert.strictEqual(menuListSpy.style.width, undefined);
elementForHandleEnter.clientHeight = MENU_LIST_HEIGHT - 1;
instance.handleEntering(elementForHandleEnter);
assert.notStrictEqual(menuListSpy.style.paddingRight, undefined);
assert.notStrictEqual(menuListSpy.style.width, undefined);
});
});
it('call handleListKeyDown without onClose prop', () => {
const wrapper = mount(<Menu {...defaultProps} />);
const instance = wrapper.find('Menu').instance();
instance.handleListKeyDown({ key: 'Tab', preventDefault: () => {} });
});
});

0 comments on commit d1a7d76

Please sign in to comment.