Skip to content

Commit

Permalink
fix: Popper Timeout Tracking (#1417)
Browse files Browse the repository at this point in the history
We noticed that Popper was sometimes closing the dropdown inaccurately.
This was pinned down to timeouts being incorrectly cleared, and therefore
sometimes running when they ought not to. Careful examination of the
code revealed that they weren't being tracked very well, and thus stomped on
or lost.

This approach leverages a ref to track the value to avoid loss during
re-renders, and to also avoid unnecessary re-renders.

* chore: Initial tidying pass on Popper
* Add a few missing types
* Let the code breathe a bit
* fix: Better Popper Timeout Tracking

---------

Co-authored-by: Haider Alshamma <[email protected]>
  • Loading branch information
jherdman and haideralsh committed Jun 21, 2024
1 parent a1d511b commit e026714
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 18 deletions.
15 changes: 15 additions & 0 deletions biome.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"formatter": {
"enabled": true,
"indentStyle": "space",
"lineWidth": 120
},
"linter": {
"enabled": false
},
"javascript": {
"formatter": {
"trailingComma": "es5"
}
}
}
57 changes: 39 additions & 18 deletions src/Popper/Popper.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
// @ts-nocheck
import React, { useState, useEffect } from "react";
import React, { useState, useEffect, useRef } from "react";
import { Manager, Reference, Popper as ReactPopperPopUp } from "react-popper";
import { useTranslation } from "react-i18next";
import { PopperArrow } from "../utils";
import { keyCodes } from "../constants";
const makeArray = (children) => {
if (!Array.isArray(children)) {
return [children];
Expand Down Expand Up @@ -49,42 +48,56 @@ const Popper: React.FC<React.PropsWithChildren<PopperProps>> = React.forwardRef(
},
popperRef
) => {
let timeoutID;
// We're going to manage the ID of the timeout in a ref so that we can examine
// it without causing a re-render. Note that "0" will denote "no jobs running",
// whereas positive values are the ID of the running job.
const timeoutId = useRef(0);

const resetTimeoutId = () => {
clearTimeout(timeoutId.current);
timeoutId.current = 0;
};

const [isOpen, setIsOpen] = useState(defaultOpen);
const conditionallyApplyDelay = (fnc, delay, skipDelay = true) => {

const conditionallyApplyDelay = (fnc: () => void, delay: number, skipDelay = true) => {
if (!skipDelay) {
timeoutID = setTimeout(fnc, delay);
timeoutId.current = setTimeout(fnc, delay);
} else {
fnc();
}
};
const setPopUpState = (nextIsOpenState, skipDelay) => {
clearTimeout(timeoutID);

const setPopUpState = (nextIsOpenState: boolean, skipDelay: boolean) => {
resetTimeoutId();
conditionallyApplyDelay(() => setIsOpen(nextIsOpenState), nextIsOpenState ? showDelay : hideDelay, skipDelay);
};
const closePopUp = (skipDelay) => {

const closePopUp = (skipDelay: boolean) => {
setPopUpState(false, skipDelay);
};

useEffect(() => {
const handleKeyDown = (event) => {
switch (event.keyCode) {
case keyCodes.ESC:
closePopUp();
break;
default:
break;
function handleKeyDown(event: KeyboardEvent) {
if (event.code === "Escape") {
closePopUp();
}
};
}

document.addEventListener("keydown", handleKeyDown);

const cleanup = () => {
document.removeEventListener("keydown", handleKeyDown);
clearTimeout(timeoutID);
resetTimeoutId();
};

return cleanup;
}, []);
const openPopUp = (skipDelay) => {

const openPopUp = (skipDelay: boolean) => {
setPopUpState(true, skipDelay);
};

const onClickEventHandlers = openOnClick
? {
onClick: () => {
Expand All @@ -96,12 +109,14 @@ const Popper: React.FC<React.PropsWithChildren<PopperProps>> = React.forwardRef(
},
}
: null;

const onHoverHandlers = openOnHover
? {
onMouseEnter: () => openPopUp(false),
onMouseLeave: () => closePopUp(false),
}
: null;

const eventHandlers = {
onFocus: () => openPopUp(false),
onBlur: () => {
Expand All @@ -110,6 +125,7 @@ const Popper: React.FC<React.PropsWithChildren<PopperProps>> = React.forwardRef(
...onHoverHandlers,
...onClickEventHandlers,
};

const transformInnerChildren = (elements) =>
makeArray(elements).map((element, i) => {
const transformedElement = wrapInFunction(element)({
Expand All @@ -127,13 +143,18 @@ const Popper: React.FC<React.PropsWithChildren<PopperProps>> = React.forwardRef(
key: i,
});
});

const renderInnerChildren = () => {
const innerChildren = children.props.children;
return typeof innerChildren !== "string" ? transformInnerChildren(innerChildren) : innerChildren;
};

const { t } = useTranslation();

const openLabel = openAriaLabel || t("open");

const closeLabel = closeAriaLabel || t("close");

return (
<Manager ref={popperRef}>
<Reference>
Expand Down

0 comments on commit e026714

Please sign in to comment.