From 8b1d9875a2cd6209ef41683fe2c92f79ae91a641 Mon Sep 17 00:00:00 2001 From: guifu Date: Mon, 16 Apr 2018 20:17:31 +0800 Subject: [PATCH] chore: improve coverage and one minor bug fix --- src/DOMWrap.jsx | 2 +- src/Menu.jsx | 20 +---- src/MenuItem.jsx | 2 +- src/MenuItemGroup.jsx | 4 +- src/MenuMixin.js | 42 ++-------- src/SubMenu.jsx | 36 ++++++--- src/SubPopupMenu.js | 18 +++-- src/util.js | 15 ++-- tests/Menu.spec.js | 3 +- tests/MenuItem.spec.js | 50 +++++++++++- tests/SubMenu.spec.js | 112 ++++++++++++++++++++++++++ tests/__snapshots__/Menu.spec.js.snap | 14 +++- 12 files changed, 228 insertions(+), 90 deletions(-) diff --git a/src/DOMWrap.jsx b/src/DOMWrap.jsx index 63231def..e4efc189 100644 --- a/src/DOMWrap.jsx +++ b/src/DOMWrap.jsx @@ -14,13 +14,13 @@ const DOMWrap = createReactClass({ getDefaultProps() { return { tag: 'div', + className: '', }; }, render() { const props = { ...this.props }; if (!props.visible) { - props.className = props.className || ''; props.className += ` ${props.hiddenClassName}`; } const Tag = props.tag; diff --git a/src/Menu.jsx b/src/Menu.jsx index d9ff7750..c8874da8 100644 --- a/src/Menu.jsx +++ b/src/Menu.jsx @@ -175,22 +175,8 @@ const Menu = createReactClass({ return transitionName; }, - isInlineMode() { - return this.props.mode === 'inline'; - }, - - lastOpenSubMenu() { - let lastOpen = []; - const { openKeys } = this.store.getState(); - if (openKeys.length) { - lastOpen = this.getFlatInstanceArray().filter((c) => { - return c && openKeys.indexOf(c.props.eventKey) !== -1; - }); - } - return lastOpen[0]; - }, - - renderMenuItem(c, i, subIndex, subMenuKey) { + renderMenuItem(c, i, subMenuKey) { + /* istanbul ignore if */ if (!c) { return null; } @@ -201,7 +187,7 @@ const Menu = createReactClass({ triggerSubMenuAction: this.props.triggerSubMenuAction, subMenuKey, }; - return this.renderCommonMenuItem(c, i, subIndex, extraProps); + return this.renderCommonMenuItem(c, i, extraProps); }, render() { diff --git a/src/MenuItem.jsx b/src/MenuItem.jsx index 77c89b79..48f06b61 100644 --- a/src/MenuItem.jsx +++ b/src/MenuItem.jsx @@ -10,7 +10,7 @@ import { noop } from './util'; /* eslint react/no-is-mounted:0 */ -const MenuItem = createReactClass({ +export const MenuItem = createReactClass({ displayName: 'MenuItem', propTypes: { diff --git a/src/MenuItemGroup.jsx b/src/MenuItemGroup.jsx index c6f189c9..bb52fe2a 100644 --- a/src/MenuItemGroup.jsx +++ b/src/MenuItemGroup.jsx @@ -17,9 +17,9 @@ const MenuItemGroup = createReactClass({ return { disabled: true }; }, - renderInnerMenuItem(item, subIndex) { + renderInnerMenuItem(item) { const { renderMenuItem, index } = this.props; - return renderMenuItem(item, index, subIndex, this.props.subMenuKey); + return renderMenuItem(item, index, this.props.subMenuKey); }, render() { diff --git a/src/MenuMixin.js b/src/MenuMixin.js index 9efaec49..98324fae 100644 --- a/src/MenuMixin.js +++ b/src/MenuMixin.js @@ -49,14 +49,9 @@ export function getActiveKey(props, originalActiveKey) { return activeKey; } -function saveRef(index, subIndex, c) { +function saveRef(index, c) { if (c) { - if (subIndex !== undefined) { - this.instanceArray[index] = this.instanceArray[index] || []; - this.instanceArray[index][subIndex] = c; - } else { - this.instanceArray[index] = c; - } + this.instanceArray[index] = c; } } @@ -106,7 +101,7 @@ const MenuMixin = { }, // all keyboard events callbacks run from here at first - onKeyDown(e, callback) { + onKeyDown(e) { const keyCode = e.keyCode; let handled; this.getFlatInstanceArray().forEach((obj) => { @@ -125,14 +120,6 @@ const MenuMixin = { e.preventDefault(); updateActiveKey(this.getStore(), this.getEventKey(), activeItem.props.eventKey); - if (typeof callback === 'function') { - callback(activeItem); - } - - return 1; - } else if (activeItem === undefined) { - e.preventDefault(); - updateActiveKey(this.getStore(), this.getEventKey(), null); return 1; } }, @@ -154,25 +141,10 @@ const MenuMixin = { }, getFlatInstanceArray() { - let instanceArray = this.instanceArray; - const hasInnerArray = instanceArray.some((a) => { - return Array.isArray(a); - }); - if (hasInnerArray) { - instanceArray = []; - this.instanceArray.forEach((a) => { - if (Array.isArray(a)) { - instanceArray.push.apply(instanceArray, a); - } else { - instanceArray.push(a); - } - }); - this.instanceArray = instanceArray; - } - return instanceArray; + return this.instanceArray; }, - renderCommonMenuItem(child, i, subIndex, extraProps) { + renderCommonMenuItem(child, i, extraProps) { const state = this.getStore().getState(); const props = this.props; const key = getKeyFromChildrenIndex(child, props.eventKey, i); @@ -188,7 +160,7 @@ const MenuMixin = { parentMenu: this, // customized ref function, need to be invoked manually in child's componentDidMount manualRef: childProps.disabled ? undefined : - createChainedFunction(child.ref, saveRef.bind(this, i, subIndex)), + createChainedFunction(child.ref, saveRef.bind(this, i)), eventKey: key, active: !childProps.disabled && isActive, multiple: props.multiple, @@ -241,7 +213,7 @@ const MenuMixin = { > {React.Children.map( props.children, - (c, i, subIndex) => this.renderMenuItem(c, i, subIndex, props.eventKey || '0-menu-'), + (c, i) => this.renderMenuItem(c, i, props.eventKey || '0-menu-'), )} /*eslint-enable */ diff --git a/src/SubMenu.jsx b/src/SubMenu.jsx index 99aa4279..75cc9faf 100644 --- a/src/SubMenu.jsx +++ b/src/SubMenu.jsx @@ -10,7 +10,7 @@ import SubPopupMenu from './SubPopupMenu'; import placements from './placements'; import { noop, - loopMenuItemRecusively, + loopMenuItemRecursively, getMenuIdFromSubMenuEventKey, } from './util'; @@ -96,6 +96,20 @@ const SubMenu = createReactClass({ this.componentDidUpdate(); }, + adjustWidth() { + /* istanbul ignore if */ + if (!this.subMenuTitle || !this.menuInstance) { + return; + } + const popupMenu = ReactDOM.findDOMNode(this.menuInstance); + if (popupMenu.offsetWidth >= this.subMenuTitle.offsetWidth) { + return; + } + + /* istanbul ignore next */ + popupMenu.style.minWidth = `${this.subMenuTitle.offsetWidth}px`; + }, + componentDidUpdate() { const { mode, parentMenu, manualRef } = this.props; @@ -108,16 +122,7 @@ const SubMenu = createReactClass({ return; } - this.minWidthTimeout = setTimeout(() => { - if (!this.subMenuTitle || !this.menuInstance) { - return; - } - const popupMenu = ReactDOM.findDOMNode(this.menuInstance); - if (popupMenu.offsetWidth >= this.subMenuTitle.offsetWidth) { - return; - } - popupMenu.style.minWidth = `${this.subMenuTitle.offsetWidth}px`; - }, 0); + this.minWidthTimeout = setTimeout(() => this.adjustWidth(), 0); }, componentWillUnmount() { @@ -125,9 +130,13 @@ const SubMenu = createReactClass({ if (onDestroy) { onDestroy(eventKey); } + + /* istanbul ignore if */ if (this.minWidthTimeout) { clearTimeout(this.minWidthTimeout); } + + /* istanbul ignore if */ if (this.mouseenterTimeout) { clearTimeout(this.mouseenterTimeout); } @@ -314,7 +323,7 @@ const SubMenu = createReactClass({ isChildrenSelected() { const ret = { find: false }; - loopMenuItemRecusively(this.props.children, this.props.selectedKeys, ret); + loopMenuItemRecursively(this.props.children, this.props.selectedKeys, ret); return ret.find; }, @@ -454,7 +463,8 @@ const SubMenu = createReactClass({ SubMenu.isSubMenu = 1; -export default connect(({ openKeys, activeKey }, { eventKey, subMenuKey }) => ({ +export default connect(({ openKeys, activeKey, selectedKeys }, { eventKey, subMenuKey }) => ({ isOpen: openKeys.indexOf(eventKey) > -1, active: activeKey[subMenuKey] === eventKey, + selectedKeys, }))(SubMenu); diff --git a/src/SubPopupMenu.js b/src/SubPopupMenu.js index 76655b0e..92974136 100644 --- a/src/SubPopupMenu.js +++ b/src/SubPopupMenu.js @@ -37,9 +37,7 @@ const SubPopupMenu = createReactClass({ componentDidMount() { // invoke customized ref to expose component to mixin - if (this.props.manualRef) { - this.props.manualRef(this); - } + this.props.manualRef(this); }, onDeselect(selectInfo) { @@ -59,6 +57,7 @@ const SubPopupMenu = createReactClass({ }, onDestroy(key) { + /* istanbul ignore next */ this.props.onDestroy(key); }, @@ -66,7 +65,8 @@ const SubPopupMenu = createReactClass({ return this.props.openTransitionName; }, - renderMenuItem(c, i, subIndex, subMenuKey) { + renderMenuItem(c, i, subMenuKey) { + /* istanbul ignore next */ if (!c) { return null; } @@ -77,24 +77,28 @@ const SubPopupMenu = createReactClass({ triggerSubMenuAction: props.triggerSubMenuAction, subMenuKey, }; - return this.renderCommonMenuItem(c, i, subIndex, extraProps); + return this.renderCommonMenuItem(c, i, extraProps); }, render() { const props = { ...this.props }; - const haveRendered = this.haveRendered; this.haveRendered = true; this.haveOpened = this.haveOpened || props.visible || props.forceSubMenuRender; + // never rendered not planning to, don't render if (!this.haveOpened) { return null; } - const transitionAppear = !(!haveRendered && props.visible && props.mode === 'inline'); + // don't show transition on first rendering (no animation for opened menu) + // show appear transition if it's not visible (not sure why) + // show appear transition if it's not inline mode + const transitionAppear = haveRendered || !props.visible || !props.mode === 'inline'; props.className += ` ${props.prefixCls}-sub`; const animProps = {}; + if (props.openTransitionName) { animProps.transitionName = props.openTransitionName; } else if (typeof props.openAnimation === 'object') { diff --git a/src/util.js b/src/util.js index fbb8739e..82c07f4f 100644 --- a/src/util.js +++ b/src/util.js @@ -27,23 +27,24 @@ export function loopMenuItem(children, cb) { }); } -export function loopMenuItemRecusively(children, keys, ret) { +export function loopMenuItemRecursively(children, keys, ret) { + /* istanbul ignore if */ if (!children || ret.find) { return; } React.Children.forEach(children, (c) => { - if (ret.find) { - return; - } if (c) { - const construt = c.type; - if (!construt || !(construt.isSubMenu || construt.isMenuItem || construt.isMenuItemGroup)) { + const construct = c.type; + if (!construct + || + !(construct.isSubMenu || construct.isMenuItem || construct.isMenuItemGroup) + ) { return; } if (keys.indexOf(c.key) !== -1) { ret.find = true; } else if (c.props.children) { - loopMenuItemRecusively(c.props.children, keys, ret); + loopMenuItemRecursively(c.props.children, keys, ret); } } }); diff --git a/tests/Menu.spec.js b/tests/Menu.spec.js index 16030040..764282cb 100644 --- a/tests/Menu.spec.js +++ b/tests/Menu.spec.js @@ -33,13 +33,12 @@ describe('Menu', () => { ['vertical', 'horizontal', 'inline'].forEach((mode) => { it(`renders ${mode} menu correctly`, () => { - const wrapper = render(createMenu({ [mode]: true })); + const wrapper = render(createMenu({ mode })); expect(renderToJson(wrapper)).toMatchSnapshot(); }); }); }); - it('set activeKey', () => { const wrapper = mount( diff --git a/tests/MenuItem.spec.js b/tests/MenuItem.spec.js index 2cc6a744..b87064c7 100644 --- a/tests/MenuItem.spec.js +++ b/tests/MenuItem.spec.js @@ -1,9 +1,11 @@ /* eslint-disable no-undef */ import React from 'react'; -import { mount } from 'enzyme'; +import { mount, shallow } from 'enzyme'; import KeyCode from 'rc-util/lib/KeyCode'; import Menu, { MenuItem } from '../src'; +import { MenuItem as NakedMenuItem } from '../src/MenuItem'; + describe('MenuItem', () => { describe('disabled', () => { it('can not be active by key down', () => { @@ -33,4 +35,50 @@ describe('MenuItem', () => { expect(handleSelect).not.toBeCalled(); }); }); + + describe('menuItem events', () => { + let onMouseEnter; + let onMouseLeave; + let onItemHover; + let wrapper; + let instance; + const domEvent = { keyCode: 13 }; + const key = '1'; + + beforeEach(() => { + onMouseEnter = jest.fn(); + onMouseLeave = jest.fn(); + onItemHover = jest.fn(); + + wrapper = shallow( + 1); + instance = wrapper.instance(); + }); + + it('on enter key down should trigger mouse click', () => { + instance.onClick = jest.fn(); + instance.onKeyDown(domEvent); + + expect(instance.onClick).toHaveBeenCalledWith(domEvent); + }); + + it('on mouse enter should trigger props.onItemHover props.onMouseEnter', () => { + instance.onMouseEnter(domEvent); + + expect(onItemHover).toHaveBeenCalledWith({ key, hover: true }); + expect(onMouseEnter).toHaveBeenCalledWith({ key, domEvent }); + }); + + it('on mouse leave should trigger props.onItemHover props.onMouseLeave', () => { + instance.onMouseLeave(domEvent); + + expect(onItemHover).toHaveBeenCalledWith({ key, hover: false }); + expect(onMouseLeave).toHaveBeenCalledWith({ key, domEvent }); + }); + }); }); diff --git a/tests/SubMenu.spec.js b/tests/SubMenu.spec.js index 8e14a463..69938e5e 100644 --- a/tests/SubMenu.spec.js +++ b/tests/SubMenu.spec.js @@ -121,6 +121,30 @@ describe('SubMenu', () => { expect(wrapper.find('.rc-menu-sub').first().is('.rc-menu-hidden')).toBe(false); expect(wrapper.find('MenuItem').first().props().active).toBe(false); }); + + it('mouse enter/mouse leave on a subMenu item should trigger hooks', () => { + const onMouseEnter = jest.fn(); + const onMouseLeave = jest.fn(); + const wrapper = mount( + + + 1 + + + ); + const subMenu = wrapper.find('.rc-menu-submenu').first(); + + subMenu.simulate('mouseEnter'); + expect(onMouseEnter).toHaveBeenCalledTimes(1); + + subMenu.simulate('mouseLeave'); + expect(onMouseLeave).toHaveBeenCalledTimes(1); + }); }); describe('key press', () => { @@ -201,6 +225,17 @@ describe('SubMenu', () => { expect(handleSelect.mock.calls[0][0].key).toBe('s1-1'); }); + it('fires select event', () => { + const wrapper = mount(createMenu()); + wrapper.find('.rc-menu-submenu-title').first().simulate('mouseEnter'); + + jest.runAllTimers(); + wrapper.update(); + + wrapper.find('MenuItem').first().simulate('click'); + expect(wrapper.find('.rc-menu-submenu').first().is('.rc-menu-submenu-selected')).toBe(true); + }); + it('fires deselect event for multiple menu', () => { const handleDeselect = jest.fn(); const wrapper = mount(createMenu({ @@ -217,4 +252,81 @@ describe('SubMenu', () => { expect(handleDeselect.mock.calls[0][0].key).toBe('s1-1'); }); + + describe('horizontal menu', () => { + it('should automatically adjust width', () => { + const wrapper = mount(createMenu({ + mode: 'horizontal', + openKeys: ['s1'], + })); + + const subMenuInstance = wrapper.find('SubMenu').first().instance(); + const adjustWidthSpy = jest.spyOn(subMenuInstance, 'adjustWidth'); + + jest.runAllTimers(); + + expect(adjustWidthSpy).toHaveBeenCalledTimes(2); + }); + }); + + describe('submenu animation', () => { + const appear = () => {}; + + it('should animate with transition class', () => { + const wrapper = mount(createMenu({ + openTransitionName: 'fade', + mode: 'inline', + })); + + const title = wrapper.find('.rc-menu-submenu-title').first(); + + title.simulate('click'); + jest.runAllTimers(); + + expect(wrapper.find('Animate').prop('transitionName')).toEqual('fade'); + }); + + it('should animate on initially opened menu', () => { + const wrapper = mount(createMenu({ + openAnimation: { appear }, + mode: 'inline', + openKeys: ['s1'], + })); + + expect(wrapper.find('Animate').first().prop('animation')).toEqual({ appear }); + }); + + it('should animate with config', () => { + const wrapper = mount(createMenu({ + openAnimation: { appear }, + mode: 'inline', + })); + + const title = wrapper.find('.rc-menu-submenu-title').first(); + + title.simulate('click'); + jest.runAllTimers(); + + expect(wrapper.find('Animate').first().prop('animation')).toEqual({ appear }); + }); + }); + + describe('.componentWillUnmount()', () => { + it('should invoke hooks', () => { + const onDestroy = jest.fn(); + const App = (props) => ( + + {props.show && + 1 + } + + ); + + const wrapper = mount(); + + const subMenuInstance = wrapper.setProps({ show: false }); + + expect(onDestroy).toHaveBeenCalledWith('s1'); + }); + }); }); diff --git a/tests/__snapshots__/Menu.spec.js.snap b/tests/__snapshots__/Menu.spec.js.snap index 6b76277f..d5de1dad 100644 --- a/tests/__snapshots__/Menu.spec.js.snap +++ b/tests/__snapshots__/Menu.spec.js.snap @@ -3,7 +3,7 @@ exports[`Menu render renders horizontal menu correctly 1`] = `
    • @@ -126,6 +127,7 @@ exports[`Menu render renders inline menu correctly 1`] = ` aria-selected="false" class="rc-menu-item" role="menuitem" + style="padding-left:24px" > 2 @@ -135,6 +137,7 @@ exports[`Menu render renders inline menu correctly 1`] = ` aria-selected="false" class="rc-menu-item" role="menuitem" + style="padding-left:24px" > 3 @@ -154,6 +157,7 @@ exports[`Menu render renders inline menu correctly 1`] = ` aria-selected="false" class="rc-menu-item" role="menuitem" + style="padding-left:24px" > 4 @@ -162,19 +166,21 @@ exports[`Menu render renders inline menu correctly 1`] = ` aria-selected="false" class="rc-menu-item rc-menu-item-disabled" role="menuitem" + style="padding-left:24px" > 5