Skip to content

Commit

Permalink
Merge branch 'bvandercar/popover-autofocus-fix' into bvandercar/fix-o…
Browse files Browse the repository at this point in the history 
…verlay-autoFocus
  • Loading branch information
@bvandercar-vt
bvandercar-vt committed Jan 27, 2025
2 parents ce42430 + 0c87edd commit 61d22c7
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 39 deletions.
8 changes: 4 additions & 4 deletions packages/core/src/components/overlay/overlay.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ export class Overlay extends AbstractPureComponent<OverlayProps, OverlayState> {
* the `startFocusTrapElement`), depending on whether the element losing focus is inside the
* Overlay.
*/
private handleStartFocusTrapElementFocus = (e: React.FocusEvent<HTMLDivElement>) => {
private handleStartFocusTrapElementFocus = (e: React.FocusEvent<HTMLDivElement, Element>) => {
if (!this.props.enforceFocus || this.isAutoFocusing) {
return;
}
Expand All @@ -293,7 +293,7 @@ export class Overlay extends AbstractPureComponent<OverlayProps, OverlayState> {
// element in this transition group.
if (
e.relatedTarget != null &&
this.containerElement.current?.contains(e.relatedTarget as Element) &&
this.containerElement.current?.contains(e.relatedTarget) &&
e.relatedTarget !== this.endFocusTrapElement.current
) {
this.endFocusTrapElement.current?.focus({ preventScroll: true });
Expand Down Expand Up @@ -323,7 +323,7 @@ export class Overlay extends AbstractPureComponent<OverlayProps, OverlayState> {
* `startFocusTrapElement`), depending on whether the element losing focus is inside the
* Overlay.
*/
private handleEndFocusTrapElementFocus = (e: React.FocusEvent<HTMLDivElement>) => {
private handleEndFocusTrapElementFocus = (e: React.FocusEvent<HTMLDivElement, Element>) => {
// No need for this.props.enforceFocus check here because this element is only rendered
// when that prop is true.
// During user interactions, e.relatedTarget will be defined, and we should wrap around to the
Expand All @@ -332,7 +332,7 @@ export class Overlay extends AbstractPureComponent<OverlayProps, OverlayState> {
// presses shift+tab from the first focusable element in the overlay.
if (
e.relatedTarget != null &&
this.containerElement.current?.contains(e.relatedTarget as Element) &&
this.containerElement.current?.contains(e.relatedTarget) &&
e.relatedTarget !== this.startFocusTrapElement.current
) {
const firstFocusableElement = getKeyboardFocusableElements(this.containerElement).shift();
Expand Down
16 changes: 6 additions & 10 deletions packages/core/src/components/overlay2/overlay2.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,10 @@ export const Overlay2 = React.forwardRef<OverlayInstance, Overlay2Props>((props,
/** Ref for backdrop element */
const backdropElement = React.useRef<HTMLDivElement>(null);

/* An empty, keyboard-focusable div at the beginning of the Overlay content */
/** An empty, keyboard-focusable div at the beginning of the Overlay content */
const startFocusTrapElement = React.useRef<HTMLDivElement>(null);

/* An empty, keyboard-focusable div at the end of the Overlay content */
/** An empty, keyboard-focusable div at the end of the Overlay content */
const endFocusTrapElement = React.useRef<HTMLDivElement>(null);

/**
Expand Down Expand Up @@ -477,7 +477,7 @@ export const Overlay2 = React.forwardRef<OverlayInstance, Overlay2Props>((props,
* Overlay.
*/
const handleStartFocusTrapElementFocus = React.useCallback(
(e: React.FocusEvent<HTMLDivElement>) => {
(e: React.FocusEvent<HTMLDivElement, Element>) => {
if (!enforceFocus || isAutoFocusing) {
return;
}
Expand All @@ -487,11 +487,7 @@ export const Overlay2 = React.forwardRef<OverlayInstance, Overlay2Props>((props,
// element in this transition group.
const container = getRef(containerElement);
const endFocusTrap = getRef(endFocusTrapElement);
if (
e.relatedTarget != null &&
container?.contains(e.relatedTarget as Element) &&
e.relatedTarget !== endFocusTrap
) {
if (e.relatedTarget != null && container?.contains(e.relatedTarget) && e.relatedTarget !== endFocusTrap) {
endFocusTrap?.focus({ preventScroll: true });
}
},
Expand Down Expand Up @@ -525,7 +521,7 @@ export const Overlay2 = React.forwardRef<OverlayInstance, Overlay2Props>((props,
* Overlay.
*/
const handleEndFocusTrapElementFocus = React.useCallback(
(e: React.FocusEvent<HTMLDivElement>) => {
(e: React.FocusEvent<HTMLDivElement, Element>) => {
// No need for this.props.enforceFocus check here because this element is only rendered
// when that prop is true.
// During user interactions, e.relatedTarget will be defined, and we should wrap around to the
Expand All @@ -535,7 +531,7 @@ export const Overlay2 = React.forwardRef<OverlayInstance, Overlay2Props>((props,
const startFocusTrap = getRef(startFocusTrapElement);
if (
e.relatedTarget != null &&
getRef(containerElement)?.contains(e.relatedTarget as Element) &&
getRef(containerElement)?.contains(e.relatedTarget) &&
e.relatedTarget !== startFocusTrap
) {
const firstFocusableElement = getKeyboardFocusableElements(containerElement).shift();
Expand Down
55 changes: 30 additions & 25 deletions packages/core/src/components/popover/popover.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import {
} from "../../common";
import * as Errors from "../../common/errors";
import { Overlay2 } from "../overlay2/overlay2";
import type { OverlayInstance } from "../overlay2/overlayInstance";
import { ResizeSensor } from "../resize-sensor/resizeSensor";
// eslint-disable-next-line import/no-cycle
import { Tooltip } from "../tooltip/tooltip";
Expand Down Expand Up @@ -198,27 +199,29 @@ export class Popover<
*/
public targetRef = React.createRef<HTMLElement>();

/**
* Overlay2 transition container element ref.
*/
private transitionContainerElement = React.createRef<HTMLDivElement>();
/** Overlay2 transition container element ref */
private transitionContainerRef = React.createRef<HTMLDivElement>();

/** Overlay2 overlay container ref */
private overlayRef = React.createRef<OverlayInstance>();

private cancelOpenTimeout?: () => void;

// a flag that lets us detect mouse movement between the target and popover,
// now that mouseleave is triggered when you cross the gap between the two.
/**
* A flag that lets us detect mouse movement between the target and popover,
* now that mouseleave is triggered when you cross the gap between the two.
*/
private isMouseInTargetOrPopover = false;

// a flag that indicates whether the target previously lost focus to another
// element on the same page.
/** A flag that indicates whether the target previously lost focus to another element on the same page */
private lostFocusOnSamePage = true;

// Reference to the Poppper.scheduleUpdate() function, this changes every time the popper is mounted
/** Reference to the Poppper.scheduleUpdate() function, this changes every time the popper is mounted */
private popperScheduleUpdate?: () => Promise<Partial<PopperState> | null>;

private isControlled = () => this.props.isOpen !== undefined;

// arrow is disabled if minimal, or if the arrow modifier was explicitly disabled
/** Arrow is disabled if minimal, or if the arrow modifier was explicitly disabled */
private isArrowEnabled = () => !this.props.minimal && this.props.modifiers?.arrow?.enabled !== false;

private isHoverInteractionKind = () => {
Expand Down Expand Up @@ -503,7 +506,7 @@ export class Popover<
backdropProps={backdropProps}
canEscapeKeyClose={canEscapeKeyClose}
canOutsideClickClose={interactionKind === PopoverInteractionKind.CLICK}
childRef={this.transitionContainerElement}
childRef={this.transitionContainerRef}
enforceFocus={enforceFocus}
hasBackdrop={hasBackdrop}
isOpen={isOpen}
Expand All @@ -513,6 +516,7 @@ export class Popover<
onClosing={this.props.onClosing}
onOpened={this.props.onOpened}
onOpening={this.props.onOpening}
ref={this.overlayRef}
transitionDuration={this.props.transitionDuration}
transitionName={Classes.POPOVER}
usePortal={usePortal}
Expand All @@ -529,7 +533,7 @@ export class Popover<
// accepts a ref object (not a callback) due to a CSSTransition API limitation.
// N.B. react-popper has a wide type for this ref, but we can narrow it based on the source,
// see https://github.com/floating-ui/react-popper/blob/beac280d61082852c4efc302be902911ce2d424c/src/Popper.js#L94
ref={mergeRefs(popperProps.ref as React.RefCallback<HTMLElement>, this.transitionContainerElement)}
ref={mergeRefs(popperProps.ref as React.RefCallback<HTMLElement>, this.transitionContainerRef)}
style={popperProps.style}
>
<ResizeSensor onResize={this.reposition}>
Expand Down Expand Up @@ -612,30 +616,27 @@ export class Popover<
return popperModifiers;
}

private handleTargetFocus = (e: React.FocusEvent<HTMLElement>) => {
private handleTargetFocus = (e: React.FocusEvent<HTMLElement, HTMLElement>) => {
if (this.props.openOnTargetFocus && this.isHoverInteractionKind()) {
if (e.relatedTarget == null && !this.lostFocusOnSamePage) {
// ignore this focus event -- the target was already focused but the page itself
// lost focus (e.g. due to switching tabs).
return;
}
this.handleMouseEnter(e as unknown as React.MouseEvent<HTMLElement>);
this.handleMouseEnter(e);
}
};

private handleTargetBlur = (e: React.FocusEvent<HTMLElement>) => {
private handleTargetBlur = (e: React.FocusEvent<HTMLElement, HTMLElement>) => {
if (this.props.openOnTargetFocus && this.isHoverInteractionKind()) {
if (e.relatedTarget != null) {
// if the next element to receive focus is within the popover, we'll want to leave the
// popover open.
if (
e.relatedTarget !== this.popoverElement &&
!this.isElementInPopover(e.relatedTarget as HTMLElement)
) {
this.handleMouseLeave(e as unknown as React.MouseEvent<HTMLElement>);
// if the next element to receive focus is within the overlay (such as the Overlay
// startFocusTrapElement), we'll want to leave the popover open.
if (!this.isElementInOverlay(e.relatedTarget)) {
this.handleMouseLeave(e);
}
} else {
this.handleMouseLeave(e as unknown as React.MouseEvent<HTMLElement>);
this.handleMouseLeave(e);
}
}
this.lostFocusOnSamePage = e.relatedTarget != null;
Expand All @@ -649,7 +650,7 @@ export class Popover<
}
};

private handleMouseEnter = (e: React.MouseEvent<HTMLElement>) => {
private handleMouseEnter = (e: React.MouseEvent<HTMLElement> | React.SyntheticEvent<HTMLElement>) => {
this.isMouseInTargetOrPopover = true;

// if we're entering the popover, and the mode is set to be HOVER_TARGET_ONLY, we want to manually
Expand All @@ -667,7 +668,7 @@ export class Popover<
}
};

private handleMouseLeave = (e: React.MouseEvent<HTMLElement>) => {
private handleMouseLeave = (e: React.MouseEvent<HTMLElement> | React.SyntheticEvent<HTMLElement>) => {
this.isMouseInTargetOrPopover = false;

// Wait until the event queue is flushed, because we want to leave the
Expand Down Expand Up @@ -784,6 +785,10 @@ export class Popover<
return this.getPopoverElement()?.contains(element) ?? false;
}

private isElementInOverlay(element: Element) {
return this.overlayRef.current?.containerElement.current?.contains(element);
}

private getIsContentEmpty() {
const { content } = this.props;
return content == null || Utils.isEmptyString(content);
Expand Down

0 comments on commit 61d22c7

Please sign in to comment.