Skip to content

Commit

Permalink
[core] fix(Popover): return focus to target on ESC keypress (#6587)
Browse files Browse the repository at this point in the history
  • Loading branch information
adidahiya authored Dec 7, 2023
1 parent 65a0ac7 commit c0cbdc2
Showing 1 changed file with 22 additions and 15 deletions.
37 changes: 22 additions & 15 deletions packages/core/src/components/popover/popover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,9 @@ export interface PopoverProps<TProps extends DefaultPopoverTargetHTMLProps = Def
* Whether the application should return focus to the last active element in the
* document after this popover closes.
*
* This is automatically set (overridden) to `false` for hover interaction popovers.
* This is automatically set (overridden) to:
* - `false` for hover interaction popovers
* - `true` when a popover closes due to an ESC keypress
*
* If you are attaching a popover _and_ a tooltip to the same target, you must take
* care to either disable this prop for the popover _or_ disable the tooltip's
Expand All @@ -131,8 +133,10 @@ export interface PopoverProps<TProps extends DefaultPopoverTargetHTMLProps = Def
}

export interface PopoverState {
isOpen: boolean;
hasDarkParent: boolean;
// when an ESC keypress interaction closes the overlay, we want to force-enable `shouldReturnFocusOnClose` behavior
isClosingViaEscapeKeypress: boolean;
isOpen: boolean;
}

/**
Expand Down Expand Up @@ -174,6 +178,7 @@ export class Popover<

public state: PopoverState = {
hasDarkParent: false,
isClosingViaEscapeKeypress: false,
isOpen: this.getIsOpen(this.props),
};

Expand Down Expand Up @@ -439,17 +444,9 @@ export class Popover<
};

private renderPopover = (popperProps: PopperChildrenProps) => {
const {
autoFocus,
enforceFocus,
backdropProps,
canEscapeKeyClose,
hasBackdrop,
interactionKind,
shouldReturnFocusOnClose,
usePortal,
} = this.props;
const { isOpen } = this.state;
const { autoFocus, enforceFocus, backdropProps, canEscapeKeyClose, hasBackdrop, interactionKind, usePortal } =
this.props;
const { isClosingViaEscapeKeypress, isOpen } = this.state;

// compute an appropriate transform origin so the scale animation points towards target
const transformOrigin = getTransformOrigin(
Expand Down Expand Up @@ -490,6 +487,12 @@ export class Popover<
);

const defaultAutoFocus = this.isHoverInteractionKind() ? false : undefined;
// if hover interaction, it doesn't make sense to take over focus control
const shouldReturnFocusOnClose = this.isHoverInteractionKind()
? false
: isClosingViaEscapeKeypress
? true
: this.props.shouldReturnFocusOnClose;

return (
<Overlay
Expand All @@ -513,8 +516,7 @@ export class Popover<
portalClassName={this.props.portalClassName}
portalContainer={this.props.portalContainer}
portalStopPropagationEvents={this.props.portalStopPropagationEvents}
// if hover interaction, it doesn't make sense to take over focus control
shouldReturnFocusOnClose={this.isHoverInteractionKind() ? false : shouldReturnFocusOnClose}
shouldReturnFocusOnClose={shouldReturnFocusOnClose}
>
<div className={Classes.POPOVER_TRANSITION_CONTAINER} ref={popperProps.ref} style={popperProps.style}>
<ResizeSensor onResize={this.reposition}>
Expand Down Expand Up @@ -747,6 +749,7 @@ export class Popover<
// non-null assertion because the only time `e` is undefined is when in controlled mode
// or the rare special case in uncontrolled mode when the `disabled` flag is toggled true
this.props.onClose?.(e!);
this.setState({ isClosingViaEscapeKeypress: isEscapeKeypressEvent(e?.nativeEvent) });
}
}
}
Expand All @@ -763,6 +766,10 @@ export class Popover<
}
}

function isEscapeKeypressEvent(e?: Event) {
return e instanceof KeyboardEvent && e.key === "Escape";
}

function noop() {
// no-op
}

1 comment on commit c0cbdc2

@adidahiya
Copy link
Contributor Author

Choose a reason for hiding this comment

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

[core] fix(Popover): return focus to target on ESC keypress (#6587)

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.