fix(Popover): use Bootstrap default offsets (react-bootstrap#5663)

* fix(Popover): use Bootstrap default offsets

* Add comment about useOverlayOffset
Add offset constant into popover
Move overlay types into another types.tsx to prevent circular dependency warning

Author: kyletsang

src/Overlay.tsx

@@ -6,16 +6,10 @@ import BaseOverlay, {
 } from 'react-overlays/Overlay';
 import safeFindDOMNode from 'react-overlays/safeFindDOMNode';
 import { componentOrElement, elementType } from 'prop-types-extra';
-import usePopperMarginModifiers from './usePopperMarginModifiers';
+import useOverlayOffset from './useOverlayOffset';
 import Fade from './Fade';
 import { TransitionType } from './helpers';
-
-export type Placement = import('react-overlays/usePopper').Placement;
-
-export type ArrowProps = {
-  ref: React.RefCallback<HTMLElement>;
-  style: React.CSSProperties;
-};
+import { ArrowProps, Placement } from './types';
 
 export interface OverlayInjectedProps {
   ref: React.RefCallback<HTMLElement>;
@@ -166,7 +160,7 @@ function Overlay({
   ...outerProps
 }: OverlayProps) {
   const popperRef = useRef({});
-  const [ref, marginModifiers] = usePopperMarginModifiers();
+  const [ref, modifiers] = useOverlayOffset();
 
   const actualTransition = transition === true ? Fade : transition || null;
 
@@ -176,7 +170,7 @@ function Overlay({
       ref={ref}
       popperConfig={{
         ...popperConfig,
-        modifiers: marginModifiers.concat(popperConfig.modifiers || []),
+        modifiers: modifiers.concat(popperConfig.modifiers || []),
       }}
       transition={actualTransition as any}
     >

src/Popover.tsx

@@ -5,7 +5,7 @@ import isRequiredForA11y from 'prop-types-extra/lib/isRequiredForA11y';
 import { useBootstrapPrefix } from './ThemeProvider';
 import PopoverTitle from './PopoverTitle';
 import PopoverBody from './PopoverBody';
-import { ArrowProps, Placement } from './Overlay';
+import { ArrowProps, Placement } from './types';
 import { BsPrefixProps } from './helpers';
 
 export interface PopoverProps
@@ -136,4 +136,8 @@ Popover.defaultProps = defaultProps;
 export default Object.assign(Popover, {
   Title: PopoverTitle,
   Body: PopoverBody,
+
+  // Default popover offset.
+  // https://github.com/twbs/bootstrap/blob/5c32767e0e0dbac2d934bcdee03719a65d3f1187/js/src/popover.js#L28
+  POPPER_OFFSET: [0, 8] as const,
 });

src/Tooltip.tsx

@@ -3,8 +3,7 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import isRequiredForA11y from 'prop-types-extra/lib/isRequiredForA11y';
 import { useBootstrapPrefix } from './ThemeProvider';
-
-import { ArrowProps, Placement } from './Overlay';
+import { ArrowProps, Placement } from './types';
 import { BsPrefixProps } from './helpers';
 
 export interface TooltipProps

src/types.tsx

@@ -30,3 +30,10 @@ export type Color =
   | 'light'
   | 'white'
   | 'muted';
+
+export type Placement = import('react-overlays/usePopper').Placement;
+
+export type ArrowProps = {
+  ref: React.RefCallback<HTMLElement>;
+  style: React.CSSProperties;
+};

src/useOverlayOffset.tsx

@@ -0,0 +1,35 @@
+import { useMemo, useRef } from 'react';
+import hasClass from 'dom-helpers/hasClass';
+import { Options } from 'react-overlays/usePopper';
+import { useBootstrapPrefix } from './ThemeProvider';
+import Popover from './Popover';
+
+// This is meant for internal use.
+// This applies a custom offset to the overlay if it's a popover.
+export default function useOverlayOffset(): [
+  React.RefObject<HTMLElement>,
+  Options['modifiers'],
+] {
+  const overlayRef = useRef<HTMLDivElement | null>(null);
+  const popoverClass = useBootstrapPrefix(undefined, 'popover');
+
+  const offset = useMemo(
+    () => ({
+      name: 'offset',
+      options: {
+        offset: () => {
+          if (
+            overlayRef.current &&
+            hasClass(overlayRef.current, popoverClass)
+          ) {
+            return Popover.POPPER_OFFSET;
+          }
+          return [0, 0];
+        },
+      },
+    }),
+    [popoverClass],
+  );
+
+  return [overlayRef, [offset]];
+}

test/useOverlayOffsetSpec.js

@@ -0,0 +1,59 @@
+import React, { useImperativeHandle } from 'react';
+import { mount } from 'enzyme';
+
+import Popover from '../src/Popover';
+import Tooltip from '../src/Tooltip';
+import useOverlayOffset from '../src/useOverlayOffset';
+
+describe('useOverlayOffset', () => {
+  const Wrapper = React.forwardRef((props, outerRef) => {
+    const [ref, modifiers] = useOverlayOffset();
+
+    useImperativeHandle(outerRef, () => ({
+      modifiers,
+    }));
+
+    return React.cloneElement(props.children, {
+      ref,
+    });
+  });
+
+  it('should have offset of [0, 8] for Popovers', () => {
+    const ref = React.createRef();
+
+    mount(
+      <Wrapper ref={ref}>
+        <Popover id="test-popover" />
+      </Wrapper>,
+    );
+
+    const offset = ref.current.modifiers[0].options.offset();
+    expect(offset).to.eql([0, 8]);
+  });
+
+  it('should have offset of [0, 0] for Tooltips', () => {
+    const ref = React.createRef();
+
+    mount(
+      <Wrapper ref={ref}>
+        <Tooltip id="test-tooltip" />
+      </Wrapper>,
+    );
+
+    const offset = ref.current.modifiers[0].options.offset();
+    expect(offset).to.eql([0, 0]);
+  });
+
+  it('should have offset of [0, 0] for any overlay', () => {
+    const ref = React.createRef();
+
+    mount(
+      <Wrapper ref={ref}>
+        <div>test</div>
+      </Wrapper>,
+    );
+
+    const offset = ref.current.modifiers[0].options.offset();
+    expect(offset).to.eql([0, 0]);
+  });
+});