Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: improve key type to support React.Key #692

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 10 additions & 9 deletions src/Menu.tsx
Expand Up @@ -24,6 +24,7 @@ import type {
ItemType,
MenuClickEventHandler,
MenuInfo,
MenuKey,
MenuMode,
MenuRef,
RenderIconType,
Expand Down Expand Up @@ -76,18 +77,18 @@ export interface MenuProps

// Open control
defaultOpenKeys?: string[];
openKeys?: string[];
openKeys?: MenuKey[];

// Active control
activeKey?: string;
activeKey?: MenuKey;
defaultActiveFirst?: boolean;

// Selection
selectable?: boolean;
multiple?: boolean;

defaultSelectedKeys?: string[];
selectedKeys?: string[];
defaultSelectedKeys?: MenuKey[];
selectedKeys?: MenuKey[];

onSelect?: SelectEventHandler;
onDeselect?: SelectEventHandler;
Expand Down Expand Up @@ -120,7 +121,7 @@ export interface MenuProps

// >>>>> Events
onClick?: MenuClickEventHandler;
onOpenChange?: (openKeys: string[]) => void;
onOpenChange?: (openKeys: MenuKey[]) => void;

// >>>>> Internal
/***
Expand Down Expand Up @@ -260,7 +261,7 @@ const Menu = React.forwardRef<MenuRef, MenuProps>((props, ref) => {

// React 18 will merge mouse event which means we open key will not sync
// ref: https://github.com/ant-design/ant-design/issues/38818
const triggerOpenKeys = (keys: string[], forceFlush = false) => {
const triggerOpenKeys = (keys: MenuKey[], forceFlush = false) => {
function doUpdate() {
setMergedOpenKeys(keys);
onOpenChange?.(keys);
Expand Down Expand Up @@ -439,7 +440,7 @@ const Menu = React.forwardRef<MenuRef, MenuProps>((props, ref) => {
// Insert or Remove
const { key: targetKey } = info;
const exist = mergedSelectKeys.includes(targetKey);
let newSelectKeys: string[];
let newSelectKeys: MenuKey[];

if (multiple) {
if (exist) {
Expand Down Expand Up @@ -481,7 +482,7 @@ const Menu = React.forwardRef<MenuRef, MenuProps>((props, ref) => {
triggerSelection(info);
});

const onInternalOpenChange = useMemoCallback((key: string, open: boolean) => {
const onInternalOpenChange = useMemoCallback((key: MenuKey, open: boolean) => {
let newOpenKeys = mergedOpenKeys.filter(k => k !== key);

if (open) {
Expand All @@ -498,7 +499,7 @@ const Menu = React.forwardRef<MenuRef, MenuProps>((props, ref) => {
});

// ==================== Accessibility =====================
const triggerAccessibilityOpen = (key: string, open?: boolean) => {
const triggerAccessibilityOpen = (key: MenuKey, open?: boolean) => {
const nextOpen = open ?? !mergedOpenKeys.includes(key);

onInternalOpenChange(key, nextOpen);
Expand Down
4 changes: 2 additions & 2 deletions src/MenuItem.tsx
Expand Up @@ -12,7 +12,7 @@ import PrivateContext from './context/PrivateContext';
import useActive from './hooks/useActive';
import useDirectionStyle from './hooks/useDirectionStyle';
import Icon from './Icon';
import type { MenuInfo, MenuItemType } from './interface';
import type { MenuInfo, MenuItemType, MenuKey } from './interface';
import { warnItemProp } from './utils/warnUtil';

export interface MenuItemProps
Expand All @@ -24,7 +24,7 @@ export interface MenuItemProps
children?: React.ReactNode;

/** @private Internal filled key. Do not set it directly */
eventKey?: string;
eventKey?: MenuKey;

/** @private Do not use. Private warning empty usage */
warnKey?: boolean;
Expand Down
4 changes: 2 additions & 2 deletions src/MenuItemGroup.tsx
Expand Up @@ -3,7 +3,7 @@ import omit from 'rc-util/lib/omit';
import * as React from 'react';
import { MenuContext } from './context/MenuContext';
import { useFullPath, useMeasure } from './context/PathContext';
import type { MenuItemGroupType } from './interface';
import type { MenuItemGroupType, MenuKey } from './interface';
import { parseChildren } from './utils/commonUtil';

export interface MenuItemGroupProps
Expand All @@ -13,7 +13,7 @@ export interface MenuItemGroupProps
children?: React.ReactNode;

/** @private Internal filled key. Do not set it directly */
eventKey?: string;
eventKey?: MenuKey;

/** @private Do not use. Private warning empty usage */
warnKey?: boolean;
Expand Down
4 changes: 2 additions & 2 deletions src/SubMenu/InlineSubMenuList.tsx
Expand Up @@ -3,12 +3,12 @@ import CSSMotion from 'rc-motion';
import { getMotion } from '../utils/motionUtil';
import MenuContextProvider, { MenuContext } from '../context/MenuContext';
import SubMenuList from './SubMenuList';
import type { MenuMode } from '../interface';
import type { MenuKey, MenuMode } from '../interface';

export interface InlineSubMenuListProps {
id?: string;
open: boolean;
keyPath: string[];
keyPath: MenuKey[];
children: React.ReactNode;
}

Expand Down
4 changes: 2 additions & 2 deletions src/SubMenu/index.tsx
Expand Up @@ -4,7 +4,7 @@ import Overflow from 'rc-overflow';
import warning from 'rc-util/lib/warning';
import SubMenuList from './SubMenuList';
import { parseChildren } from '../utils/commonUtil';
import type { MenuInfo, SubMenuType } from '../interface';
import type { MenuInfo, MenuKey, SubMenuType } from '../interface';
import MenuContextProvider, { MenuContext } from '../context/MenuContext';
import useMemoCallback from '../hooks/useMemoCallback';
import PopupTrigger from './PopupTrigger';
Expand Down Expand Up @@ -32,7 +32,7 @@ export interface SubMenuProps
internalPopupClose?: boolean;

/** @private Internal filled key. Do not set it directly */
eventKey?: string;
eventKey?: MenuKey;

/** @private Do not use. Private warning empty usage */
warnKey?: boolean;
Expand Down
5 changes: 3 additions & 2 deletions src/context/IdContext.ts
@@ -1,8 +1,9 @@
import { MenuKey } from '@/interface';
import * as React from 'react';

export const IdContext = React.createContext<string>(null);

export function getMenuId(uuid: string, eventKey: string) {
export function getMenuId(uuid: string, eventKey: MenuKey) {
if (uuid === undefined) {
return null;
}
Expand All @@ -12,7 +13,7 @@ export function getMenuId(uuid: string, eventKey: string) {
/**
* Get `data-menu-id`
*/
export function useMenuId(eventKey: string) {
export function useMenuId(eventKey: MenuKey) {
const id = React.useContext(IdContext);
return getMenuId(id, eventKey);
}
13 changes: 7 additions & 6 deletions src/context/MenuContext.tsx
Expand Up @@ -5,6 +5,7 @@ import isEqual from 'rc-util/lib/isEqual';
import type {
BuiltinPlacements,
MenuClickEventHandler,
MenuKey,
MenuMode,
RenderIconType,
TriggerSubMenuAction,
Expand All @@ -13,7 +14,7 @@ import type {
export interface MenuContextProps {
prefixCls: string;
rootClassName?: string;
openKeys: string[];
openKeys: MenuKey[];
rtl?: boolean;

// Mode
Expand All @@ -25,12 +26,12 @@ export interface MenuContextProps {
overflowDisabled?: boolean;

// Active
activeKey: string;
onActive: (key: string) => void;
onInactive: (key: string) => void;
activeKey: MenuKey;
onActive: (key: MenuKey) => void;
onInactive: (key: MenuKey) => void;

// Selection
selectedKeys: string[];
selectedKeys: MenuKey[];

// Level
inlineIndent: number;
Expand All @@ -52,7 +53,7 @@ export interface MenuContextProps {

// Function
onItemClick: MenuClickEventHandler;
onOpenChange: (key: string, open: boolean) => void;
onOpenChange: (key: MenuKey, open: boolean) => void;
getPopupContainer: (node: HTMLElement) => HTMLElement;
}

Expand Down
11 changes: 6 additions & 5 deletions src/context/PathContext.tsx
@@ -1,11 +1,12 @@
import { MenuKey } from '@/interface';
import * as React from 'react';

const EmptyList: string[] = [];

// ========================= Path Register =========================
export interface PathRegisterContextProps {
registerPath: (key: string, keyPath: string[]) => void;
unregisterPath: (key: string, keyPath: string[]) => void;
registerPath: (key: MenuKey, keyPath: MenuKey[]) => void;
unregisterPath: (key: MenuKey, keyPath: MenuKey[]) => void;
}

export const PathRegisterContext = React.createContext<PathRegisterContextProps>(
Expand All @@ -17,9 +18,9 @@ export function useMeasure() {
}

// ========================= Path Tracker ==========================
export const PathTrackerContext = React.createContext<string[]>(EmptyList);
export const PathTrackerContext = React.createContext<MenuKey[]>(EmptyList);

export function useFullPath(eventKey?: string) {
export function useFullPath(eventKey?: MenuKey) {
const parentKeyPath = React.useContext(PathTrackerContext);
return React.useMemo(
() =>
Expand All @@ -30,7 +31,7 @@ export function useFullPath(eventKey?: string) {

// =========================== Path User ===========================
export interface PathUserContextProps {
isSubPathKey: (pathKeys: string[], eventKey: string) => boolean;
isSubPathKey: (pathKeys: MenuKey[], eventKey: MenuKey) => boolean;
}

export const PathUserContext = React.createContext<PathUserContextProps>(null);
20 changes: 10 additions & 10 deletions src/hooks/useAccessibility.ts
Expand Up @@ -3,7 +3,7 @@ import KeyCode from 'rc-util/lib/KeyCode';
import raf from 'rc-util/lib/raf';
import * as React from 'react';
import { getMenuId } from '../context/IdContext';
import type { MenuMode } from '../interface';
import type { MenuKey, MenuMode } from '../interface';

// destruct to reduce minify size
const { LEFT, RIGHT, UP, DOWN, ENTER, ESC, HOME, END } = KeyCode;
Expand Down Expand Up @@ -181,10 +181,10 @@ function getNextFocusElement(
return sameLevelFocusableMenuElementList[focusIndex];
}

export const refreshElements = (keys: string[], id: string) => {
export const refreshElements = (keys: MenuKey[], id: string) => {
const elements = new Set<HTMLElement>();
const key2element = new Map<string, HTMLElement>();
const element2key = new Map<HTMLElement, string>();
const key2element = new Map<MenuKey, HTMLElement>();
const element2key = new Map<HTMLElement, MenuKey>();

keys.forEach(key => {
const element = document.querySelector(
Expand All @@ -203,22 +203,22 @@ export const refreshElements = (keys: string[], id: string) => {

export function useAccessibility<T extends HTMLElement>(
mode: MenuMode,
activeKey: string,
activeKey: MenuKey,
isRtl: boolean,
id: string,

containerRef: React.RefObject<HTMLUListElement>,
getKeys: () => string[],
getKeyPath: (key: string, includeOverflow?: boolean) => string[],
getKeys: () => MenuKey[],
getKeyPath: (key: MenuKey, includeOverflow?: boolean) => string[],

triggerActiveKey: (key: string) => void,
triggerAccessibilityOpen: (key: string, open?: boolean) => void,
triggerActiveKey: (key: MenuKey) => void,
triggerAccessibilityOpen: (key: MenuKey, open?: boolean) => void,

originOnKeyDown?: React.KeyboardEventHandler<T>,
): React.KeyboardEventHandler<T> {
const rafRef = React.useRef<number>();

const activeRef = React.useRef<string>();
const activeRef = React.useRef<MenuKey>();
activeRef.current = activeKey;

const cleanRaf = () => {
Expand Down
4 changes: 2 additions & 2 deletions src/hooks/useActive.ts
@@ -1,6 +1,6 @@
import * as React from 'react';
import { MenuContext } from '../context/MenuContext';
import type { MenuHoverEventHandler } from '../interface';
import type { MenuHoverEventHandler, MenuKey } from '../interface';

interface ActiveObj {
active: boolean;
Expand All @@ -9,7 +9,7 @@ interface ActiveObj {
}

export default function useActive(
eventKey: string,
eventKey: MenuKey,
disabled: boolean,
onMouseEnter?: MenuHoverEventHandler,
onMouseLeave?: MenuHoverEventHandler,
Expand Down
9 changes: 5 additions & 4 deletions src/hooks/useKeyRecords.ts
Expand Up @@ -2,6 +2,7 @@ import * as React from 'react';
import { useRef, useCallback } from 'react';
import warning from 'rc-util/lib/warning';
import { nextSlice } from '../utils/timeUtil';
import { MenuKey } from '@/interface';

const PATH_SPLIT = '__RC_UTIL_PATH_SPLIT__';

Expand All @@ -12,8 +13,8 @@ export const OVERFLOW_KEY = 'rc-menu-more';

export default function useKeyRecords() {
const [, internalForceUpdate] = React.useState({});
const key2pathRef = useRef(new Map<string, string>());
const path2keyRef = useRef(new Map<string, string>());
const key2pathRef = useRef(new Map<MenuKey, string>());
const path2keyRef = useRef(new Map<MenuKey, string>());
const [overflowKeys, setOverflowKeys] = React.useState([]);
const updateRef = useRef(0);
const destroyRef = useRef(false);
Expand Down Expand Up @@ -95,12 +96,12 @@ export default function useKeyRecords() {
/**
* Find current key related child path keys
*/
const getSubPathKeys = useCallback((key: string): Set<string> => {
const getSubPathKeys = useCallback((key: MenuKey): Set<MenuKey> => {
const connectedPath = `${key2pathRef.current.get(key)}${PATH_SPLIT}`;
const pathKeys = new Set<string>();

[...path2keyRef.current.keys()].forEach(pathKey => {
if (pathKey.startsWith(connectedPath)) {
if (`${pathKey}`.startsWith(connectedPath)) {
pathKeys.add(path2keyRef.current.get(pathKey));
}
});
Expand Down
14 changes: 8 additions & 6 deletions src/interface.ts
Expand Up @@ -6,14 +6,16 @@ interface ItemSharedProps {
className?: string;
}

export type MenuKey = React.Key;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个好不容易是 string 了,又要改会 key 吗,如果要改,改成 T 吧

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

React.Key 会有问题吗?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

里面包含 number,如果我想执行处理字符串的方法,还需要转一次字符串

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

还是希望 string | number 都能支持,并且输入的类型和输出的类型一致,如果合并了 #692 ,那 React.Key 默认是不是就支持了?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

用泛型

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

范型 +1,React.Key 有时会包含奇怪的东西,让用户自己指定 key 类型比较好


export interface SubMenuType extends ItemSharedProps {
label?: React.ReactNode;

children: ItemType[];

disabled?: boolean;

key: string;
key: MenuKey;

rootClassName?: string;

Expand Down Expand Up @@ -92,27 +94,27 @@ export type RenderIconType =
| ((props: RenderIconInfo) => React.ReactNode);

export interface MenuInfo {
key: string;
keyPath: string[];
key: MenuKey;
keyPath: MenuKey[];
/** @deprecated This will not support in future. You should avoid to use this */
item: React.ReactInstance;
domEvent: React.MouseEvent<HTMLElement> | React.KeyboardEvent<HTMLElement>;
}

export interface MenuTitleInfo {
key: string;
key: MenuKey;
domEvent: React.MouseEvent<HTMLElement> | React.KeyboardEvent<HTMLElement>;
}

// ========================== Hover ==========================
export type MenuHoverEventHandler = (info: {
key: string;
key: MenuKey;
domEvent: React.MouseEvent<HTMLElement>;
}) => void;

// ======================== Selection ========================
export interface SelectInfo extends MenuInfo {
selectedKeys: string[];
selectedKeys: MenuKey[];
}

export type SelectEventHandler = (info: SelectInfo) => void;
Expand Down