[LG-5008] fix(popover): improve positioning logic with enhanced fallback behavior and improve guide cue alignment (#3226)

* chore: clean up GuideCue and Tooltip stories

* fix(guide-cue): beacon positioning

* fix(popover): fallback position, consider adjustOnMutation, and hide if reference hidden

* fix(popover): rm hide middleware for now

* chore: changeset

* test(popover): add story showing adjustOnMutation behavior and update docs

* refactor(popover): revert consideration of adjustOnMutation prop

* fix(date-picker): flaky spec

* refactor(guide-cue): rename TooltipContent to GuideCueTooltip

Author: stephl3

.changeset/fix-popover-positioning.md

@@ -0,0 +1,9 @@
+---
+'@leafygreen-ui/guide-cue': minor
+'@leafygreen-ui/popover': minor
+---
+
+[LG-5008](https://jira.mongodb.org/browse/LG-5008)
+
+- Improves positioning logic with enhanced positioning fallback to optimize popover element visibility
+- Assigns `GuideCue` component's internal beacon ref for better positioning of tooltip instances relative to beacon

packages/date-picker/src/DatePicker/DatePicker.keyboard2.spec.tsx

@@ -1,4 +1,8 @@
-import { waitFor, waitForElementToBeRemoved } from '@testing-library/react';
+import {
+  act,
+  waitFor,
+  waitForElementToBeRemoved,
+} from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 
 import { Month, newUTC } from '@leafygreen-ui/date-utils';
@@ -183,30 +187,25 @@ describe('DatePicker keyboard interaction', () => {
     });
 
     test('does not close the main menu if a select menu is open', async () => {
-      const { openMenu, queryAllByRole, findAllByRole } = renderDatePicker();
+      const { openMenu, findAllByRole } = renderDatePicker();
       const { monthSelect, menuContainerEl } = await openMenu();
 
       tabNTimes(3);
       expect(monthSelect).toHaveFocus();
 
       userEvent.keyboard('[Enter]');
-      await waitFor(() => {
-        jest.advanceTimersByTime(transitionDuration.default);
-      });
+      act(() => jest.advanceTimersByTime(transitionDuration.default));
 
       const options = await findAllByRole('option');
       const firstOption = options[0];
       userEvent.keyboard('{arrowdown}');
       expect(firstOption).toHaveFocus();
 
-      const listBoxes = queryAllByRole('listbox');
-      expect(listBoxes).toHaveLength(2);
-
-      const selectMenu = listBoxes[1];
       userEvent.keyboard('{escape}');
-      await waitForElementToBeRemoved(selectMenu);
-      expect(menuContainerEl).toBeInTheDocument();
-      expect(monthSelect).toHaveFocus();
+      act(() => jest.advanceTimersByTime(transitionDuration.default));
+
+      expect(menuContainerEl).toBeVisible();
+      await waitFor(() => expect(monthSelect).toHaveFocus());
     });
   });
 

packages/guide-cue/src/GuideCue.stories.tsx

@@ -108,13 +108,23 @@ const meta: StoryMetaType<any> = {
     buttonText: {
       control: 'text',
     },
+    tooltipAlign: {
+      control: { type: 'radio' },
+      options: Object.values(TooltipAlign),
+    },
+    tooltipJustify: {
+      control: { type: 'radio' },
+      options: Object.values(TooltipJustify),
+    },
   },
   args: {
     title: 'New feature',
     children: 'This is a new feature. You should try it out',
     buttonText: 'Next',
     numberOfSteps: 4,
     currentStep: 2,
+    tooltipAlign: TooltipAlign.Top,
+    tooltipJustify: TooltipJustify.Middle,
   },
 };
 

packages/guide-cue/src/GuideCue/GuideCue.tsx

@@ -10,7 +10,7 @@ import Popover, {
   RenderMode,
 } from '@leafygreen-ui/popover';
 
-import TooltipContent from '../TooltipContent';
+import GuideCueTooltip from '../GuideCueTooltip';
 
 import { beaconStyles, timeout1, timeout2 } from './GuideCue.styles';
 import { GuideCueProps, TooltipAlign, TooltipJustify } from './GuideCue.types';
@@ -140,37 +140,36 @@ function GuideCue({
       {isStandalone ? (
         // Standalone tooltip
         // this is using the reference from the `refEl` prop to position itself against
-        <TooltipContent {...tooltipContentProps}>{children}</TooltipContent>
+        <GuideCueTooltip {...tooltipContentProps}>{children}</GuideCueTooltip>
       ) : (
         // Multistep tooltip
-        <Popover
-          active={popoverOpen}
-          refEl={refEl}
-          align={beaconAlign}
-          justify={TooltipJustify.Middle}
-          spacing={-12} // width of beacon is 24px, 24/2 = 12
-          adjustOnMutation={true}
-          dismissMode={DismissMode.Manual}
-          renderMode={RenderMode.TopLayer}
-        >
-          {/* The beacon is using the popover component to position itself */}
-          <div
+        <>
+          <Popover
+            active={popoverOpen}
+            refEl={refEl}
+            align={beaconAlign}
+            justify={TooltipJustify.Middle}
+            spacing={-12} // width of beacon is 24px, 24/2 = 12
+            adjustOnMutation={true}
+            dismissMode={DismissMode.Manual}
+            renderMode={RenderMode.TopLayer}
             ref={beaconRef}
-            className={beaconStyles(prefersReducedMotion, darkMode)}
           >
-            <div />
-          </div>
-
+            {/* The beacon is using the popover component to position itself */}
+            <div className={beaconStyles(prefersReducedMotion, darkMode)}>
+              <div />
+            </div>
+          </Popover>
           {/* The tooltip is using the ref of the beacon as the trigger to position itself against */}
           {/* Instead of passing the beacon as the tooltip trigger prop we pass a reference to the beacon to the `refEl` prop. By passing only the reference we avoid default tooltip behaviors such as closing the tooltip on background click or showing and hiding the tooltip on hover. */}
-          <TooltipContent
+          <GuideCueTooltip
             {...tooltipContentProps}
             refEl={beaconRef}
             open={tooltipOpen}
           >
             {children}
-          </TooltipContent>
-        </Popover>
+          </GuideCueTooltip>
+        </>
       )}
     </>
   );

packages/guide-cue/src/TooltipContent/TooltipContent.styles.ts renamed to packages/guide-cue/src/GuideCueTooltip/GuideCueTooltip.styles.ts

@@ -24,7 +24,7 @@ import {
   stepStyles,
   tooltipMultistepStyles,
   tooltipStyles,
-} from './TooltipContent.styles';
+} from './GuideCueTooltip.styles';
 
 const ariaLabelledby = 'guide-cue-label';
 const ariaDescribedby = 'guide-cue-desc';
@@ -47,7 +47,7 @@ const focusTrapOptions: Options = {
   },
 };
 
-type TooltipContentProps = Partial<GuideCueProps> & {
+type GuideCueTooltipProps = Partial<GuideCueProps> & {
   theme: Theme;
   title: string;
   isStandalone: boolean;
@@ -57,7 +57,7 @@ type TooltipContentProps = Partial<GuideCueProps> & {
   handleCloseClick: () => void;
 };
 
-function TooltipContent({
+function GuideCueTooltip({
   darkMode,
   open,
   setOpen,
@@ -76,7 +76,7 @@ function TooltipContent({
   handleButtonClick,
   handleCloseClick,
   ...tooltipProps
-}: TooltipContentProps) {
+}: GuideCueTooltipProps) {
   const focusId = useIdAllocator({ prefix: 'guide-cue' });
 
   return (
@@ -151,5 +151,5 @@ function TooltipContent({
   );
 }
 
-TooltipContent.displayName = 'TooltipContent';
-export default TooltipContent;
+GuideCueTooltip.displayName = 'GuideCueTooltip';
+export default GuideCueTooltip;

packages/guide-cue/src/TooltipContent/TooltipContent.tsx renamed to packages/guide-cue/src/GuideCueTooltip/GuideCueTooltip.tsx

@@ -0,0 +1 @@
+export { default } from './GuideCueTooltip';

packages/guide-cue/src/GuideCueTooltip/index.ts

@@ -8,7 +8,7 @@ import { StoryFn, StoryObj } from '@storybook/react';
 import Button from '@leafygreen-ui/button';
 import { css } from '@leafygreen-ui/emotion';
 import { palette } from '@leafygreen-ui/palette';
-import { color } from '@leafygreen-ui/tokens';
+import { color, spacing } from '@leafygreen-ui/tokens';
 
 import { getPopoverRenderModeProps } from './utils/getPopoverRenderModeProps';
 import {
@@ -21,10 +21,10 @@ import {
   ToggleEvent,
 } from './Popover';
 
-const popoverStyle = css`
+const popoverStyles = css`
   border: 1px solid ${palette.gray.light1};
   text-align: center;
-  padding: 12px;
+  padding: ${spacing[300]}px;
   max-height: 100%;
   overflow: hidden;
   // Reset these properties since they'll be inherited
@@ -91,7 +91,7 @@ const meta: StoryMetaType<typeof Popover> = {
       },
       args: {
         active: true,
-        children: <div className={popoverStyle}>Popover content</div>,
+        children: <div className={popoverStyles}>Popover content</div>,
       },
 
       decorator: Instance => {
@@ -211,7 +211,7 @@ export const LiveExample: StoryFn<PopoverStoryProps> = ({
         {buttonText}
       </Button>
       <Popover {...popoverProps}>
-        <div className={popoverStyle}>Popover content</div>
+        <div className={popoverStyles}>Popover content</div>
       </Popover>
     </div>
   );
@@ -244,7 +244,7 @@ const PortalPopoverInScrollableContainer = ({
             portalRef={portalRef}
             scrollContainer={scrollContainer.current}
           >
-            <div className={popoverStyle}>Popover content</div>
+            <div className={popoverStyles}>Popover content</div>
           </Popover>
         </Button>
       </div>
@@ -295,7 +295,7 @@ const InlinePopover = ({ buttonText, ...props }: PopoverStoryProps) => {
         refEl={buttonRef}
         renderMode={RenderMode.Inline}
       >
-        <div className={popoverStyle}>Popover content</div>
+        <div className={popoverStyles}>Popover content</div>
       </Popover>
     </div>
   );

packages/guide-cue/src/TooltipContent/index.ts

@@ -134,11 +134,14 @@ export const Popover = forwardRef<HTMLDivElement, PopoverComponentProps>(
         ),
         flip({
           boundary: scrollContainer ?? 'clippingAncestors',
+          mainAxis: true,
+          crossAxis: true,
+          fallbackAxisSideDirection: 'start',
         }),
       ],
       open: active,
       placement: getFloatingPlacement(align, justify),
-      strategy: renderMode === RenderMode.TopLayer ? 'fixed' : 'absolute',
+      strategy: 'absolute',
       transform: false,
       whileElementsMounted: autoUpdate,
     });