feat: improve the accessibility of Nav

rsuite · Oct 16, 2020 · 278696e · 278696e
1 parent 268aef9
commit 278696e
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 29 deletions.
diff --git a/src/Breadcrumb/Breadcrumb.tsx b/src/Breadcrumb/Breadcrumb.tsx
@@ -89,6 +89,7 @@ const Breadcrumb: BreadcrumbComponent = React.forwardRef((props: BreadcrumbProps
           <BreadcrumbItem
             role="button"
             key="ellipsis"
+            title={locale.expandText}
             aria-label={locale.expandText}
             onClick={handleClickEllipsis}
           >

diff --git a/src/Nav/Nav.tsx b/src/Nav/Nav.tsx
@@ -1,9 +1,8 @@
 import React from 'react';
 import PropTypes from 'prop-types';
-import shallowEqual from '../utils/shallowEqual';
-
 import NavItem from './NavItem';
-import { ReactChildren, useClassNames } from '../utils';
+import Dropdown from '../Dropdown';
+import { ReactChildren, useClassNames, shallowEqual } from '../utils';
 import { NavbarContext } from '../Navbar/Navbar';
 import { SidenavContext } from '../Sidenav/Sidenav';
 import { WithAsProps, RsRefForwardingComponent } from '../@types/common';
@@ -30,7 +29,7 @@ export interface NavProps<T = any>
   activeKey?: T;
 
   /** Callback function triggered after selection */
-  onSelect?: (eventKey: T, event: React.SyntheticEvent<any>) => void;
+  onSelect?: (eventKey: T, event: React.SyntheticEvent) => void;
 }
 
 const defaultProps: Partial<NavProps> = {
@@ -40,6 +39,7 @@ const defaultProps: Partial<NavProps> = {
 };
 
 interface NavComponent extends RsRefForwardingComponent<'div', NavProps> {
+  Dropdown?: typeof Dropdown;
   Item?: typeof NavItem;
 }
 
@@ -62,22 +62,22 @@ const Nav: NavComponent = React.forwardRef((props: NavProps, ref: React.Ref<HTML
   const { sidenav = false, expanded = false, activeKey = activeKeyProp, onSelect = onSelectProp } =
     React.useContext(SidenavContext) || {};
 
-  const { withClassPrefix, merge, prefix } = useClassNames(classPrefix);
-
+  const { withClassPrefix, merge, rootPrefix, prefix } = useClassNames(classPrefix);
   const hasWaterline = appearance !== 'default';
 
   const items = ReactChildren.mapCloneElement(children, item => {
     const { eventKey, active, ...rest } = item.props;
     const displayName = item?.type?.displayName;
     const hasTooltip = sidenav && !expanded;
-    if (~displayName?.indexOf('NavItem')) {
+
+    if (displayName === 'NavItem') {
       return {
         ...rest,
         onSelect,
         hasTooltip,
         active: typeof activeKey === 'undefined' ? active : shallowEqual(activeKey, eventKey)
       };
-    } else if (~displayName?.indexOf('Dropdown')) {
+    } else if (displayName === 'Dropdown') {
       return {
         ...rest,
         onSelect,
@@ -95,10 +95,12 @@ const Nav: NavComponent = React.forwardRef((props: NavProps, ref: React.Ref<HTML
       {navbar => {
         const classes = merge(
           className,
-          withClassPrefix(appearance, {
+          rootPrefix({
             'navbar-nav': navbar,
             'navbar-right': pullRight,
-            'sidenav-nav': sidenav,
+            'sidenav-nav': sidenav
+          }),
+          withClassPrefix(appearance, {
             horizontal: navbar || (!vertical && !sidenav),
             vertical: vertical || sidenav,
             justified: justified,
@@ -117,9 +119,10 @@ const Nav: NavComponent = React.forwardRef((props: NavProps, ref: React.Ref<HTML
   );
 });
 
+Nav.Dropdown = Dropdown;
 Nav.Item = NavItem;
-Nav.displayName = 'Nav';
 
+Nav.displayName = 'Nav';
 Nav.defaultProps = defaultProps;
 Nav.propTypes = {
   classPrefix: PropTypes.string,

diff --git a/src/Nav/NavItem.tsx b/src/Nav/NavItem.tsx
@@ -1,15 +1,12 @@
-import React from 'react';
+import React, { useCallback } from 'react';
 import PropTypes from 'prop-types';
-
 import Ripple from '../Ripple';
-import appendTooltip from '../utils/appendTooltip';
 import SafeAnchor from '../SafeAnchor';
-
-import { createChainedFunction, useClassNames } from '../utils';
+import { createChainedFunction, useClassNames, appendTooltip } from '../utils';
 import { WithAsProps, RsRefForwardingComponent } from '../@types/common';
-import { IconProps } from '../Icon/Icon.d';
+import { IconProps } from '../Icon';
 
-export interface NavItemProps<T = any>
+export interface NavItemProps<T = string>
   extends WithAsProps,
     Omit<React.HTMLAttributes<HTMLElement>, 'onSelect'> {
   /** Activation status */
@@ -37,7 +34,7 @@ export interface NavItemProps<T = any>
   href?: string;
 
   /** Select the callback function that the event triggers. */
-  onSelect?: (eventKey: T, event: React.SyntheticEvent<any>) => void;
+  onSelect?: (eventKey: T, event: React.SyntheticEvent) => void;
 
   /** Custom rendering item */
   renderItem?: (item: React.ReactNode) => React.ReactNode;
@@ -52,10 +49,9 @@ const defaultProps: Partial<NavItemProps> = {
 const NavItem: RsRefForwardingComponent<'li', NavItemProps> = React.forwardRef(
   (props: NavItemProps, ref: React.Ref<HTMLLIElement>) => {
     const {
+      as: Component,
       active,
       disabled,
-      onClick,
-      onSelect,
       eventKey,
       className,
       classPrefix,
@@ -66,16 +62,20 @@ const NavItem: RsRefForwardingComponent<'li', NavItemProps> = React.forwardRef(
       hasTooltip,
       divider,
       panel,
-      as: Component,
       renderItem,
+      onClick,
+      onSelect,
       ...rest
     } = props;
 
-    const handleClick = (event: React.MouseEvent) => {
-      if (onSelect && !disabled) {
-        onSelect(eventKey, event);
-      }
-    };
+    const handleClick = useCallback(
+      (event: React.MouseEvent) => {
+        if (onSelect && !disabled) {
+          onSelect(eventKey, event);
+        }
+      },
+      [onSelect, disabled, eventKey]
+    );
 
     const { withClassPrefix, merge, prefix } = useClassNames(classPrefix);
     const classes = merge(className, withClassPrefix({ active, disabled }));
@@ -103,7 +103,6 @@ const NavItem: RsRefForwardingComponent<'li', NavItemProps> = React.forwardRef(
       <Component
         {...rest}
         disabled={Component === SafeAnchor ? disabled : null}
-        role="button"
         tabIndex={tabIndex}
         className={merge(className, prefix('content'))}
         onClick={createChainedFunction(onClick, handleClick)}

diff --git a/src/Navbar/Navbar.tsx b/src/Navbar/Navbar.tsx
@@ -33,7 +33,7 @@ const Navbar: NavbarComponent = React.forwardRef(
     const classes = merge(className, withClassPrefix(appearance));
     return (
       <NavbarContext.Provider value={hasChildContext}>
-        <Component {...rest} ref={ref} className={classes} role="navigation" />
+        <Component {...rest} ref={ref} className={classes} />
       </NavbarContext.Provider>
     );
   }