Skip to content

Fix aria attributes in ContextualMenu and CommandBar #3951

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.

Sign up for GitHub

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

Merged
manishgarg1 merged 3 commits into from
Feb 13, 2018
Merged

Fix aria attributes in ContextualMenu and CommandBar #3951

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e3325b3
Add aria-setsize and -posinset to command bar items.
T-Hugs Feb 7, 2018
33fa304
Fix the aria-owns property on contextual menu items.
T-Hugs Feb 12, 2018
8ae27ad
Add unit test for aria properties on CommandBar.
T-Hugs Feb 12, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions common/changes/office-ui-fabric-react/master_2018-02-12-21-15.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"changes": [
{
"packageName": "office-ui-fabric-react",
"comment": "Fix aria-owns in contextual menu items, and add appropriate aria properties to CommandBar items.",
"type": "patch"
}
],
"packageName": "office-ui-fabric-react",
"email": "[email protected]"
}
46 changes: 46 additions & 0 deletions packages/office-ui-fabric-react/src/components/CommandBar/CommandBar.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,52 @@ describe('CommandBar', () => {
expect(tree).toMatchSnapshot();
});

it('adds the correct aria-setsize and -posinset attributes to the command bar items.', () => {
const items: IContextualMenuItem[] = [
{
name: 'TestText 1',
key: 'TestKey1',
className: 'item1',
subMenuProps: {
items: [
{
name: 'SubmenuText 1',
key: 'SubmenuKey1',
className: 'SubMenuClass'
}
]
}
},
{
name: 'TestText 2',
key: 'TestKey2',
className: 'item2',
},
{
name: 'TestText 3',
key: 'TestKey3',
className: 'item3',
},
];

const renderedContent = ReactTestUtils.renderIntoDocument<CommandBar>(
<CommandBar
items={ items }
/>
) as React.Component<CommandBar, {}>;
document.body.appendChild(ReactDOM.findDOMNode(renderedContent));

console.log(document.body.innerHTML);

const [item1, item2, item3] = ['.item1', '.item2', '.item3'].map(i => document.querySelector(i)!.children[0]);
expect(item1.getAttribute('aria-setsize')).toBe('3');
expect(item2.getAttribute('aria-setsize')).toBe('3');
expect(item3.getAttribute('aria-setsize')).toBe('3');
expect(item1.getAttribute('aria-posinset')).toBe('1');
expect(item2.getAttribute('aria-posinset')).toBe('2');
expect(item3.getAttribute('aria-posinset')).toBe('3');
});

it('opens a menu with deprecated IContextualMenuItem.items property', () => {
const items: IContextualMenuItem[] = [
{
Expand Down
23 changes: 17 additions & 6 deletions packages/office-ui-fabric-react/src/components/CommandBar/CommandBar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,14 +110,17 @@ export class CommandBar extends BaseComponent<ICommandBarProps, ICommandBarState
</div>
);
}
// Total # of menu items is regular items + far items + 1 for the ellipsis, if necessary
const setSize = renderedItems!.length + renderedFarItems!.length + (renderedOverflowItems && renderedOverflowItems.length > 0 ? 1 : 0);
let posInSet = 1;

return (
<div className={ css('ms-CommandBar', styles.root, className) } ref='commandBarRegion'>
{ searchBox }
<FocusZone ref='focusZone' className={ styles.container } direction={ FocusZoneDirection.horizontal } role='menubar' >
<div className={ css('ms-CommandBar-primaryCommands', styles.primaryCommands) } ref='commandSurface'>
{ renderedItems!.map((item, index) => (
this._renderItemInCommandBar(item, index, expandedMenuItemKey!)
{ renderedItems!.map(item => (
this._renderItemInCommandBar(item, posInSet++, setSize, expandedMenuItemKey!)
Copy link
Collaborator

@manishgarg1 manishgarg1 Feb 12, 2018

Choose a reason for hiding this comment

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

posInSet [](start = 49, length = 8)

Can you please add another value to the snapshot tests such that the count increment is working correctly. Or a unit test would be equally good.

Copy link
Contributor Author

@T-Hugs T-Hugs Feb 12, 2018

Choose a reason for hiding this comment

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

Not sure what you meant by "add another value to the snapshot tests such that the count increment is working correctly" so I added a unit test.

Copy link
Member

@dzearing dzearing Feb 13, 2018

Choose a reason for hiding this comment

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

That works! But he just meant add another item in the items of the command bar in the snapshot unit test which is easier to at least validate in the snapshot that the setSize works. :)

Copy link
Contributor Author

@T-Hugs T-Hugs Feb 13, 2018

Choose a reason for hiding this comment

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

Ok, thanks David!

)).concat((renderedOverflowItems && renderedOverflowItems.length) ? [
<div className={ css('ms-CommandBarItem', styles.item) } key={ OVERFLOW_KEY } ref={ OVERFLOW_KEY }>
<button
Expand All @@ -131,6 +134,8 @@ export class CommandBar extends BaseComponent<ICommandBarProps, ICommandBarState
aria-expanded={ this.state.expandedMenuItemKey === OVERFLOW_KEY }
aria-label={ this.props.elipisisAriaLabel || '' }
aria-haspopup={ true }
aria-setsize={ setSize }
aria-posinset={ posInSet++ }
data-automation-id='commandBarOverflow'
>
<Icon className={ css('ms-CommandBarItem-overflow', styles.itemOverflow) } iconName='more' />
Expand All @@ -139,8 +144,8 @@ export class CommandBar extends BaseComponent<ICommandBarProps, ICommandBarState
] : []) }
</div>
<div className={ css('ms-CommandBar-sideCommands', styles.sideCommands) } ref='farCommandSurface'>
{ renderedFarItems!.map((item, index) => (
this._renderItemInCommandBar(item, index, expandedMenuItemKey!, true)
{ renderedFarItems!.map(item => (
this._renderItemInCommandBar(item, posInSet++, setSize, expandedMenuItemKey!, true)
)) }
</div>
</FocusZone>
Expand All @@ -163,7 +168,7 @@ export class CommandBar extends BaseComponent<ICommandBarProps, ICommandBarState
this.refs.focusZone.focus();
}

private _renderItemInCommandBar(item: ICommandBarItemProps, index: number, expandedMenuItemKey: string, isFarItem?: boolean) {
private _renderItemInCommandBar(item: ICommandBarItemProps, posInSet: number, setSize: number, expandedMenuItemKey: string, isFarItem?: boolean) {
if (item.onRender) {
return (
<div className={ css('ms-CommandBarItem', styles.item, item.className) } key={ item.key } ref={ item.key }>
Expand All @@ -172,7 +177,7 @@ export class CommandBar extends BaseComponent<ICommandBarProps, ICommandBarState
);
}

const itemKey = item.key || String(index);
const itemKey = item.key || String(posInSet);
const isLink = item.onClick || hasSubmenu(item);
const className = css(
isLink ? ('ms-CommandBarItem-link ' + styles.itemLink) : ('ms-CommandBarItem-text ' + styles.itemText),
Expand All @@ -196,6 +201,8 @@ export class CommandBar extends BaseComponent<ICommandBarProps, ICommandBarState
aria-expanded={ hasSubmenu(item) ? expandedMenuItemKey === item.key : undefined }
role='menuitem'
aria-label={ ariaLabel }
aria-setsize={ setSize }
aria-posinset={ posInSet }
>
{ (hasIcon) ? this._renderIcon(item) : (null) }
{ isNameVisible && (
Expand All @@ -222,6 +229,8 @@ export class CommandBar extends BaseComponent<ICommandBarProps, ICommandBarState
aria-haspopup={ hasSubmenu(item) }
role='menuitem'
aria-label={ ariaLabel }
aria-setsize={ setSize }
aria-posinset={ posInSet }
>
{ (hasIcon) ? this._renderIcon(item) : (null) }
{ isNameVisible && (
Expand All @@ -243,6 +252,8 @@ export class CommandBar extends BaseComponent<ICommandBarProps, ICommandBarState
data-command-key={ itemKey }
aria-haspopup={ hasSubmenu(item) }
aria-label={ ariaLabel }
aria-setsize={ setSize }
aria-posinset={ posInSet }
>
{ (hasIcon) ? this._renderIcon(item) : (null) }
{ (isNameVisible) && (
Expand Down
2 changes: 2 additions & 0 deletions ...s/office-ui-fabric-react/src/components/CommandBar/__snapshots__/CommandBar.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ exports[`CommandBar renders CommandBar correctly 1`] = `
aria-expanded={false}
aria-haspopup={true}
aria-label={undefined}
aria-posinset={1}
aria-setsize={1}
className="ms-CommandBarItem-link undefined"
data-command-key="TestKey1"
id="CommandBar0TestKey1"
Expand Down
33 changes: 33 additions & 0 deletions packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,39 @@ describe('ContextualMenu', () => {
expect(document.querySelector('.SubMenuClass')).toBeDefined();
});

it('sets the correct aria-owns attribute for the submenu', () => {
const submenuId = 'testSubmenuId';
const items: IContextualMenuItem[] = [
{
name: 'TestText 1',
key: 'TestKey1',
subMenuProps: {
id: submenuId,
items: [
{
name: 'SubmenuText 1',
key: 'SubmenuKey1',
className: 'SubMenuClass'
}
]
}
},
];

ReactTestUtils.renderIntoDocument<ContextualMenu>(
<ContextualMenu
items={ items }
/>
);

const parentMenuItem = document.querySelector('button.ms-ContextualMenu-link') as HTMLButtonElement;
ReactTestUtils.Simulate.click(parentMenuItem);
const childMenu = document.getElementById(submenuId);

expect(childMenu!.id).toBe(submenuId);
expect(parentMenuItem.getAttribute('aria-owns')).toBe(submenuId);
});

it('still works with deprecated IContextualMenuItem.items property', () => {
const items: IContextualMenuItem[] = [
{
Expand Down
11 changes: 8 additions & 3 deletions packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,10 @@ export class ContextualMenu extends BaseComponent<IContextualMenuProps, IContext
hasCheckmarks?: boolean,
hasIcons?: boolean) {
let { expandedMenuItemKey, subMenuId } = this.state;
if (item.subMenuProps && item.subMenuProps.id) {
subMenuId = item.subMenuProps.id;
}

let ariaLabel = '';

if (item.ariaLabel) {
Expand All @@ -503,11 +507,12 @@ export class ContextualMenu extends BaseComponent<IContextualMenuProps, IContext
const isChecked: boolean | null | undefined = getIsChecked(item);
const canCheck: boolean = isChecked !== null;
const defaultRole = canCheck ? 'menuitemcheckbox' : 'menuitem';
const itemHasSubmenu = hasSubmenu(item);

const itemButtonProperties = {
className: classNames.root,
onClick: this._onItemClick.bind(this, item),
onKeyDown: hasSubmenu(item) ? this._onItemKeyDown.bind(this, item) : null,
onKeyDown: itemHasSubmenu ? this._onItemKeyDown.bind(this, item) : null,
onMouseEnter: this._onItemMouseEnter.bind(this, item),
onMouseLeave: this._onMouseItemLeave.bind(this, item),
onMouseDown: (ev: any) => this._onItemMouseDown(item, ev),
Expand All @@ -516,9 +521,9 @@ export class ContextualMenu extends BaseComponent<IContextualMenuProps, IContext
href: item.href,
title: item.title,
'aria-label': ariaLabel,
'aria-haspopup': hasSubmenu(item) || null,
'aria-haspopup': itemHasSubmenu || null,
'aria-owns': item.key === expandedMenuItemKey ? subMenuId : null,
'aria-expanded': hasSubmenu(item) ? item.key === expandedMenuItemKey : null,
'aria-expanded': itemHasSubmenu ? item.key === expandedMenuItemKey : null,
'aria-checked': isChecked,
'aria-posinset': focusableElementIndex + 1,
'aria-setsize': totalItemCount,
Expand Down