Skip to content

Commit b8e1367

Browse files
committed
fix(ui-top-nav-bar,ui-popover,ui-drilldown): automatically set aria-expanded, allow override with shouldSetAriaExpanded
This change adds a new prop (shouldSetAriaExpanded) to Popover and Drilldown as well. It defaults to true. It is needed for setting automatically the aria-expanded attribute on the DOM. This can have a side-effect of duplicating aria-expanded in certain edge cases or putting aria-expanded on triggers where it doesn't need to be there. These are very rare cases so we decided to have "break" these cases and fix all the orher ones where aria-expanded was missing INSTUI-4544
1 parent 02b8eaa commit b8e1367

File tree

8 files changed

+38
-12
lines changed

8 files changed

+38
-12
lines changed

packages/ui-drilldown/src/Drilldown/index.tsx

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,8 @@ class Drilldown extends Component<DrilldownProps, DrilldownState> {
121121
shouldReturnFocus: true,
122122
withArrow: true,
123123
offsetX: 0,
124-
offsetY: 0
124+
offsetY: 0,
125+
shouldSetAriaExpanded: true
125126
}
126127

127128
static Group = DrilldownGroup
@@ -1494,11 +1495,13 @@ class Drilldown extends Component<DrilldownProps, DrilldownState> {
14941495
onFocus,
14951496
onMouseOver,
14961497
offsetX,
1497-
offsetY
1498+
offsetY,
1499+
shouldSetAriaExpanded
14981500
} = this.props
14991501

15001502
return trigger ? (
15011503
<Popover
1504+
shouldSetAriaExpanded={shouldSetAriaExpanded}
15021505
isShowingContent={show}
15031506
defaultIsShowingContent={defaultShow}
15041507
shouldCloseOnDocumentClick={true}

packages/ui-drilldown/src/Drilldown/props.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,12 @@ type DrilldownOwnProps = {
261261
* Works only if `trigger` is provided.
262262
*/
263263
offsetY?: string | number
264+
/**
265+
* If true (default), then the aria-expanded prop is added to the trigger.
266+
* If its supplied via the aria-expanded prop then it takes the given value,
267+
* otherwise its calculated automatically based on whether the content is shown.
268+
*/
269+
shouldSetAriaExpanded?: boolean
264270
}
265271

266272
type PropKeys = keyof DrilldownOwnProps
@@ -338,7 +344,8 @@ const propTypes: PropValidators<PropKeys> = {
338344
shouldReturnFocus: PropTypes.bool,
339345
withArrow: PropTypes.bool,
340346
offsetX: PropTypes.oneOfType([PropTypes.string, PropTypes.number]),
341-
offsetY: PropTypes.oneOfType([PropTypes.string, PropTypes.number])
347+
offsetY: PropTypes.oneOfType([PropTypes.string, PropTypes.number]),
348+
shouldSetAriaExpanded: PropTypes.bool
342349
}
343350

344351
const allowedProps: AllowedPropKeys = [

packages/ui-popover/src/Popover/index.tsx

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,8 @@ class Popover extends Component<PopoverProps, PopoverState> {
9898
shouldReturnFocus: true,
9999
shouldCloseOnDocumentClick: true,
100100
shouldFocusContentOnTriggerBlur: false,
101-
shouldCloseOnEscape: true
101+
shouldCloseOnEscape: true,
102+
shouldSetAriaExpanded: true
102103
}
103104

104105
constructor(props: PopoverProps) {
@@ -437,7 +438,7 @@ class Popover extends Component<PopoverProps, PopoverState> {
437438
let trigger = this._renderTrigger
438439

439440
if (trigger) {
440-
const { on, shouldContainFocus } = this.props
441+
const { on } = this.props
441442

442443
let onClick: React.MouseEventHandler | undefined = undefined
443444
let onFocus: React.FocusEventHandler | undefined = undefined
@@ -468,13 +469,13 @@ class Popover extends Component<PopoverProps, PopoverState> {
468469
}
469470
}
470471

471-
if (shouldContainFocus) {
472+
if (this.props.shouldSetAriaExpanded) {
472473
// only set aria-expanded if popover can contain focus
473474
expanded = this.shown ? 'true' : 'false'
474475

475476
if ('aria-expanded' in this.props) {
476477
// @ts-expect-error It is an escape hatch, in case someone
477-
// wants to remove/override aria-expanded even when shouldContainFocus
478+
// wants to remove/override aria-expanded even when shouldSetAriaExpanded is set
478479
expanded = this.props['aria-expanded']
479480
}
480481
} else {

packages/ui-popover/src/Popover/props.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,12 @@ type PopoverOwnProps = {
283283
* Only applies to a Popover without an arrow.
284284
*/
285285
borderWidth?: BorderWidth
286+
/**
287+
* If true (default), then the aria-expanded prop is added to the trigger.
288+
* If its supplied via the aria-expanded prop then it takes the given value,
289+
* otherwise its calculated automatically based on whether the content is shown.
290+
*/
291+
shouldSetAriaExpanded?: boolean
286292
}
287293

288294
type PopoverProps = PopoverOwnProps &
@@ -353,7 +359,8 @@ const propTypes: PropValidators<PropKeys> = {
353359
renderTrigger: PropTypes.oneOfType([PropTypes.node, PropTypes.func]),
354360
children: PropTypes.oneOfType([PropTypes.node, PropTypes.func]),
355361
elementRef: PropTypes.func,
356-
borderWidth: ThemeablePropTypes.borderWidth
362+
borderWidth: ThemeablePropTypes.borderWidth,
363+
shouldSetAriaExpanded: PropTypes.bool
357364
}
358365

359366
const allowedProps: AllowedPropKeys = [
@@ -399,7 +406,8 @@ const allowedProps: AllowedPropKeys = [
399406
'renderTrigger',
400407
'children',
401408
'elementRef',
402-
'borderWidth'
409+
'borderWidth',
410+
'shouldSetAriaExpanded'
403411
]
404412

405413
export type { PopoverOwnProps, PopoverProps, PopoverState, PopoverStyle }

packages/ui-tooltip/src/Tooltip/index.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,7 @@ class Tooltip extends Component<TooltipProps, TooltipState> {
177177
elementRef={this.handleRef}
178178
shouldCloseOnDocumentClick={false}
179179
shouldCloseOnEscape
180+
shouldSetAriaExpanded={false}
180181
>
181182
{!preventTooltip ? (
182183
<span id={this._id} css={styles?.tooltip} role="tooltip">

packages/ui-top-nav-bar/src/TopNavBar/TopNavBarActionItems/index.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,10 @@ class TopNavBarActionItems extends Component<
223223
tooltip={tooltip}
224224
showSubmenuChevron={false}
225225
renderSubmenu={
226-
<Drilldown rootPageId={this._hiddenActionItemsMenuId}>
226+
<Drilldown
227+
shouldSetAriaExpanded={false}
228+
rootPageId={this._hiddenActionItemsMenuId}
229+
>
227230
{[
228231
<Drilldown.Page
229232
id={this._hiddenActionItemsMenuId}

packages/ui-top-nav-bar/src/TopNavBar/TopNavBarMenuItems/index.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,10 @@ class TopNavBarMenuItems extends Component<
163163
id={this._hiddenMenuItemsMenuTriggerId}
164164
status={hasActiveChild ? 'active' : 'default'}
165165
renderSubmenu={
166-
<Drilldown rootPageId={this._hiddenItemsMenuId}>
166+
<Drilldown
167+
shouldSetAriaExpanded={false}
168+
rootPageId={this._hiddenItemsMenuId}
169+
>
167170
{[
168171
<Drilldown.Page
169172
id={this._hiddenItemsMenuId}

packages/ui-top-nav-bar/src/TopNavBar/utils/exampleHelpers.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ const avatarExample = {
8484
}
8585

8686
const itemSubmenuExample = (
87-
<Drilldown rootPageId="root">
87+
<Drilldown shouldSetAriaExpanded={false} rootPageId="root">
8888
<Drilldown.Page id="root">
8989
<Drilldown.Option id="linkExampleOption1" href="/#One">
9090
Link One

0 commit comments

Comments
 (0)