Skip to content

Commit

Permalink
[core] feat(MenuItem): new prop 'roleStructure' (#5377)
Browse files Browse the repository at this point in the history
  • Loading branch information
bvandercar-vt authored Jul 8, 2022
1 parent 87ce10a commit 519c09a
Show file tree
Hide file tree
Showing 9 changed files with 62 additions and 15 deletions.
32 changes: 30 additions & 2 deletions packages/core/src/components/menu/menuItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,28 @@ export interface IMenuItemProps extends ActionProps, LinkProps {
*/
labelElement?: React.ReactNode;

/**
* Changes the ARIA `role` property structure of this MenuItem to accomodate for various
* different `role`s of the parent Menu `ul` element.
*
* If `menuitem`, role structure becomes:
*
* `<li role="none"`
* `<a role="menuitem"`
*
* which is proper role structure for a `<ul role="menu"` parent (this is the default `role` of a `Menu`).
*
* If `listoption`, role structure becomes:
*
* `<li role="option"`
* `<a role=undefined`
*
* which is proper role structure for a `<ul role="listbox"` parent, or a `<select>` parent.
*
* @default "menuitem"
*/
roleStructure?: "menuitem" | "listoption";

/**
* Whether the text should be allowed to wrap to multiple lines.
* If `false`, text will be truncated with an ellipsis when it reaches `max-width`.
Expand Down Expand Up @@ -153,6 +175,7 @@ export class MenuItem extends AbstractPureComponent2<MenuItemProps & React.Ancho
labelElement,
multiline,
popoverProps,
roleStructure = "menuitem",
selected,
shouldDismissPopover,
submenuProps,
Expand Down Expand Up @@ -180,10 +203,15 @@ export class MenuItem extends AbstractPureComponent2<MenuItemProps & React.Ancho
className,
);

const [liRole, targetRole, ariaSelected] =
roleStructure === "listoption"
? ["option", undefined, active || selected] // parent has listbox role, or is a <select>
: ["none", "menuitem", undefined]; // parent has menu role

const target = React.createElement(
tagName,
{
role: "menuitem",
role: targetRole,
tabIndex: 0,
...htmlProps,
...(disabled ? DISABLED_PROPS : {}),
Expand All @@ -205,7 +233,7 @@ export class MenuItem extends AbstractPureComponent2<MenuItemProps & React.Ancho

const liClasses = classNames({ [Classes.MENU_SUBMENU]: hasSubmenu });
return (
<li className={liClasses} role="none">
<li className={liClasses} role={liRole} aria-selected={ariaSelected}>
{this.maybeRenderPopover(target, children)}
</li>
);
Expand Down
12 changes: 12 additions & 0 deletions packages/core/test/menu/menuItemTests.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,18 @@ describe("MenuItem", () => {
assert.lengthOf(submenu.props.children, 3);
});

it("default role prop structure is correct for a menuitem that is a an item of a ul with role=menu", () => {
const wrapper = mount(<MenuItem text="Roles" />);
assert.equal(wrapper.find("li").prop("role"), "none");
assert.equal(wrapper.find("a").prop("role"), "menuitem");
});

it("can set roleStructure to change role prop structure to that of a listbox or select item", () => {
const wrapper = mount(<MenuItem text="Roles" roleStructure="listoption" />);
assert.equal(wrapper.find("li").prop("role"), "option");
assert.equal(wrapper.find("a").prop("role"), undefined);
});

it("disabled MenuItem will not show its submenu", () => {
const wrapper = shallow(
<MenuItem disabled={true} icon="style" text="Style">
Expand Down
2 changes: 1 addition & 1 deletion packages/docs-app/src/common/filmSelect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export default function ({ allowCreate = false, fill, ...restProps }: Props) {
createNewItemFromQuery={maybeCreateNewItemFromQuery}
createNewItemRenderer={maybeCreateNewItemRenderer}
itemsEqual={areFilmsEqual}
noResults={<MenuItem disabled={true} text="No results." />}
noResults={<MenuItem disabled={true} text="No results." roleStructure="listoption" />}
onItemSelect={handleItemSelect}
items={items}
fill={fill}
Expand Down
2 changes: 2 additions & 0 deletions packages/docs-app/src/common/films.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ export const renderFilm: ItemRenderer<IFilm> = (film, { handleClick, handleFocus
active={modifiers.active}
disabled={modifiers.disabled}
label={film.year.toString()}
roleStructure="listoption"
key={film.rank}
onClick={handleClick}
onFocus={handleFocus}
Expand All @@ -158,6 +159,7 @@ export const renderCreateFilmOption = (
<MenuItem
icon="add"
text={`Create "${query}"`}
roleStructure="listoption"
active={active}
onClick={handleClick}
shouldDismissPopover={false}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ export class MultiSelectExample extends React.PureComponent<IExampleProps, IMult
});

const initialContent = this.state.hasInitialContent ? (
<MenuItem disabled={true} text={`${TOP_100_FILMS.length} items loaded.`} />
<MenuItem disabled={true} text={`${TOP_100_FILMS.length} items loaded.`} roleStructure="listoption" />
) : // explicit undefined (not null) for default behavior (show full list)
undefined;
const maybeCreateNewItemFromQuery = allowCreate ? createFilm : undefined;
Expand All @@ -124,7 +124,7 @@ export class MultiSelectExample extends React.PureComponent<IExampleProps, IMult
// we may customize the default filmSelectProps.items by
// adding newly created items to the list, so pass our own
items={this.state.items}
noResults={<MenuItem disabled={true} text="No results." />}
noResults={<MenuItem disabled={true} text="No results." roleStructure="listoption" />}
onClear={this.state.showClearButton ? this.handleClear : undefined}
onItemSelect={this.handleFilmSelect}
onItemsPaste={this.handleFilmsPaste}
Expand Down Expand Up @@ -250,9 +250,9 @@ export class MultiSelectExample extends React.PureComponent<IExampleProps, IMult
}
return (
<MenuItem
aria-selected={this.isFilmSelected(film)}
selected={modifiers.active}
icon={this.isFilmSelected(film) ? "tick" : "blank"}
roleStructure="listoption"
key={film.rank}
label={film.year.toString()}
onClick={handleClick}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export class SelectExample extends React.PureComponent<IExampleProps, ISelectExa
const { allowCreate, disabled, disableItems, matchTargetWidth, minimal, ...flags } = this.state;

const initialContent = this.state.hasInitialContent ? (
<MenuItem disabled={true} text={`${TOP_100_FILMS.length} items loaded.`} />
<MenuItem disabled={true} text={`${TOP_100_FILMS.length} items loaded.`} roleStructure="listoption" />
) : undefined;

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ export class SuggestExample extends React.PureComponent<IExampleProps, ISuggestE
// we may customize the default filmSelectProps.items by
// adding newly created items to the list, so pass our own.
items={this.state.items}
noResults={<MenuItem disabled={true} text="No results." />}
noResults={<MenuItem disabled={true} text="No results." roleStructure="listoption" />}
onItemSelect={this.handleValueChange}
popoverProps={{ matchTargetWidth, minimal }}
/>
Expand Down
2 changes: 2 additions & 0 deletions packages/select/src/common/listItemsProps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,8 @@ export interface IListItemsProps<T> extends Props {
* If omitted, nothing will be rendered in this case.
*
* This prop is ignored if a custom `itemListRenderer` is supplied.
*
* NOTE: if passing a `MenuItem`, ensure it has `roleStructure="listoption"` prop.
*/
noResults?: React.ReactNode;

Expand Down
17 changes: 10 additions & 7 deletions packages/select/src/components/select/select2.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ ReactDOM.render(
items={Films.items}
itemPredicate={Films.itemPredicate}
itemRenderer={Films.itemRenderer}
noResults={<MenuItem disabled={true} text="No results." />}
noResults={<MenuItem disabled={true} text="No results." roleStructure="listoption" />}
onItemSelect={...}
>
{/* children become the popover target; render value here */}
Expand Down Expand Up @@ -124,10 +124,10 @@ If you wish, you can allow users to select a brand new item that doesn't appear
in the list, based on the current query string. Use `createNewItemFromQuery` and
`createNewItemRenderer` to enable this:
- `createNewItemFromQuery`: Specifies how to convert a user-entered query string
into an item of type `<T>` that `Select2` understands.
into an item of type `<T>` that `Select2` understands.
- `createNewItemRenderer`: Renders a custom "Create Item" element that will be
shown at the bottom of the list. When selected via click or `Enter`, this element
will invoke `onItemSelect` with the item returned from `createNewItemFromQuery`.
shown at the bottom of the list. When selected via click or `Enter`, this element
will invoke `onItemSelect` with the item returned from `createNewItemFromQuery`.

<div class="@ns-callout @ns-intent-warning @ns-icon-info-sign">
<h4 class="@ns-heading">Avoiding type conflicts</h4>
Expand Down Expand Up @@ -158,6 +158,7 @@ function renderCreateFilmOption(
<MenuItem
icon="add"
text={`Create "${query}"`}
roleStructure="listoption"
selected={active}
onClick={handleClick}
shouldDismissPopover={false}
Expand All @@ -172,7 +173,7 @@ ReactDOM.render(
items={Films.items}
itemPredicate={Films.itemPredicate}
itemRenderer={Films.itemRenderer}
noResults={<MenuItem disabled={true} text="No results." />}
noResults={<MenuItem disabled={true} text="No results." roleStructure="listoption" />}
onItemSelect={...}
/>,
document.querySelector("#root")
Expand Down Expand Up @@ -257,12 +258,13 @@ const renderFilm: ItemRenderer<Film> = (film, { handleClick, handleFocus, modifi
}
return (
<MenuItem
text={film.title}
label={film.year}
roleStructure="listoption"
selected={modifiers.active}
key={film.title}
label={film.year}
onClick={handleClick}
onFocus={handleFocus}
text={film.title}
/>
);
};
Expand All @@ -286,6 +288,7 @@ const renderMenu: ItemListRenderer<Film> = ({ items, itemsParentRef, query, rend
<MenuItem
disabled={true}
text={`Found ${renderedItems.length} items matching "${query}"`}
roleStructure="listoption"
/>
{renderedItems}
</Menu>
Expand Down

1 comment on commit 519c09a

@blueprint-bot
Copy link

Choose a reason for hiding this comment

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

[core] feat(MenuItem): new prop 'roleStructure' (#5377)

Previews: documentation | landing | table | demo

Please sign in to comment.