From eee482f1b86a6ba097612830f808bcb2b146d297 Mon Sep 17 00:00:00 2001 From: Eric Olkowski Date: Thu, 11 Jul 2024 08:26:09 -0400 Subject: [PATCH 1/3] fix(Popper): prevented page scroll when popper opens --- packages/react-core/src/helpers/Popper/Popper.tsx | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/react-core/src/helpers/Popper/Popper.tsx b/packages/react-core/src/helpers/Popper/Popper.tsx index 43b7976a08b..26bd3b4e5e0 100644 --- a/packages/react-core/src/helpers/Popper/Popper.tsx +++ b/packages/react-core/src/helpers/Popper/Popper.tsx @@ -221,6 +221,7 @@ export const Popper: React.FunctionComponent = ({ const [popperContent, setPopperContent] = React.useState(null); const [ready, setReady] = React.useState(false); const [opacity, setOpacity] = React.useState(0); + const [display, setDisplay] = React.useState('none'); const [internalIsVisible, setInternalIsVisible] = React.useState(isVisible); const transitionTimerRef = React.useRef(null); const showTimerRef = React.useRef(null); @@ -443,6 +444,12 @@ export const Popper: React.FunctionComponent = ({ ] }); + React.useEffect(() => { + if (internalIsVisible) { + forceUpdate && forceUpdate(); + } + }, [internalIsVisible]); + /** We want to forceUpdate only when a tooltip's content is dynamically updated. * TODO: Investigate into 3rd party libraries for a less limited/specific solution */ @@ -475,6 +482,7 @@ export const Popper: React.FunctionComponent = ({ showTimerRef.current = setTimeout(() => { setInternalIsVisible(true); setOpacity(1); + setDisplay(''); onShown(); }, entryDelay); }; @@ -484,6 +492,7 @@ export const Popper: React.FunctionComponent = ({ clearTimeouts([showTimerRef]); hideTimerRef.current = setTimeout(() => { setOpacity(0); + setDisplay('none'); transitionTimerRef.current = setTimeout(() => { setInternalIsVisible(false); onHidden(); @@ -516,6 +525,7 @@ export const Popper: React.FunctionComponent = ({ ...popperStyles.popper, zIndex, opacity, + display, transition: getOpacityTransition(animationDuration) }, ...attributes.popper From 061213e8e0cbdf21aad3cdce64113f7cf84d5366 Mon Sep 17 00:00:00 2001 From: Eric Olkowski Date: Thu, 11 Jul 2024 08:38:35 -0400 Subject: [PATCH 2/3] Rebased --- packages/react-core/src/helpers/Popper/Popper.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/react-core/src/helpers/Popper/Popper.tsx b/packages/react-core/src/helpers/Popper/Popper.tsx index 26bd3b4e5e0..478436076e0 100644 --- a/packages/react-core/src/helpers/Popper/Popper.tsx +++ b/packages/react-core/src/helpers/Popper/Popper.tsx @@ -205,9 +205,9 @@ export const Popper: React.FunctionComponent = ({ flipBehavior = 'flip', triggerRef, popperRef, - animationDuration = 0, + animationDuration = 1000, entryDelay = 0, - exitDelay = 0, + exitDelay = 1000, onHidden = () => {}, onHide = () => {}, onMount = () => {}, @@ -492,9 +492,9 @@ export const Popper: React.FunctionComponent = ({ clearTimeouts([showTimerRef]); hideTimerRef.current = setTimeout(() => { setOpacity(0); - setDisplay('none'); transitionTimerRef.current = setTimeout(() => { setInternalIsVisible(false); + setDisplay('none'); onHidden(); }, animationDuration); }, exitDelay); From f5c02c918afa7ab479fc34dffe625e3490236edf Mon Sep 17 00:00:00 2001 From: Eric Olkowski Date: Thu, 11 Jul 2024 08:56:21 -0400 Subject: [PATCH 3/3] Updated failing test --- packages/react-core/src/helpers/Popper/Popper.tsx | 4 ++-- .../src/components/Select/__tests__/CheckboxSelect.test.tsx | 4 +--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/react-core/src/helpers/Popper/Popper.tsx b/packages/react-core/src/helpers/Popper/Popper.tsx index 478436076e0..c0e66241a5f 100644 --- a/packages/react-core/src/helpers/Popper/Popper.tsx +++ b/packages/react-core/src/helpers/Popper/Popper.tsx @@ -205,9 +205,9 @@ export const Popper: React.FunctionComponent = ({ flipBehavior = 'flip', triggerRef, popperRef, - animationDuration = 1000, + animationDuration = 0, entryDelay = 0, - exitDelay = 1000, + exitDelay = 0, onHidden = () => {}, onHide = () => {}, onMount = () => {}, diff --git a/packages/react-templates/src/components/Select/__tests__/CheckboxSelect.test.tsx b/packages/react-templates/src/components/Select/__tests__/CheckboxSelect.test.tsx index 1c1292af863..312b801ecc4 100644 --- a/packages/react-templates/src/components/Select/__tests__/CheckboxSelect.test.tsx +++ b/packages/react-templates/src/components/Select/__tests__/CheckboxSelect.test.tsx @@ -1,5 +1,5 @@ import * as React from 'react'; -import { render, screen, waitForElementToBeRemoved } from '@testing-library/react'; +import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { CheckboxSelect } from '../CheckboxSelect'; import styles from '@patternfly/react-styles/css/components/Badge/badge'; @@ -137,8 +137,6 @@ test('toggles the select menu when the toggle button is clicked', async () => { await user.click(toggleButton); - await waitForElementToBeRemoved(() => screen.queryByRole('menu')); - expect(screen.queryByRole('menu')).not.toBeInTheDocument(); });