From 093394dfdf52f039bd33192bc5500b0ed5514b05 Mon Sep 17 00:00:00 2001 From: afc163 Date: Tue, 23 Feb 2021 15:45:15 +0800 Subject: [PATCH 1/2] fix: duplicated selected menu item when selectedKeys is string close https://github.com/ant-design/ant-design/issues/29429 --- src/MenuItem.tsx | 6 +++++- tests/Menu.spec.js | 23 +++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/MenuItem.tsx b/src/MenuItem.tsx index 0e96386f..99ed3184 100644 --- a/src/MenuItem.tsx +++ b/src/MenuItem.tsx @@ -251,7 +251,11 @@ export class MenuItem extends React.Component { const connected = connect( ({ activeKey, selectedKeys }, { eventKey, subMenuKey }) => ({ active: activeKey[subMenuKey] === eventKey, - isSelected: selectedKeys.indexOf(eventKey) !== -1, + // selectedKeys should be array in any circumstance + // when it is not, we have fallback logic for https://github.com/ant-design/ant-design/issues/29430 + isSelected: Array.isArray(selectedKeys) + ? selectedKeys.indexOf(eventKey) !== -1 + : selectedKeys === eventKey, }), )(MenuItem); diff --git a/tests/Menu.spec.js b/tests/Menu.spec.js index ad61b53c..824686a9 100644 --- a/tests/Menu.spec.js +++ b/tests/Menu.spec.js @@ -221,6 +221,29 @@ describe('Menu', () => { ).toContain('-selected'); }); + it('issue https://github.com/ant-design/ant-design/issues/29429', () => { + // don't use selectedKeys as string + // it is a compatible feature for https://github.com/ant-design/ant-design/issues/29429 + const wrapper = mount( + + 1 + 2 + , + ); + expect( + wrapper + .find('li') + .at(0) + .props().className, + ).not.toContain('-selected'); + expect( + wrapper + .find('li') + .at(1) + .props().className, + ).toContain('-selected'); + }); + it('can be controlled by openKeys', () => { const wrapper = mount( From 3c1c67cefd70d4c90c86546ff3a8a347abc58727 Mon Sep 17 00:00:00 2001 From: afc163 Date: Tue, 23 Feb 2021 16:09:49 +0800 Subject: [PATCH 2/2] fix eslint --- src/DOMWrap.tsx | 18 ++++++++--------- src/Menu.tsx | 16 +++++++-------- src/MenuItem.tsx | 10 ++++----- src/MenuItemGroup.tsx | 4 ++-- src/SubMenu.tsx | 32 +++++++++++++++-------------- src/SubPopupMenu.tsx | 45 ++++++++++++++++++++++------------------- src/util.ts | 10 ++++----- src/utils/legacyUtil.ts | 4 ++-- 8 files changed, 72 insertions(+), 67 deletions(-) diff --git a/src/DOMWrap.tsx b/src/DOMWrap.tsx index ed587017..2f994d1f 100644 --- a/src/DOMWrap.tsx +++ b/src/DOMWrap.tsx @@ -2,7 +2,7 @@ import * as React from 'react'; import ResizeObserver from 'resize-observer-polyfill'; import SubMenu from './SubMenu'; import { getWidth, setStyle, menuAllProps } from './util'; -import { MenuMode } from './interface'; +import type { MenuMode } from './interface'; const MENUITEM_OVERFLOWED_CLASSNAME = 'menuitem-overflowed'; const FLOAT_PRECISION_ADJUST = 0.5; @@ -60,7 +60,7 @@ class DOMWrap extends React.Component { if (!menuUl) { return; } - this.resizeObserver = new ResizeObserver(entries => { + this.resizeObserver = new ResizeObserver((entries) => { entries.forEach(() => { const { cancelFrameId } = this; cancelAnimationFrame(cancelFrameId); @@ -166,7 +166,7 @@ class DOMWrap extends React.Component { const popupClassName = theme ? `${prefixCls}-${theme}` : ''; const props = {}; - menuAllProps.forEach(k => { + menuAllProps.forEach((k) => { if (rest[k] !== undefined) { props[k] = rest[k]; } @@ -218,16 +218,16 @@ class DOMWrap extends React.Component { // and then reset to original state after width calculation const overflowedItems = menuItemNodes.filter( - c => c.className.split(' ').indexOf(MENUITEM_OVERFLOWED_CLASSNAME) >= 0, + (c) => c.className.split(' ').indexOf(MENUITEM_OVERFLOWED_CLASSNAME) >= 0, ); - overflowedItems.forEach(c => { + overflowedItems.forEach((c) => { setStyle(c, 'display', 'inline-block'); }); - this.menuItemSizes = menuItemNodes.map(c => getWidth(c, true)); + this.menuItemSizes = menuItemNodes.map((c) => getWidth(c, true)); - overflowedItems.forEach(c => { + overflowedItems.forEach((c) => { setStyle(c, 'display', 'none'); }); this.overflowedIndicatorWidth = getWidth( @@ -266,7 +266,7 @@ class DOMWrap extends React.Component { if (this.originalTotalWidth > width + FLOAT_PRECISION_ADJUST) { lastVisibleIndex = -1; - this.menuItemSizes.forEach(liWidth => { + this.menuItemSizes.forEach((liWidth) => { currentSumWidth += liWidth; if (currentSumWidth + this.overflowedIndicatorWidth <= width) { lastVisibleIndex += 1; @@ -314,7 +314,7 @@ class DOMWrap extends React.Component { if (index === lastVisibleIndex + 1) { this.overflowedItems = children .slice(lastVisibleIndex + 1) - .map(c => + .map((c) => React.cloneElement( c, // children[index].key will become '.$key' in clone by default, diff --git a/src/Menu.tsx b/src/Menu.tsx index 8eeeed12..5048476a 100644 --- a/src/Menu.tsx +++ b/src/Menu.tsx @@ -1,10 +1,10 @@ import * as React from 'react'; import { Provider, create } from 'mini-store'; import omit from 'rc-util/lib/omit'; -import { CSSMotionProps } from 'rc-motion'; +import type { CSSMotionProps } from 'rc-motion'; import SubPopupMenu, { getActiveKey } from './SubPopupMenu'; import { noop } from './util'; -import { +import type { RenderIconType, SelectInfo, SelectEventHandler, @@ -69,7 +69,7 @@ export interface MenuProps export interface MenuState { switchingModeFromInline: boolean; prevProps: MenuProps; - inlineOpenKeys: Array; + inlineOpenKeys: string[]; store: MiniStore; } @@ -235,7 +235,7 @@ class Menu extends React.Component { } }; - onClick: MenuClickEventHandler = e => { + onClick: MenuClickEventHandler = (e) => { const mode = this.getRealMenuMode(); const { store, @@ -259,11 +259,11 @@ class Menu extends React.Component { this.innerMenu.getWrappedInstance().onKeyDown(e, callback); }; - onOpenChange = event => { + onOpenChange = (event) => { const { props } = this; const openKeys = this.store.getState().openKeys.concat(); let changed = false; - const processSingle = e => { + const processSingle = (e) => { let oneChanged = false; if (e.open) { oneChanged = openKeys.indexOf(e.key) === -1; @@ -379,7 +379,7 @@ class Menu extends React.Component { } } - setInnerMenu = node => { + setInnerMenu = (node) => { this.innerMenu = node; }; @@ -427,7 +427,7 @@ class Menu extends React.Component { return ( - + {this.props.children} diff --git a/src/MenuItem.tsx b/src/MenuItem.tsx index 99ed3184..26b030ea 100644 --- a/src/MenuItem.tsx +++ b/src/MenuItem.tsx @@ -4,7 +4,7 @@ import classNames from 'classnames'; import omit from 'rc-util/lib/omit'; import { connect } from 'mini-store'; import { noop, menuAllProps } from './util'; -import { +import type { SelectEventHandler, HoverEventHandler, DestroyEventHandler, @@ -91,7 +91,7 @@ export class MenuItem extends React.Component { return undefined; }; - onMouseLeave: React.MouseEventHandler = e => { + onMouseLeave: React.MouseEventHandler = (e) => { const { eventKey, onItemHover, onMouseLeave } = this.props; onItemHover({ key: eventKey, @@ -103,7 +103,7 @@ export class MenuItem extends React.Component { }); }; - onMouseEnter: React.MouseEventHandler = e => { + onMouseEnter: React.MouseEventHandler = (e) => { const { eventKey, onItemHover, onMouseEnter } = this.props; onItemHover({ key: eventKey, @@ -115,7 +115,7 @@ export class MenuItem extends React.Component { }); }; - onClick: React.MouseEventHandler = e => { + onClick: React.MouseEventHandler = (e) => { const { eventKey, multiple, @@ -221,7 +221,7 @@ export class MenuItem extends React.Component { style.paddingLeft = props.inlineIndent * props.level; } } - menuAllProps.forEach(key => delete props[key]); + menuAllProps.forEach((key) => delete props[key]); delete props.direction; let icon = this.props.itemIcon; if (typeof this.props.itemIcon === 'function') { diff --git a/src/MenuItemGroup.tsx b/src/MenuItemGroup.tsx index f65a3678..ac533e19 100644 --- a/src/MenuItemGroup.tsx +++ b/src/MenuItemGroup.tsx @@ -1,6 +1,6 @@ import * as React from 'react'; import { menuAllProps } from './util'; -import { MenuClickEventHandler } from './interface'; +import type { MenuClickEventHandler } from './interface'; export interface MenuItemGroupProps { disabled?: boolean; @@ -36,7 +36,7 @@ class MenuItemGroup extends React.Component { const titleClassName = `${rootPrefixCls}-item-group-title`; const listClassName = `${rootPrefixCls}-item-group-list`; const { title, children } = props; - menuAllProps.forEach(key => delete props[key]); + menuAllProps.forEach((key) => delete props[key]); // Set onClick to null, to ignore propagated onClick event delete props.onClick; diff --git a/src/SubMenu.tsx b/src/SubMenu.tsx index b6a89114..b44ac2da 100644 --- a/src/SubMenu.tsx +++ b/src/SubMenu.tsx @@ -3,10 +3,12 @@ import * as ReactDOM from 'react-dom'; import Trigger from 'rc-trigger'; import raf from 'rc-util/lib/raf'; import KeyCode from 'rc-util/lib/KeyCode'; -import CSSMotion, { CSSMotionProps } from 'rc-motion'; +import type { CSSMotionProps } from 'rc-motion'; +import CSSMotion from 'rc-motion'; import classNames from 'classnames'; import { connect } from 'mini-store'; -import SubPopupMenu, { SubPopupMenuProps } from './SubPopupMenu'; +import type { SubPopupMenuProps } from './SubPopupMenu'; +import SubPopupMenu from './SubPopupMenu'; import { placements, placementsRtl } from './placements'; import { noop, @@ -14,7 +16,7 @@ import { getMenuIdFromSubMenuEventKey, menuAllProps, } from './util'; -import { +import type { MiniStore, RenderIconType, LegacyFunctionRef, @@ -29,7 +31,7 @@ import { TriggerSubMenuAction, HoverEventHandler, } from './interface'; -import { MenuItem } from './MenuItem'; +import type { MenuItem } from './MenuItem'; let guid = 0; @@ -232,7 +234,7 @@ export class SubMenu extends React.Component { * This legacy code that `onKeyDown` is called by parent instead of dom self. * which need return code to check if this event is handled */ - onKeyDown: React.KeyboardEventHandler = e => { + onKeyDown: React.KeyboardEventHandler = (e) => { const { keyCode } = e; const menu = this.menuInstance; const { store } = this.props; @@ -275,7 +277,7 @@ export class SubMenu extends React.Component { return undefined; }; - onOpenChange: OpenEventHandler = e => { + onOpenChange: OpenEventHandler = (e) => { this.props.onOpenChange(e); }; @@ -283,7 +285,7 @@ export class SubMenu extends React.Component { this.triggerOpenChange(visible, visible ? 'mouseenter' : 'mouseleave'); }; - onMouseEnter: React.MouseEventHandler = e => { + onMouseEnter: React.MouseEventHandler = (e) => { const { eventKey: key, onMouseEnter, store } = this.props; updateDefaultActiveFirst(store, this.props.eventKey, false); onMouseEnter({ @@ -292,7 +294,7 @@ export class SubMenu extends React.Component { }); }; - onMouseLeave: React.MouseEventHandler = e => { + onMouseLeave: React.MouseEventHandler = (e) => { const { parentMenu, eventKey, onMouseLeave } = this.props; parentMenu.subMenuInstance = this; onMouseLeave({ @@ -301,7 +303,7 @@ export class SubMenu extends React.Component { }); }; - onTitleMouseEnter: React.MouseEventHandler = domEvent => { + onTitleMouseEnter: React.MouseEventHandler = (domEvent) => { const { eventKey: key, onItemHover, onTitleMouseEnter } = this.props; onItemHover({ key, @@ -313,7 +315,7 @@ export class SubMenu extends React.Component { }); }; - onTitleMouseLeave: React.MouseEventHandler = e => { + onTitleMouseLeave: React.MouseEventHandler = (e) => { const { parentMenu, eventKey, onItemHover, onTitleMouseLeave } = this.props; parentMenu.subMenuInstance = this; onItemHover({ @@ -349,11 +351,11 @@ export class SubMenu extends React.Component { } }; - onSelect: SelectEventHandler = info => { + onSelect: SelectEventHandler = (info) => { this.props.onSelect(info); }; - onDeselect: SelectEventHandler = info => { + onDeselect: SelectEventHandler = (info) => { this.props.onDeselect(info); }; @@ -649,12 +651,12 @@ export class SubMenu extends React.Component { subMenuCloseDelay, builtinPlacements, } = props; - menuAllProps.forEach(key => delete props[key]); + menuAllProps.forEach((key) => delete props[key]); // Set onClick to null, to ignore propagated onClick event delete props.onClick; const placement = isRTL - ? Object.assign({}, placementsRtl, builtinPlacements) - : Object.assign({}, placements, builtinPlacements); + ? { ...placementsRtl, ...builtinPlacements } + : { ...placements, ...builtinPlacements }; delete props.direction; // [Legacy] It's a fast fix, diff --git a/src/SubPopupMenu.tsx b/src/SubPopupMenu.tsx index 14a35170..10467584 100644 --- a/src/SubPopupMenu.tsx +++ b/src/SubPopupMenu.tsx @@ -1,6 +1,6 @@ import * as React from 'react'; import { connect } from 'mini-store'; -import { CSSMotionProps } from 'rc-motion'; +import type { CSSMotionProps } from 'rc-motion'; import KeyCode from 'rc-util/lib/KeyCode'; import createChainedFunction from 'rc-util/lib/createChainedFunction'; import toArray from 'rc-util/lib/Children/toArray'; @@ -14,7 +14,7 @@ import { isMobileDevice, } from './util'; import DOMWrap from './DOMWrap'; -import { +import type { SelectEventHandler, OpenEventHandler, DestroyEventHandler, @@ -28,14 +28,14 @@ import { MenuInfo, TriggerSubMenuAction, } from './interface'; -import { MenuItem, MenuItemProps } from './MenuItem'; -import { MenuItemGroupProps } from './MenuItemGroup'; +import type { MenuItem, MenuItemProps } from './MenuItem'; +import type { MenuItemGroupProps } from './MenuItemGroup'; function allDisabled(arr: MenuItem[]) { if (!arr.length) { return true; } - return arr.every(c => !!c.props.disabled); + return arr.every((c) => !!c.props.disabled); } function updateActiveKey( @@ -96,16 +96,19 @@ export function getActiveKey( } export function saveRef(c: React.ReactInstance) { - if (c) { - const index = this.instanceArray.indexOf(c); - if (index !== -1) { - // update component if it's already inside instanceArray - this.instanceArray[index] = c; - } else { - // add component if it's not in instanceArray yet; - this.instanceArray.push(c); - } + if (!c) { + return; + } + /* eslint-disable @typescript-eslint/no-invalid-this */ + const index = this.instanceArray.indexOf(c); + if (index !== -1) { + // update component if it's already inside instanceArray + this.instanceArray[index] = c; + } else { + // add component if it's not in instanceArray yet; + this.instanceArray.push(c); } + /* eslint-enable @typescript-eslint/no-invalid-this */ } export interface SubPopupMenuProps { @@ -262,7 +265,7 @@ export class SubPopupMenu extends React.Component { return undefined; }; - onItemHover: HoverEventHandler = e => { + onItemHover: HoverEventHandler = (e) => { const { key, hover } = e; updateActiveKey( this.props.store, @@ -271,23 +274,23 @@ export class SubPopupMenu extends React.Component { ); }; - onDeselect: SelectEventHandler = selectInfo => { + onDeselect: SelectEventHandler = (selectInfo) => { this.props.onDeselect(selectInfo); }; - onSelect: SelectEventHandler = selectInfo => { + onSelect: SelectEventHandler = (selectInfo) => { this.props.onSelect(selectInfo); }; - onClick: MenuClickEventHandler = e => { + onClick: MenuClickEventHandler = (e) => { this.props.onClick(e); }; - onOpenChange: OpenEventHandler = e => { + onOpenChange: OpenEventHandler = (e) => { this.props.onOpenChange(e); }; - onDestroy: DestroyEventHandler = key => { + onDestroy: DestroyEventHandler = (key) => { /* istanbul ignore next */ this.props.onDestroy(key); }; @@ -447,7 +450,7 @@ export class SubPopupMenu extends React.Component { overflowedIndicator, theme, } = props; - menuAllProps.forEach(key => delete props[key]); + menuAllProps.forEach((key) => delete props[key]); // Otherwise, the propagated click event will trigger another onClick delete props.onClick; diff --git a/src/util.ts b/src/util.ts index 1125825f..9f1ab844 100644 --- a/src/util.ts +++ b/src/util.ts @@ -1,8 +1,8 @@ import * as React from 'react'; import isMobile from './utils/isMobile'; -import MenuItemGroup from './MenuItemGroup'; -import SubMenu from './SubMenu'; -import MenuItem from './MenuItem'; +import type MenuItemGroup from './MenuItemGroup'; +import type SubMenu from './SubMenu'; +import type MenuItem from './MenuItem'; export function noop() {} @@ -48,10 +48,10 @@ export function loopMenuItemRecursively( } React.Children.forEach(children, (c: React.ReactElement) => { if (c) { - const construct = c.type as ( + const construct = c.type as | typeof MenuItemGroup | typeof MenuItem - | typeof SubMenu); + | typeof SubMenu; if ( !construct || !( diff --git a/src/utils/legacyUtil.ts b/src/utils/legacyUtil.ts index ab791384..7e885864 100644 --- a/src/utils/legacyUtil.ts +++ b/src/utils/legacyUtil.ts @@ -1,6 +1,6 @@ import warning from 'rc-util/lib/warning'; -import { CSSMotionProps } from 'rc-motion'; -import { OpenAnimation, MenuMode } from '../interface'; +import type { CSSMotionProps } from 'rc-motion'; +import type { OpenAnimation, MenuMode } from '../interface'; interface GetMotionProps { motion?: CSSMotionProps;