Skip to content

Commit

Permalink
[core] fix(Popover): apply aria attrs to child, not wrapper (#6604)
Browse files Browse the repository at this point in the history
  • Loading branch information
bvandercar-vt authored Jan 4, 2024
1 parent 6b63649 commit d8a5e6a
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 7 deletions.
14 changes: 8 additions & 6 deletions packages/core/src/components/popover/popover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -373,12 +373,6 @@ export class Popover<
// Ensure target is focusable if relevant prop enabled
const targetTabIndex = openOnTargetFocus && isHoverInteractionKind ? 0 : undefined;
const ownTargetProps = {
"aria-expanded": isOpen,
"aria-haspopup":
this.props.popupKind ??
(this.props.interactionKind === PopoverInteractionKind.HOVER_TARGET_ONLY
? undefined
: ("true" as const)),
// N.B. this.props.className is passed along to renderTarget even though the user would have access to it.
// If, instead, renderTarget is undefined and the target is provided as a child, this.props.className is
// applied to the generated target wrapper element.
Expand All @@ -390,6 +384,12 @@ export class Popover<
ref,
...targetEventHandlers,
} satisfies React.HTMLProps<HTMLElement>;
const childTargetProps = {
"aria-expanded": isOpen,
"aria-haspopup":
this.props.popupKind ??
(this.props.interactionKind === PopoverInteractionKind.HOVER_TARGET_ONLY ? undefined : true),
} satisfies React.HTMLProps<HTMLElement>;

const targetModifierClasses = {
// this class is mainly useful for Blueprint <Button> targets; we should only apply it for
Expand All @@ -404,6 +404,7 @@ export class Popover<
if (renderTarget !== undefined) {
target = renderTarget({
...ownTargetProps,
...childTargetProps,
className: classNames(ownTargetProps.className, targetModifierClasses),
// if the consumer renders a tooltip target, it's their responsibility to disable that tooltip
// when *this* popover is open
Expand All @@ -418,6 +419,7 @@ export class Popover<
}

const clonedTarget: JSX.Element = React.cloneElement(childTarget, {
...childTargetProps,
className: classNames(childTarget.props.className, targetModifierClasses),
// force disable single Tooltip child when popover is open
disabled: isOpen && Utils.isElementOfType(childTarget, Tooltip) ? true : childTarget.props.disabled,
Expand Down
2 changes: 1 addition & 1 deletion packages/core/test/popover/popoverTests.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ describe("<Popover>", () => {

it("renders with aria-haspopup attr", () => {
wrapper = renderPopover({ isOpen: true });
assert.isTrue(wrapper.find("[aria-haspopup='true']").exists());
assert.isTrue(wrapper.find("[aria-haspopup=true]").exists());
});

it("sets aria-haspopup attr base on popupKind", () => {
Expand Down

1 comment on commit d8a5e6a

@adidahiya
Copy link
Contributor

Choose a reason for hiding this comment

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

[core] fix(Popover): apply aria attrs to child, not wrapper (#6604)

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.