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(MenuItem, MenuItem2): roleStructure="none" #5840

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
15 changes: 12 additions & 3 deletions packages/core/src/components/menu/menuItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -86,18 +86,25 @@ export interface IMenuItemProps extends ActionProps, LinkProps, IElementRefProps
* `<li role="option"`
* `<a role=undefined`
*
* which is proper role structure for a `<ul role="listbox"` parent, or a `<select>` parent.
* which is proper role structure for a `<ul role="listbox"` parent, or a `<select>` parent.
*
* If `listitem`, role structure becomes:
*
* `<li role=undefined`
* `<a role=undefined`
*
* which can be used if this item is within a basic `<ul/>` (or `role="list"`) parent.
* which can be used if this item is within a basic `<ul/>` (or `role="list"`) parent.
*
* If `none`, role structure becomes:
*
* `<li role="none"`
* `<a role=undefined`
*
* which can be used if wrapping this item in a custom `<li>` parent.
*
* @default "menuitem"
*/
roleStructure?: "menuitem" | "listoption" | "listitem";
roleStructure?: "menuitem" | "listoption" | "listitem" | "none";

/**
* Whether the text should be allowed to wrap to multiple lines.
Expand Down Expand Up @@ -213,6 +220,8 @@ export class MenuItem extends AbstractPureComponent2<MenuItemProps & React.Ancho
? ["option", undefined, active || selected] // parent has listbox role, or is a <select>
: roleStructure === "menuitem"
? ["none", "menuitem", undefined] // parent has menu role
: roleStructure === "none"
? ["none", undefined, undefined] // if wrapping in a custom <li>
: [undefined, undefined, undefined]; // roleStructure === "listitem"-- needs no role prop, li is listitem by default

const target = React.createElement(
Expand Down
2 changes: 2 additions & 0 deletions packages/popover2/src/menu-item2.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ depending on the `role` attribute of its parent `<ul>` list:
`<li role="option">` and `<a>` (anchor role undefined).
- `roleStructure="listitem"` is appropriate for a `<ul>` (no role defined) or a `<ul role="list">` parent. The
item will render with `<li>` and `<a>` (roles undefined).
- `roleStructure="none"` is useful when wrapping in a custom `<li>`. The
item will render with `<li role="none">` and `<a>` (roles undefined).

@## Selection state

Expand Down
20 changes: 17 additions & 3 deletions packages/popover2/src/menuItem2.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -94,18 +94,25 @@ export interface MenuItem2Props extends ActionProps, LinkProps, IElementRefProps
* `<li role="option"`
* `<a role=undefined`
*
* which is proper role structure for a `<ul role="listbox"` parent, or a `<select>` parent.
* which is proper role structure for a `<ul role="listbox"` parent, or a `<select>` parent.
*
* If `listitem`, role structure becomes:
*
* `<li role=undefined`
* `<a role=undefined`
*
* which can be used if this item is within a basic `<ul/>` (or `role="list"`) parent.
* which can be used if this item is within a basic `<ul/>` (or `role="list"`) parent.
*
* If `none`, role structure becomes:
*
* `<li role="none"`
* `<a role=undefined`
*
* which can be used if wrapping this item in a custom `<li>` parent.
*
* @default "menuitem"
*/
roleStructure?: "menuitem" | "listoption" | "listitem";
roleStructure?: "menuitem" | "listoption" | "listitem" | "none";

/**
* Whether the text should be allowed to wrap to multiple lines.
Expand Down Expand Up @@ -218,6 +225,13 @@ export class MenuItem2 extends AbstractPureComponent2<MenuItem2Props & React.Anc
this.props.icon,
undefined, // don't set aria-selected prop
]
: roleStructure === "none" // "none": allows wrapping MenuItem in custom <li>
? [
"none",
undefined, // target should have no role
this.props.icon,
undefined, // don't set aria-selected prop
]
: // roleStructure === "listitem"
[
undefined, // needs no role prop, li is listitem by default
Expand Down