fix(Overlay): fix flickering of tooltips and popovers during initial …

…render (#6544)
react-bootstrap · Feb 10, 2023 · 821624d · 821624d
1 parent 4bb4489
commit 821624d
Show file tree

Hide file tree

Showing 8 changed files with 110 additions and 11 deletions.
diff --git a/src/Overlay.tsx b/src/Overlay.tsx
@@ -1,5 +1,5 @@
 import * as React from 'react';
-import { useRef } from 'react';
+import { useEffect, useRef, useState } from 'react';
 import classNames from 'classnames';
 import PropTypes from 'prop-types';
 import BaseOverlay, {
@@ -8,7 +8,6 @@ import BaseOverlay, {
 } from '@restart/ui/Overlay';
 import { State } from '@restart/ui/usePopper';
 import { componentOrElement, elementType } from 'prop-types-extra';
-import useCallbackRef from '@restart/hooks/useCallbackRef';
 import useEventCallback from '@restart/hooks/useEventCallback';
 import useIsomorphicEffect from '@restart/hooks/useIsomorphicEffect';
 import useMergedRefs from '@restart/hooks/useMergedRefs';
@@ -28,6 +27,8 @@ export interface OverlayInjectedProps {
   show: boolean;
   placement: Placement | undefined;
   popper: PopperRef;
+  hasDoneInitialMeasure?: boolean;
+
   [prop: string]: any;
 }
 
@@ -162,7 +163,9 @@ const Overlay = React.forwardRef<HTMLElement, OverlayProps>(
     outerRef,
   ) => {
     const popperRef = useRef<Partial<PopperRef>>({});
-    const [firstRenderedState, setFirstRenderedState] = useCallbackRef<State>();
+    const [firstRenderedState, setFirstRenderedState] = useState<State | null>(
+      null,
+    );
     const [ref, modifiers] = useOverlayOffset(outerProps.offset);
     const mergedRef = useMergedRefs(outerRef, ref);
 
@@ -180,6 +183,12 @@ const Overlay = React.forwardRef<HTMLElement, OverlayProps>(
       }
     }, [firstRenderedState]);
 
+    useEffect(() => {
+      if (!outerProps.show) {
+        setFirstRenderedState(null);
+      }
+    }, [outerProps.show]);
+
     return (
       <BaseOverlay
         {...outerProps}
@@ -201,8 +210,11 @@ const Overlay = React.forwardRef<HTMLElement, OverlayProps>(
             placement: updatedPlacement,
             outOfBoundaries:
               popperObj?.state?.modifiersData.hide?.isReferenceHidden || false,
+            strategy: popperConfig.strategy,
           });
 
+          const hasDoneInitialMeasure = !!firstRenderedState;
+
           if (typeof overlay === 'function')
             return overlay({
               ...overlayProps,
@@ -211,13 +223,15 @@ const Overlay = React.forwardRef<HTMLElement, OverlayProps>(
               ...(!transition && show && { className: 'show' }),
               popper,
               arrowProps,
+              hasDoneInitialMeasure,
             });
 
           return React.cloneElement(overlay as React.ReactElement, {
             ...overlayProps,
             placement: updatedPlacement,
             arrowProps,
             popper,
+            hasDoneInitialMeasure,
             className: classNames(
               (overlay as React.ReactElement).props.className,
               !transition && show && 'show',

diff --git a/src/Popover.tsx b/src/Popover.tsx
@@ -7,6 +7,7 @@ import PopoverHeader from './PopoverHeader';
 import PopoverBody from './PopoverBody';
 import { Placement, PopperRef } from './types';
 import { BsPrefixProps, getOverlayDirection } from './helpers';
+import getInitialPopperStyles from './getInitialPopperStyles';
 
 export interface PopoverProps
   extends React.HTMLAttributes<HTMLDivElement>,
@@ -17,6 +18,7 @@ export interface PopoverProps
   body?: boolean;
   popper?: PopperRef;
   show?: boolean;
+  hasDoneInitialMeasure?: boolean;
 }
 
 const propTypes = {
@@ -71,6 +73,11 @@ const propTypes = {
    */
   body: PropTypes.bool,
 
+  /**
+   * Whether or not Popper has done its initial measurement and positioning.
+   */
+  hasDoneInitialMeasure: PropTypes.bool,
+
   /** @private */
   popper: PropTypes.object,
 
@@ -92,8 +99,9 @@ const Popover = React.forwardRef<HTMLDivElement, PopoverProps>(
       children,
       body,
       arrowProps,
-      popper: _,
-      show: _1,
+      hasDoneInitialMeasure,
+      popper,
+      show: _,
       ...props
     },
     ref,
@@ -103,11 +111,19 @@ const Popover = React.forwardRef<HTMLDivElement, PopoverProps>(
     const [primaryPlacement] = placement?.split('-') || [];
     const bsDirection = getOverlayDirection(primaryPlacement, isRTL);
 
+    let computedStyle = style;
+    if (!hasDoneInitialMeasure) {
+      computedStyle = {
+        ...style,
+        ...getInitialPopperStyles(popper?.strategy),
+      };
+    }
+
     return (
       <div
         ref={ref}
         role="tooltip"
-        style={style}
+        style={computedStyle}
         x-placement={primaryPlacement}
         className={classNames(
           className,

diff --git a/src/Tooltip.tsx b/src/Tooltip.tsx
@@ -5,6 +5,7 @@ import { OverlayArrowProps } from '@restart/ui/Overlay';
 import { useBootstrapPrefix, useIsRTL } from './ThemeProvider';
 import { Placement, PopperRef } from './types';
 import { BsPrefixProps, getOverlayDirection } from './helpers';
+import getInitialPopperStyles from './getInitialPopperStyles';
 
 export interface TooltipProps
   extends React.HTMLAttributes<HTMLDivElement>,
@@ -13,6 +14,7 @@ export interface TooltipProps
   arrowProps?: Partial<OverlayArrowProps>;
   show?: boolean;
   popper?: PopperRef;
+  hasDoneInitialMeasure?: boolean;
 }
 
 const propTypes = {
@@ -63,6 +65,11 @@ const propTypes = {
     style: PropTypes.object,
   }),
 
+  /**
+   * Whether or not Popper has done its initial measurement and positioning.
+   */
+  hasDoneInitialMeasure: PropTypes.bool,
+
   /** @private */
   popper: PropTypes.object,
 
@@ -83,8 +90,9 @@ const Tooltip = React.forwardRef<HTMLDivElement, TooltipProps>(
       style,
       children,
       arrowProps,
-      popper: _,
-      show: _2,
+      hasDoneInitialMeasure,
+      popper,
+      show: _,
       ...props
     }: TooltipProps,
     ref,
@@ -95,10 +103,18 @@ const Tooltip = React.forwardRef<HTMLDivElement, TooltipProps>(
     const [primaryPlacement] = placement?.split('-') || [];
     const bsDirection = getOverlayDirection(primaryPlacement, isRTL);
 
+    let computedStyle = style;
+    if (!hasDoneInitialMeasure) {
+      computedStyle = {
+        ...style,
+        ...getInitialPopperStyles(popper?.strategy),
+      };
+    }
+
     return (
       <div
         ref={ref}
-        style={style}
+        style={computedStyle}
         role="tooltip"
         x-placement={primaryPlacement}
         className={classNames(className, bsPrefix, `bs-tooltip-${bsDirection}`)}

diff --git a/src/getInitialPopperStyles.ts b/src/getInitialPopperStyles.ts
@@ -0,0 +1,11 @@
+export default function getInitialPopperStyles(
+  position: React.CSSProperties['position'] = 'absolute',
+): Partial<React.CSSProperties> {
+  return {
+    position,
+    top: '0',
+    left: '0',
+    opacity: '0',
+    pointerEvents: 'none',
+  };
+}
diff --git a/src/types.tsx b/src/types.tsx
@@ -1,5 +1,5 @@
 import PropTypes from 'prop-types';
-import { State } from '@restart/ui/usePopper';
+import { State, UsePopperOptions } from '@restart/ui/usePopper';
 
 export type Variant =
   | 'primary'
@@ -69,4 +69,5 @@ export interface PopperRef {
   outOfBoundaries: boolean;
   placement: Placement | undefined;
   scheduleUpdate?: () => void;
+  strategy: UsePopperOptions['strategy'];
 }
diff --git a/test/getInitialPopperStylesSpec.ts b/test/getInitialPopperStylesSpec.ts
@@ -0,0 +1,24 @@
+import { expect } from 'chai';
+import getInitialPopperStyles from '../src/getInitialPopperStyles';
+
+describe('getInitialPopperStyles', () => {
+  it('defaults to absolute positioning when no strategy is provided', () => {
+    expect(getInitialPopperStyles()).to.eql({
+      position: 'absolute',
+      top: '0',
+      left: '0',
+      opacity: '0',
+      pointerEvents: 'none',
+    });
+  });
+
+  it('sets the position to the provided strategy', () => {
+    expect(getInitialPopperStyles('fixed')).to.eql({
+      position: 'fixed',
+      top: '0',
+      left: '0',
+      opacity: '0',
+      pointerEvents: 'none',
+    });
+  });
+});
diff --git a/www/src/examples/Overlays/Overlay.js b/www/src/examples/Overlays/Overlay.js
@@ -12,7 +12,14 @@ function Example() {
         Click me to see
       </Button>
       <Overlay target={target.current} show={show} placement="right">
-        {({ placement, arrowProps, show: _show, popper, ...props }) => (
+        {({
+          placement: _placement,
+          arrowProps: _arrowProps,
+          show: _show,
+          popper: _popper,
+          hasDoneInitialMeasure: _hasDoneInitialMeasure,
+          ...props
+        }) => (
           <div
             {...props}
             style={{

diff --git a/www/src/pages/components/overlays.mdx b/www/src/pages/components/overlays.mdx
@@ -54,6 +54,16 @@ documentation for more details about the injected props.
 
 <ReactPlayground codeText={Overlay} />
 
+### Customizing Overlay rendering
+
+The `Overlay` injects a number of props that you can use to customize the
+rendering behavior. There is a case where you would need to show the overlay
+before `Popper` can measure and position it properly. In React-Bootstrap,
+tooltips and popovers sets the opacity and position to avoid issues where
+the initial positioning of the overlay is incorrect. See the
+[Tooltip](https://github.com/react-bootstrap/react-bootstrap/blob/master/src/Tooltip.tsx)
+implementation for an example on how this is done.
+
 ## OverlayTrigger
 
 Since the above pattern is pretty common, but verbose, we've included