diff --git a/packages/material-ui/src/Popover/Popover.js b/packages/material-ui/src/Popover/Popover.js index 0df86142704746..700d2b7229316f 100644 --- a/packages/material-ui/src/Popover/Popover.js +++ b/packages/material-ui/src/Popover/Popover.js @@ -9,6 +9,7 @@ import EventListener from 'react-event-listener'; import ownerDocument from '../utils/ownerDocument'; import ownerWindow from '../utils/ownerWindow'; import { createChainedFunction } from '../utils/helpers'; +import withForwardedRef from '../utils/withForwardedRef'; import withStyles from '../styles/withStyles'; import Modal from '../Modal'; import Grow from '../Grow'; @@ -285,6 +286,7 @@ class Popover extends React.Component { container: containerProp, elevation, getContentAnchorEl, + innerRef, marginThreshold, ModalClasses, onEnter, @@ -295,7 +297,6 @@ class Popover extends React.Component { onExiting, open, PaperProps, - role, transformOrigin, TransitionComponent, transitionDuration: transitionDurationProp, @@ -320,6 +321,7 @@ class Popover extends React.Component { classes={ModalClasses} container={container} open={open} + ref={innerRef} BackdropProps={{ invisible: true }} {...other} > @@ -331,7 +333,6 @@ class Popover extends React.Component { onExit={onExit} onExited={onExited} onExiting={onExiting} - role={role} timeout={transitionDuration} {...TransitionProps} onEntering={createChainedFunction(this.handleEntering, TransitionProps.onEntering)} @@ -482,10 +483,6 @@ Popover.propTypes = { * Properties applied to the [`Paper`](/api/paper/) element. */ PaperProps: PropTypes.object, - /** - * @ignore - */ - role: PropTypes.string, /** * This is the point on the popover which * will attach to the anchor's origin. @@ -536,4 +533,4 @@ Popover.defaultProps = { transitionDuration: 'auto', }; -export default withStyles(styles, { name: 'MuiPopover' })(Popover); +export default withStyles(styles, { name: 'MuiPopover' })(withForwardedRef(Popover)); diff --git a/packages/material-ui/src/Popover/Popover.test.js b/packages/material-ui/src/Popover/Popover.test.js index 8255ecb4d9ca4f..88986150d8f0b1 100644 --- a/packages/material-ui/src/Popover/Popover.test.js +++ b/packages/material-ui/src/Popover/Popover.test.js @@ -2,8 +2,14 @@ import React from 'react'; import { assert } from 'chai'; import { spy, stub, useFakeTimers } from 'sinon'; import css from 'dom-helpers/style'; -import { createShallow, createMount, getClasses, unwrap } from '@material-ui/core/test-utils'; +import { + createShallow, + createMount, + findOutermostIntrinsic, + getClasses, +} from '@material-ui/core/test-utils'; import Grow from '../Grow'; +import Modal from '../Modal'; import Paper from '../Paper'; import Popover from './Popover'; @@ -14,7 +20,6 @@ describe('', () => { const defaultProps = { open: false, }; - const PopoverNaked = unwrap(Popover); before(() => { shallow = createShallow({ dive: true }); @@ -32,35 +37,27 @@ describe('', () => { describe('root node', () => { it('should render a Modal with an invisible backdrop as the root node', () => { - const wrapper = shallow( - -
- , - ); - assert.strictEqual(wrapper.props().BackdropProps.invisible, true); - }); - - it('should pass onClose prop to Modal', () => { - const fn = () => {}; - const wrapper = shallow( - + const wrapper = mount( +
, ); - assert.strictEqual(wrapper.props().onClose, fn, 'should be the onClose function'); + const root = wrapper.find('Popover > [data-root-node]').first(); + assert.strictEqual(root.type(), Modal); + assert.strictEqual(root.props().BackdropProps.invisible, true); }); it('should pass open prop to Modal as `open`', () => { - const wrapper = shallow( + const wrapper = mount(
, ); - assert.strictEqual(wrapper.props().open, false); + assert.strictEqual(wrapper.find(Modal).props().open, false); wrapper.setProps({ open: true }); - assert.strictEqual(wrapper.props().open, true); + assert.strictEqual(wrapper.find(Modal).props().open, true); wrapper.setProps({ open: false }); - assert.strictEqual(wrapper.props().open, false); + assert.strictEqual(wrapper.find(Modal).props().open, false); }); describe('getOffsetTop', () => { @@ -68,11 +65,13 @@ describe('', () => { let rect; before(() => { - instance = shallow( + instance = mount(
, - ).instance(); + ) + .find('Popover') + .instance(); rect = { height: 1 }; }); @@ -106,11 +105,13 @@ describe('', () => { let rect; before(() => { - instance = shallow( + instance = mount(
, - ).instance(); + ) + .find('Popover') + .instance(); rect = { width: 1 }; }); @@ -141,32 +142,40 @@ describe('', () => { }); describe('transition', () => { - it('should have Transition as the only child of Modal', () => { - const wrapper = shallow( - + let clock; + + beforeEach(() => { + clock = useFakeTimers(); + }); + + afterEach(() => { + clock.restore(); + }); + + it('uses Grow as the Transition of the modal', () => { + const wrapper = mount( +
, ); - assert.strictEqual(wrapper.children().length, 1, 'should have one child'); - assert.strictEqual(wrapper.childAt(0).type(), Grow); - assert.strictEqual( - wrapper.childAt(0).props().appear, - true, - 'should transition on first appearance', - ); + const modal = wrapper.find('[data-mui-test="Modal"]'); + const transition = modal.find(Grow); + + assert.strictEqual(transition.exists(), true); + assert.strictEqual(transition.props().appear, true, 'should transition on first appearance'); }); it('should set the transition in/out based on the open prop', () => { - const wrapper = shallow( - + const wrapper = mount( +
, ); - assert.strictEqual(wrapper.childAt(0).props().in, false); + assert.strictEqual(wrapper.find(Grow).props().in, false); wrapper.setProps({ open: true }); - assert.strictEqual(wrapper.childAt(0).props().in, true); + assert.strictEqual(wrapper.find(Grow).props().in, true); wrapper.setProps({ open: false }); - assert.strictEqual(wrapper.childAt(0).props().in, false); + assert.strictEqual(wrapper.find(Grow).props().in, false); }); it('should fire Popover transition event callbacks', () => { @@ -177,15 +186,18 @@ describe('', () => { return result; }, {}); - const wrapper = shallow( - + // transitions towards entered + const wrapper = mount( +
, ); + clock.tick(0); + // transition towards exited + wrapper.setProps({ open: false }); + clock.tick(0); events.forEach(eventHook => { - const event = eventHook.charAt(2).toLowerCase() + eventHook.slice(3); - wrapper.find(Grow).simulate(event, { style: {} }); assert.strictEqual( handlers[eventHook].callCount, 1, @@ -196,56 +208,48 @@ describe('', () => { }); describe('paper', () => { - it('should have Paper as the only child of Transition', () => { - const wrapper = shallow( - + it('should have Paper as a child of Transition', () => { + const wrapper = mount( +
, ); - assert.strictEqual(wrapper.childAt(0).children().length, 1, 'should have one child'); + assert.strictEqual( wrapper - .childAt(0) - .childAt(0) - .type(), - Paper, + .find(Grow) + .find(Paper) + .exists(), + true, ); }); it('should have the paper class and user classes', () => { - const wrapper = shallow( - + const wrapper = mount( +
, ); - assert.strictEqual(wrapper.hasClass('test-class'), true); - const paper = wrapper.childAt(0).childAt(0); + assert.strictEqual(findOutermostIntrinsic(wrapper).hasClass('test-class'), true); + const paper = wrapper.find(Paper); assert.strictEqual(paper.hasClass(classes.paper), true); }); it('should have a elevation prop passed down', () => { - const wrapper = shallow( - + const wrapper = mount( +
, ); + assert.strictEqual( - wrapper - .childAt(0) - .childAt(0) - .props().elevation, + wrapper.find(Paper).props().elevation, 8, 'should be 8 elevation by default', ); + wrapper.setProps({ elevation: 16 }); - assert.strictEqual( - wrapper - .childAt(0) - .childAt(0) - .props().elevation, - 16, - 'should be 16 elevation', - ); + assert.strictEqual(wrapper.find(Paper).props().elevation, 16, 'should be 16 elevation'); }); }); @@ -262,20 +266,16 @@ describe('', () => { }; describe('handleEntering(element)', () => { - let wrapper; - let handleEntering; - - before(() => { - handleEntering = spy(); - wrapper = shallow( + it('should set the inline styles for the enter phase', () => { + const handleEntering = spy(); + const wrapper = mount(
, ); - wrapper.instance().handleEntering(element); - }); + const instance = wrapper.find('Popover').instance(); + instance.handleEntering(element); - it('should set the inline styles for the enter phase', () => { assert.strictEqual( element.style.top === '16px' && element.style.left === '16px', true, @@ -283,7 +283,7 @@ describe('', () => { ); assert.strictEqual( element.style.transformOrigin, - wrapper.instance().getPositioningStyle(element).transformOrigin, + instance.getPositioningStyle(element).transformOrigin, 'should have a transformOrigin', ); }); @@ -293,7 +293,7 @@ describe('', () => { describe('prop: anchorEl', () => { it('should accept a function', () => { const anchorElSpy = spy(); - shallow( + mount(
, @@ -365,7 +365,9 @@ describe('', () => { }); after(() => { - window.document.body.removeChild(anchorEl); + if (anchorEl) { + window.document.body.removeChild(anchorEl); + } }); it('should be positioned over the top left of the anchor', async () => { @@ -421,14 +423,14 @@ describe('', () => { it('should pass through container prop if container and anchorEl props are provided', () => { const container = {}; - const shallowWrapper = shallow(); + const shallowWrapper = shallow(); + assert.strictEqual( shallowWrapper .dive() - .find('Modal') + .find(Modal) .props().container, container, - 'should pass through container prop if both container and anchorEl props are provided', ); }); @@ -437,7 +439,7 @@ describe('', () => { assert.strictEqual( wrapper .dive() - .find('Modal') + .find(Modal) .props().container, window.document.body, "should use anchorEl's parent body as Modal container", @@ -446,15 +448,8 @@ describe('', () => { }); it('should not pass container to Modal if container or anchorEl props are not provided', () => { - const shallowWrapper = shallow(); - assert.strictEqual( - shallowWrapper - .dive() - .find('Modal') - .props().container, - undefined, - 'should not pass a container prop if neither container or anchorEl are provided', - ); + const otherWrapper = mount(); + assert.strictEqual(otherWrapper.find('Modal').props().container, undefined); }); }); @@ -581,34 +576,33 @@ describe('', () => { }); it('should recalculate position if the popover is open', () => { - const wrapper = shallow( + const wrapper = mount(
, ); - const instance = wrapper.instance(); + const instance = wrapper.find('Popover').instance(); stub(instance, 'setPositioningStyles'); - wrapper - .find('EventListener') - .at(0) - .simulate('resize'); + window.dispatchEvent(new window.Event('resize')); clock.tick(166); assert.strictEqual(instance.setPositioningStyles.called, true); }); it('should not recalculate position if the popover is closed', () => { const wrapper = mount( - +
- , + , ); - const instance = wrapper.instance(); - assert.strictEqual(wrapper.contains('EventListener'), false); - stub(instance, 'setPositioningStyles'); - wrapper.instance().handleResize(); + + const setPositioningStyles = stub(wrapper.find('Popover').instance(), 'setPositioningStyles'); + + window.dispatchEvent(new window.Event('resize')); + wrapper.setProps({ open: false }); clock.tick(166); - assert.strictEqual(instance.setPositioningStyles.called, false); + + assert.strictEqual(setPositioningStyles.called, false); }); }); @@ -625,11 +619,13 @@ describe('', () => { let innerWidthContainer; before(() => { - instance = shallow( + instance = mount(
, - ).instance(); + ) + .find('Popover') + .instance(); instance.getContentAnchorOffset = spy(); innerHeightContainer = global.window.innerHeight; @@ -659,10 +655,6 @@ describe('', () => { positioningStyle = instance.getPositioningStyle(element); }); - after(() => { - instance.getAnchorOffset = stub().returns(anchorOffset); - }); - it('should set top to marginThreshold', () => { assert.strictEqual(positioningStyle.top, `${marginThreshold}px`); }); @@ -781,13 +773,19 @@ describe('', () => { describe('prop: getContentAnchorEl', () => { it('should position accordingly', () => { const element = { scrollTop: 5, contains: () => true }; - const child = { offsetTop: 40, clientHeight: 20, parentNode: element }; - const wrapper = shallow( - child}> + const getContentAnchorEl = stub().returns({ + offsetTop: 40, + clientHeight: 20, + parentNode: element, + }); + const wrapper = mount( +
, ); - assert.strictEqual(wrapper.instance().getContentAnchorOffset(element), 45); + + const instance = wrapper.find('Popover').instance(); + assert.strictEqual(instance.getContentAnchorOffset(element), 45); }); }); @@ -818,8 +816,8 @@ describe('', () => { describe('prop: transitionDuration', () => { it('should apply the auto property if supported', () => { - const wrapper = shallow( - + const wrapper = mount( +
, ); @@ -827,9 +825,9 @@ describe('', () => { }); it('should not apply the auto property if not supported', () => { - const TransitionComponent = props =>
; - const wrapper = shallow( - + const TransitionComponent = () =>
; + const wrapper = mount( +
, ); @@ -838,18 +836,43 @@ describe('', () => { }); describe('prop: TransitionProp', () => { - it('should fire Popover transition event callbacks', () => { - const handler1 = spy(); - const handler2 = spy(); - const wrapper = shallow( - + it('chains onEntering with the apparent onEntering prop', () => { + const apparentHandler = spy(); + const transitionHandler = spy(); + + mount( +
, ); - wrapper.find(Grow).simulate('entering', { style: {} }); - assert.strictEqual(handler1.callCount, 1); - assert.strictEqual(handler2.callCount, 1); + assert.strictEqual(apparentHandler.callCount, 1); + assert.strictEqual(transitionHandler.callCount, 1); + }); + + it('does not chain other transition callbacks with the apparent ones', () => { + const apparentHandler = spy(); + const transitionHandler = spy(); + const wrapper = mount( + +
+ , + ); + + wrapper.setProps({ open: false }); + + assert.strictEqual(apparentHandler.callCount, 0); + assert.strictEqual(transitionHandler.callCount, 1); }); }); });