@react-aria/overlays - Fix overlay position clamping (#3756)

* Fix overlay position clamping

* Add tests and update test data

Co-authored-by: Robert Snow <[email protected]>
Co-authored-by: Daniel Lu <[email protected]>

Author: 3 people

packages/@react-aria/overlays/src/calculatePosition.ts

@@ -11,6 +11,7 @@
  */
 
 import {Axis, Placement, PlacementAxis, SizeAxis} from '@react-types/overlays';
+import {clamp} from '@react-aria/utils';
 
 interface Position {
   top?: number,
@@ -202,13 +203,16 @@ function computePosition(
   // add the crossOffset from props
   position[crossAxis] += crossOffset;
 
-  // this is button center position - the overlay size + half of the button to align bottom of overlay with button center
-  let minViablePosition = childOffset[crossAxis] + (childOffset[crossSize] / 2) - overlaySize[crossSize];
-  // this is button position of center, aligns top of overlay with button center
-  let maxViablePosition = childOffset[crossAxis] + (childOffset[crossSize] / 2);
+  // button bottom touching overlay bottom
+  let bottomsTouchPosition = childOffset[crossAxis] + childOffset[crossSize] - overlaySize[crossSize];
+  // button top touching overlay top
+  let topsTouchPosition = childOffset[crossAxis];
 
-  // clamp it into the range of the min/max positions
-  position[crossAxis] = Math.min(Math.max(minViablePosition, position[crossAxis]), maxViablePosition);
+  // we can't assume which one is bigger because button could be bigger than overlay and vice versa
+  const minPosition = Math.min(bottomsTouchPosition, topsTouchPosition);
+  const maxPosition = Math.max(bottomsTouchPosition, topsTouchPosition);
+
+  position[crossAxis] = clamp(position[crossAxis], minPosition, maxPosition);
 
   // Floor these so the position isn't placed on a partial pixel, only whole pixels. Shouldn't matter if it was floored or ceiled, so chose one.
   if (placement === axis) {

packages/@react-aria/overlays/stories/UseOverlayPosition.stories.tsx

@@ -21,9 +21,10 @@ import {useOverlayTriggerState} from '@react-stately/overlays';
 function Trigger(props: {
   withPortal: boolean,
   placement: Placement,
-  maxHeight?: number
+  maxHeight?: number,
+  buttonWidth?: number
 }) {
-  const {withPortal, placement, maxHeight} = props;
+  const {withPortal, placement, maxHeight, buttonWidth} = props;
   const targetRef = React.useRef<HTMLButtonElement>(null);
   const overlayRef = React.useRef<HTMLDivElement>(null);
   const state = useOverlayTriggerState({
@@ -71,7 +72,13 @@ function Trigger(props: {
   }
   return (
     <div style={{position: 'relative', margin: 'auto'}}>
-      <button ref={targetRef} {...triggerProps} onClick={() => state.toggle()}>Trigger (open: {`${state.isOpen}`})</button>
+      <button
+        ref={targetRef}
+        {...triggerProps}
+        style={{width: buttonWidth}}
+        onClick={() => state.toggle()}>
+        Trigger (open: {`${state.isOpen}`})
+      </button>
       {state.isOpen && overlay}
     </div>
   );
@@ -82,5 +89,6 @@ storiesOf('UseOverlayPosition', module)
   .add('document.body container top', () => <Trigger withPortal placement="top" />)
   .add('positioned container bottom', () => <Trigger withPortal={false} placement="bottom" />)
   .add('positioned container top', () => <Trigger withPortal={false} placement="top" />)
+  .add('buttonWidth=500 document.body bottom start', () => <Trigger withPortal buttonWidth={500} placement="bottom start" />)
   .add('maxHeight=200 container bottom', () => <Trigger withPortal maxHeight={200} placement="bottom" />)
   .add('maxHeight=200 container top', () => <Trigger withPortal maxHeight={200} placement="top" />);

packages/@react-aria/overlays/test/calculatePosition.test.js

@@ -194,95 +194,107 @@ describe('calculatePosition', function () {
       noOffset: [50, 200, undefined, 100, 350],
       offsetBefore: [-200, 50, undefined, 0, 500],
       offsetAfter: [300, 350, undefined, 200, 200],
-      crossAxisOffset: [50, 210, undefined, 90, 340],
+      crossAxisOffsetPositive: [50, 210, undefined, 90, 340],
+      crossAxisOffsetNegative: [50, 190, undefined, 110, 360],
       mainAxisOffset: [40, 200, undefined, 100, 350]
     },
     {
       placement: 'left top',
       noOffset: [50, 250, undefined, 50, 300],
       offsetBefore: [-200, 50, undefined, 0, 500],
       offsetAfter: [300, 350, undefined, 200, 200],
-      crossAxisOffset: [50, 260, undefined, 40, 290],
+      crossAxisOffsetPositive: [50, 250, undefined, 50, 300],
+      crossAxisOffsetNegative: [50, 240, undefined, 60, 310],
       mainAxisOffset: [40, 250, undefined, 50, 300]
     },
     {
       placement: 'left bottom',
       noOffset: [50, 150, undefined, 150, 400],
       offsetBefore: [-200, 50, undefined, 0, 500],
       offsetAfter: [300, 350, undefined, 200, 200],
-      crossAxisOffset: [50, 160, undefined, 140, 390],
+      crossAxisOffsetPositive: [50, 160, undefined, 140, 390],
+      crossAxisOffsetNegative: [50, 150, undefined, 150, 400],
       mainAxisOffset: [40, 150, undefined, 150, 400]
     },
     {
       placement: 'top',
       noOffset: [200, 50, 100, undefined, 200],
       offsetBefore: [50, -200, 0, undefined, 0],
       offsetAfter: [350, 300, 200, undefined, 450],
-      mainAxisOffset: [200, 40, 100, undefined, 200],
-      crossAxisOffset: [210, 50, 90, undefined, 200]
+      crossAxisOffsetPositive: [210, 50, 90, undefined, 200],
+      crossAxisOffsetNegative: [190, 50, 110, undefined, 200],
+      mainAxisOffset: [200, 40, 100, undefined, 200]
     },
     {
       placement: 'top left',
       noOffset: [250, 50, 50, undefined, 200],
       offsetBefore: [50, -200, 0, undefined, 0],
       offsetAfter: [350, 300, 200, undefined, 450],
-      mainAxisOffset: [250, 40, 50, undefined, 200],
-      crossAxisOffset: [260, 50, 40, undefined, 200]
+      crossAxisOffsetPositive: [250, 50, 50, undefined, 200],
+      crossAxisOffsetNegative: [240, 50, 60, undefined, 200],
+      mainAxisOffset: [250, 40, 50, undefined, 200]
     },
     {
       placement: 'top right',
       noOffset: [150, 50, 150, undefined, 200],
       offsetBefore: [50, -200, 0, undefined, 0],
       offsetAfter: [350, 300, 200, undefined, 450],
-      mainAxisOffset: [150, 40, 150, undefined, 200],
-      crossAxisOffset: [160, 50, 140, undefined, 200]
+      crossAxisOffsetPositive: [160, 50, 140, undefined, 200],
+      crossAxisOffsetNegative: [150, 50, 150, undefined, 200],
+      mainAxisOffset: [150, 40, 150, undefined, 200]
     },
     {
       placement: 'bottom',
       noOffset: [200, 350, 100, undefined, 200],
       offsetBefore: [50, 100, 0, undefined, 450],
       offsetAfter: [350, 600, 200, undefined, 0],
-      mainAxisOffset: [200, 360, 100, undefined, 190],
-      crossAxisOffset: [210, 350, 90, undefined, 200]
+      crossAxisOffsetPositive: [210, 350, 90, undefined, 200],
+      crossAxisOffsetNegative: [190, 350, 110, undefined, 200],
+      mainAxisOffset: [200, 360, 100, undefined, 190]
     },
     {
       placement: 'bottom left',
       noOffset: [250, 350, 50, undefined, 200],
       offsetBefore: [50, 100, 0, undefined, 450],
       offsetAfter: [350, 600, 200, undefined, 0],
-      mainAxisOffset: [250, 360, 50, undefined, 190],
-      crossAxisOffset: [260, 350, 40, undefined, 200]
+      crossAxisOffsetPositive: [250, 350, 50, undefined, 200],
+      crossAxisOffsetNegative: [240, 350, 60, undefined, 200],
+      mainAxisOffset: [250, 360, 50, undefined, 190]
     },
     {
       placement: 'bottom right',
       noOffset: [150, 350, 150, undefined, 200],
       offsetBefore: [50, 100, 0, undefined, 450],
       offsetAfter: [350, 600, 200, undefined, 0],
-      mainAxisOffset: [150, 360, 150, undefined, 190],
-      crossAxisOffset: [160, 350, 140, undefined, 200]
+      crossAxisOffsetPositive: [160, 350, 140, undefined, 200],
+      crossAxisOffsetNegative: [150, 350, 150, undefined, 200],
+      mainAxisOffset: [150, 360, 150, undefined, 190]
     },
     {
       placement: 'right',
       noOffset: [350, 200, undefined, 100, 350],
       offsetBefore: [100, 50, undefined, 0, 500],
       offsetAfter: [600, 350, undefined, 200, 200],
-      crossAxisOffset: [350, 210, undefined, 90, 340],
+      crossAxisOffsetPositive: [350, 210, undefined, 90, 340],
+      crossAxisOffsetNegative: [350, 190, undefined, 110, 360],
       mainAxisOffset: [360, 200, undefined, 100, 350]
     },
     {
       placement: 'right top',
       noOffset: [350, 250, undefined, 50, 300],
       offsetBefore: [100, 50, undefined, 0, 500],
       offsetAfter: [600, 350, undefined, 200, 200],
-      crossAxisOffset: [350, 260, undefined, 40, 290],
+      crossAxisOffsetPositive: [350, 250, undefined, 50, 300],
+      crossAxisOffsetNegative: [350, 240, undefined, 60, 310],
       mainAxisOffset: [360, 250, undefined, 50, 300]
     },
     {
       placement: 'right bottom',
       noOffset: [350, 150, undefined, 150, 400],
       offsetBefore: [100, 50, undefined, 0, 500],
       offsetAfter: [600, 350, undefined, 200, 200],
-      crossAxisOffset: [350, 160, undefined, 140, 390],
+      crossAxisOffsetPositive: [350, 160, undefined, 140, 390],
+      crossAxisOffsetNegative: [350, 150, undefined, 150, 400],
       mainAxisOffset: [360, 150, undefined, 150, 400]
     }
   ];
@@ -321,9 +333,15 @@ describe('calculatePosition', function () {
         );
       });
 
-      describe('cross axis offset', function () {
+      describe('cross axis offset positive', function () {
         checkPosition(
-          placement, getTargetDimension({left: 250, top: 250}), testCase.crossAxisOffset, 0, 10
+          placement, getTargetDimension({left: 250, top: 250}), testCase.crossAxisOffsetPositive, 0, 10
+        );
+      });
+
+      describe('cross axis offset negative', function () {
+        checkPosition(
+          placement, getTargetDimension({left: 250, top: 250}), testCase.crossAxisOffsetNegative, 0, -10
         );
       });
     });