fix: consider margins in Popper calculations (#5112)

* fix: consider margins in Popper calculations

* WIP

* fix entire problem by removing margins

* remove extra popper stuff, fix for other elements

* fix test

src/DropdownMenu.js

@@ -6,6 +6,7 @@ import useMergedRefs from '@restart/hooks/useMergedRefs';
 import NavbarContext from './NavbarContext';
 import { useBootstrapPrefix } from './ThemeProvider';
 import useWrappedRefWithWarning from './useWrappedRefWithWarning';
+import usePopperMarginModifiers from './usePopperMarginModifiers';
 
 const propTypes = {
   /**
@@ -64,17 +65,19 @@ const DropdownMenu = React.forwardRef(
       alignRight,
       rootCloseEvent,
       flip,
-      popperConfig,
       show: showProps,
       renderOnMount,
       // Need to define the default "as" during prop destructuring to be compatible with styled-components github.com/react-bootstrap/react-bootstrap/issues/3595
       as: Component = 'div',
+      popperConfig = {},
       ...props
     },
     ref,
   ) => {
     const isNavbar = useContext(NavbarContext);
     const prefix = useBootstrapPrefix(bsPrefix, 'dropdown-menu');
+    const [popperRef, marginModifiers] = usePopperMarginModifiers();
+
     const {
       hasShown,
       placement,
@@ -84,16 +87,22 @@ const DropdownMenu = React.forwardRef(
       props: menuProps,
     } = useDropdownMenu({
       flip,
-      popperConfig,
       rootCloseEvent,
       show: showProps,
       alignEnd: alignRight,
       usePopper: !isNavbar,
+      popperConfig: {
+        ...popperConfig,
+        modifiers: marginModifiers.concat(popperConfig.modifiers || []),
+      },
     });
 
     menuProps.ref = useMergedRefs(
-      menuProps.ref,
-      useWrappedRefWithWarning(ref, 'DropdownMenu'),
+      popperRef,
+      useMergedRefs(
+        useWrappedRefWithWarning(ref, 'DropdownMenu'),
+        menuProps.ref,
+      ),
     );
 
     if (!hasShown && !renderOnMount) return null;

src/Overlay.js

@@ -5,6 +5,8 @@ import BaseOverlay from 'react-overlays/Overlay';
 import safeFindDOMNode from 'react-overlays/safeFindDOMNode';
 import { componentOrElement, elementType } from 'prop-types-extra';
 
+import usePopperMarginModifiers from './usePopperMarginModifiers';
+
 import Fade from './Fade';
 
 const propTypes = {
@@ -120,12 +122,27 @@ function wrapRefs(props, arrowProps) {
     aRef.__wrapped || (aRef.__wrapped = (r) => aRef(safeFindDOMNode(r)));
 }
 
-function Overlay({ children: overlay, transition, ...outerProps }) {
+function Overlay({
+  children: overlay,
+  transition,
+  popperConfig = {},
+  ...outerProps
+}) {
   const popperRef = useRef({});
+  const [ref, marginModifiers] = usePopperMarginModifiers();
+
   transition = transition === true ? Fade : transition || null;
 
   return (
-    <BaseOverlay {...outerProps} transition={transition}>
+    <BaseOverlay
+      {...outerProps}
+      ref={ref}
+      popperConfig={{
+        ...popperConfig,
+        modifiers: marginModifiers.concat(popperConfig.modifiers || []),
+      }}
+      transition={transition}
+    >
       {({
         props: overlayProps,
         arrowProps,
@@ -164,7 +181,10 @@ function Overlay({ children: overlay, transition, ...outerProps }) {
             overlay.props.className,
             !transition && show && 'show',
           ),
-          style: { ...overlay.props.style, ...overlayProps.style },
+          style: {
+            ...overlay.props.style,
+            ...overlayProps.style,
+          },
         });
       }}
     </BaseOverlay>

src/Popover.js

@@ -85,7 +85,12 @@ const Popover = React.forwardRef(
         )}
         {...props}
       >
-        <div className="arrow" {...arrowProps} />
+        <div
+          className="arrow"
+          {...arrowProps}
+          // this prevents an error if you render a Popover without arrow props, like in a test
+          style={arrowProps ? { ...arrowProps.style, margin: 0 } : undefined}
+        />
         {content ? <PopoverContent>{children}</PopoverContent> : children}
       </div>
     );

src/usePopperMarginModifiers.js

@@ -0,0 +1,57 @@
+import { useCallback, useRef, useMemo } from 'react';
+import hasClass from 'dom-helpers/hasClass';
+
+function getMargins(element) {
+  const styles = getComputedStyle(element);
+
+  const top = parseFloat(styles.marginTop) || 0;
+  const right = parseFloat(styles.marginRight) || 0;
+  const bottom = parseFloat(styles.marginBottom) || 0;
+  const left = parseFloat(styles.marginLeft) || 0;
+  return { top, right, bottom, left };
+}
+
+export default function usePopperMarginModifiers() {
+  const overlayRef = useRef(null);
+  const margins = useRef(null);
+
+  return [
+    useCallback((overlay) => {
+      if (
+        !overlay ||
+        !(hasClass(overlay, 'popover') || hasClass(overlay, 'dropdown-menu'))
+      )
+        return;
+
+      margins.current = getMargins(overlay);
+      overlay.style.margin = 0;
+      overlayRef.current = overlay;
+    }, []),
+    [
+      useMemo(() => {
+        return {
+          name: 'offset',
+          options: {
+            offset: ({ placement }) => {
+              if (!margins.current) return [0, 0];
+              const { top, left, bottom, right } = margins.current;
+
+              switch (placement.split('-')[0]) {
+                case 'top':
+                  return [0, bottom];
+                case 'left':
+                  return [0, right];
+                case 'bottom':
+                  return [0, top];
+                case 'right':
+                  return [0, left];
+                default:
+                  return [0, 0];
+              }
+            },
+          },
+        };
+      }, [margins]),
+    ],
+  ];
+}