Skip to content

Commit

Permalink
feat(MenuItem, MenuItem2): roleStructure="none" (#5840)
Browse files Browse the repository at this point in the history
Co-authored-by: Adi Dahiya <adahiya@palantir.com>
  • Loading branch information
bvandercar-vt and adidahiya authored Jan 23, 2023
1 parent 38a25b8 commit b44ec38
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 6 deletions.
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

1 comment on commit b44ec38

@adidahiya
Copy link
Contributor

Choose a reason for hiding this comment

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

feat(MenuItem, MenuItem2): roleStructure="none" (#5840)

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

Please sign in to comment.