From e5843873af4865b22bce2039bd3c8affb0f5205c Mon Sep 17 00:00:00 2001 From: guifu Date: Fri, 27 Apr 2018 20:14:22 +0800 Subject: [PATCH 1/2] chore: fix aria-* attirbutes --- src/MenuItem.jsx | 22 +++++++++++++++++---- src/SubMenu.jsx | 17 +++++++++++++--- src/SubPopupMenu.js | 4 ++-- tests/__snapshots__/Menu.spec.js.snap | 24 +++-------------------- tests/__snapshots__/MenuItem.spec.js.snap | 5 +---- 5 files changed, 38 insertions(+), 34 deletions(-) diff --git a/src/MenuItem.jsx b/src/MenuItem.jsx index 19e483a6..91be0798 100644 --- a/src/MenuItem.jsx +++ b/src/MenuItem.jsx @@ -11,6 +11,7 @@ import { noop, menuAllProps } from './util'; export class MenuItem extends React.Component { static propTypes = { + attribute: PropTypes.object, rootPrefixCls: PropTypes.string, eventKey: PropTypes.string, active: PropTypes.bool, @@ -145,14 +146,27 @@ export class MenuItem extends React.Component { [this.getSelectedClassName()]: props.isSelected, [this.getDisabledClassName()]: props.disabled, }); - const attrs = { + let attrs = { ...props.attribute, title: props.title, className, + // set to menuitem by default role: 'menuitem', - 'aria-selected': props.isSelected, 'aria-disabled': props.disabled, }; + + if (props.role === 'option') { + // overwrite to option + attrs = { + ...attrs, + role: 'option', + 'aria-selected': props.isSelected, + }; + } else if (attrs.role === null) { + // sometimes we want to specify role inside
  • element + //
  • Link
    • would be a good example + delete attrs.role; + } let mouseEvent = {}; if (!props.disabled) { mouseEvent = { @@ -181,11 +195,11 @@ export class MenuItem extends React.Component { } } +MenuItem.isMenuItem = true; + const connected = connect(({ activeKey, selectedKeys }, { eventKey, subMenuKey }) => ({ active: activeKey[subMenuKey] === eventKey, isSelected: selectedKeys.indexOf(eventKey) !== -1, }))(MenuItem); -connected.isMenuItem = true; - export default connected; diff --git a/src/SubMenu.jsx b/src/SubMenu.jsx index 951fe578..d1e81b06 100644 --- a/src/SubMenu.jsx +++ b/src/SubMenu.jsx @@ -399,7 +399,7 @@ export class SubMenu extends React.Component { component="" transitionAppear={transitionAppear} > - {children} + {children} ); } @@ -448,6 +448,17 @@ export class SubMenu extends React.Component { if (isInlineMode) { style.paddingLeft = props.inlineIndent * props.level; } + + let ariaOwns = {}; + // only set aria-owns when menu is open + // otherwise it would be an invalid aria-owns value + // since corresponding node cannot be found + if (this.props.isOpen) { + ariaOwns = { + 'aria-owns': this._menuId, + }; + } + const title = (
    @@ -479,7 +490,7 @@ export class SubMenu extends React.Component { } = props; menuAllProps.forEach(key => delete props[key]); return ( -
  • +
  • {isInlineMode && title} {isInlineMode && children} {!isInlineMode && ( diff --git a/src/SubPopupMenu.js b/src/SubPopupMenu.js index cd3c71f1..0fec13ab 100644 --- a/src/SubPopupMenu.js +++ b/src/SubPopupMenu.js @@ -316,8 +316,8 @@ export class SubPopupMenu extends React.Component { ); const domProps = { className, - role: 'menu', - 'aria-activedescendant': '', + // role could be 'select' and by default set to menu + role: props.role || 'menu', }; if (props.id) { domProps.id = props.id; diff --git a/tests/__snapshots__/Menu.spec.js.snap b/tests/__snapshots__/Menu.spec.js.snap index 241f13d0..5bb4bd00 100644 --- a/tests/__snapshots__/Menu.spec.js.snap +++ b/tests/__snapshots__/Menu.spec.js.snap @@ -2,7 +2,6 @@ exports[`Menu render renders horizontal menu correctly 1`] = `
  • + test +
    • +`; + +exports[`MenuItem overwrite default role should set specific role 1`] = ` +
  • + test +
    • +`; + exports[`MenuItem rest props can render all props to sub component 1`] = `