fix(Popover): add some logic to Popover to manage focus on blur

src/components/Popover/LazyTippy.tsx

@@ -6,7 +6,6 @@ import Tippy, { TippyProps } from '@tippyjs/react';
 export type LazyTippyProps = TippyProps & {
   setInstance?: (instance?: TippyInstance) => void;
   continuePropagationOnTrigger?: boolean;
-  hasRelatedTarget?: boolean;
 };
 
 /**

src/components/Popover/Popover.stories.tsx

@@ -490,15 +490,65 @@ Common.parameters = {
 };
 
 const WithMeetingListItemWithAvatarWithPopover = Template<PopoverProps>((args) => {
+  return (
+    <Popover
+      {...args}
+      triggerComponent={
+        <MeetingListItem style={{ margin: '10rem auto', display: 'flex' }}>
+          <Popover
+            {...args}
+            triggerComponent={
+              <Avatar
+                // eslint-disable-next-line
+                onPress={() => {}}
+                initials="AB"
+              >
+                Hover or click me!
+              </Avatar>
+            }
+          >
+            <div>
+              <ButtonPill>test 1</ButtonPill>
+              <ButtonPill>test 2</ButtonPill>
+              <ButtonPill>test 3</ButtonPill>
+            </div>
+          </Popover>
+          test
+        </MeetingListItem>
+      }
+      trigger="click"
+      interactive
+    >
+      <div>
+        <ButtonPill>test 4</ButtonPill>
+        <ButtonPill>test 5</ButtonPill>
+        <ButtonPill>test 6</ButtonPill>
+      </div>
+    </Popover>
+  );
+}).bind({});
+
+WithMeetingListItemWithAvatarWithPopover.argTypes = { ...argTypes };
+
+WithMeetingListItemWithAvatarWithPopover.args = {
+  trigger: 'mouseenter',
+  placement: PLACEMENTS.TOP,
+  showArrow: true,
+  interactive: true,
+  appendTo: () => document.querySelector('#theme-provider'),
+};
+
+const WithHideOnBlur = Template<PopoverProps>((args) => {
   return (
     <>
-      <div id="outer">
+      <div id="container-1">
         <Popover
           {...args}
           triggerComponent={
             <ButtonPill>test 1</ButtonPill>
           }
           trigger="click"
+          focusBackOnTrigger
           interactive
           hideOnBlur
           disableFocusLock
@@ -510,13 +560,14 @@ const WithMeetingListItemWithAvatarWithPopover = Template<PopoverProps>((args) =
           </List>
         </Popover>
       </div>
-      <div id="other">
+      <div id="container-2">
         <Popover
           {...args}
           triggerComponent={
             <ButtonPill>test 2</ButtonPill>
           }
           trigger="click"
+          focusBackOnTrigger
           interactive
           hideOnBlur
           disableFocusLock
@@ -533,15 +584,6 @@ const WithMeetingListItemWithAvatarWithPopover = Template<PopoverProps>((args) =
 
 }).bind({});
 
-WithMeetingListItemWithAvatarWithPopover.argTypes = { ...argTypes };
-
-WithMeetingListItemWithAvatarWithPopover.args = {
-  trigger: 'mouseenter',
-  placement: PLACEMENTS.TOP,
-  showArrow: true,
-  interactive: true,
-};
-
 export {
   Example,
   InteractiveContent,
@@ -558,4 +600,5 @@ export {
   WithSearchInput,
   WithMeetingListItemWithButtonsWithPopoverInList,
   WithMeetingListItemWithAvatarWithPopover,
-};
+  WithHideOnBlur
+};

src/components/Popover/Popover.tsx

@@ -16,8 +16,6 @@ import { addTippyPlugins } from './Popover.utils';
 import { v4 as uuidV4 } from 'uuid';
 import { PopoverInstance } from '.';
 
-type CustomInstance = PopoverInstance & { props: Props } & {hasRelatedTarget?: boolean;};
-
 /**
  * The Popover component allows adding a Popover to whatever provided
  * `triggerComponent`. It will show the Popover after a specific event, which is
@@ -86,7 +84,7 @@ const Popover = forwardRef((props: Props, ref: ForwardedRef<HTMLElement>) => {
       ? DEFAULTS.FOCUS_BACK_ON_TRIGGER_COMPONENT_INTERACTIVE
       : DEFAULTS.FOCUS_BACK_ON_TRIGGER_COMPONENT_NON_INTERACTIVE);
 
-  const popoverInstance = React.useRef<CustomInstance>(undefined);
+  const popoverInstance = React.useRef<PopoverInstance>(undefined);
 
   const generatedTriggerIdRef = useRef(uuidV4());
   const generatedTriggerId = generatedTriggerIdRef.current;
@@ -107,11 +105,16 @@ const Popover = forwardRef((props: Props, ref: ForwardedRef<HTMLElement>) => {
   const arrowId = React.useMemo(() => `${ARROW_ID}${uuidV4()}`, []);
 
   const popoverSetInstance = useCallback(
-    (instance?: CustomInstance) => {
+    (instance?: PopoverInstance) => {
+      if (instance) {
+        // initialize to whatever prop value is
+        // from now on, will be controlled by hideOnBlur plugin
+        instance.shouldFocusTrigger = focusBackOnTrigger;
+      }
       popoverInstance.current = instance;
       setInstance?.(instance);
     },
-    [setInstance]
+    [setInstance, focusBackOnTrigger]
   );
 
   const handleOnCloseButtonClick = useCallback(() => {
@@ -121,12 +124,15 @@ const Popover = forwardRef((props: Props, ref: ForwardedRef<HTMLElement>) => {
   // needs special handling since FocusScope doesn't work with the Popover from Tippy
   // needs to focus back to the reference item when the popover is completely hidden
   const handleOnPopoverHidden = useCallback(() => {
-    if (focusBackOnTrigger && !popoverInstance?.current?.hasRelatedTarget) {
+    // When the popover hides, the focus goes to the next focusable element by default. Except if focusBackOnTrigger popover prop is true AND shouldFocusTrigger (determined by hideOnBlurPlugin) is true.
+    // shouldFocusTrigger is true when the focusout event has no relatedTarget, that is, focusOut was triggered by Esc or Click.
+
+    if (focusBackOnTrigger && popoverInstance?.current?.shouldFocusTrigger) {
       // eslint-disable-next-line @typescript-eslint/ban-ts-comment
       // @ts-ignore
       popoverInstance.current?.reference?.focus();
     }
-  }, [focusBackOnTrigger, popoverInstance?.current?.hasRelatedTarget]);
+  }, [focusBackOnTrigger, popoverInstance?.current?.shouldFocusTrigger]);
 
   useEffect(() => {
     firstFocusElement?.focus();
@@ -251,7 +257,6 @@ const Popover = forwardRef((props: Props, ref: ForwardedRef<HTMLElement>) => {
       }}
       setInstance={popoverSetInstance}
       zIndex={zIndex}
-      hasRelatedTarget={false}
     >
       {clonedTriggerComponent}
     </LazyTippy>

src/components/Popover/Popover.types.ts

@@ -14,10 +14,12 @@ export type TriggerType = TippyProps['trigger'];
 export type PositioningStrategy = TippyProps['popperOptions']['strategy'];
 export type AppendToType = TippyProps['appendTo'];
 
+export type CustomTippyPopoverProperties = { shouldFocusTrigger?: boolean };
+
 /**
  * Popover instance interface abstracted from Tippy.js
  */
-export type PopoverInstance = Instance;
+export type PopoverInstance = Instance & CustomTippyPopoverProperties;
 
 export type PopoverCommonStyleProps = {
   /**
@@ -242,6 +244,4 @@ export interface Props extends PopoverCommonStyleProps, Partial<LifecycleHooks>
    * @default `false`
    */
   disableFocusLock?: boolean;
-
-  hasRelatedTarget?: boolean;
 }

src/components/Popover/Popover.unit.test.tsx

@@ -2,7 +2,6 @@ import React, { FC, useCallback, useState } from 'react';
 import { render, screen, waitFor } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import '@testing-library/jest-dom';
-
 import ButtonSimple from '../ButtonSimple';
 
 import Popover from './';
@@ -1194,6 +1193,65 @@ describe('<Popover />', () => {
         });
       });
 
+      it('should not focus back on the trigger element when popover is hidden and shouldFocusTrigger is false', async () => {
+        const user = userEvent.setup();
+
+        render(
+          <>
+            <div>
+              <Popover
+                ref={ref}
+                triggerComponent={<button>Click Me!</button>}
+                interactive
+                hideOnBlur
+                disableFocusLock
+                focusBackOnTrigger
+                setInstance={(instance: PopoverInstance) => {
+                  if (instance) {
+                    instance.shouldFocusTrigger = false;
+                  }
+                }}
+              >
+              <button>content</button>
+              </Popover>
+            </div>
+            <div>
+              <button>another button</button>
+            </div>
+          </>
+        );
+
+        checkRef(screen.getByRole('button', { name: 'Click Me!' }));
+
+        // // assert no popover on screen
+        const contentBeforeClick = screen.queryByText('Content');
+        expect(contentBeforeClick).not.toBeInTheDocument();
+
+        // // after click, popover should be shown
+        await openPopoverByClickingOnTriggerAndCheckContent(user);
+
+
+        // goes to listitem
+        await user.keyboard('{Tab}');
+
+        // tabs out of popover
+        await user.keyboard('{Tab}');
+
+        // Wait for the popover to be hidden
+        await waitFor(() => {
+          expect(screen.queryByText('Content')).not.toBeInTheDocument();
+        });
+
+        // Check if the focus is not back on the trigger element
+        const triggerElement = screen.getByRole('button', { name: /click me!/i });
+        expect(document.activeElement).not.toEqual(triggerElement);
+
+        // Check if the focus is on the next focusable element
+        const anotherButton = screen.getByRole('button', { name: /another button/i });
+        expect(document.activeElement).toEqual(anotherButton);
+      });
+
+
       it('it should focus on the trigger component when focusBackOnTrigger= = true and popover gets closed', async () => {
         expect.assertions(6);
         const user = userEvent.setup();

src/components/Popover/tippy-plugins/hideOnBlurPlugin.test.ts

@@ -54,6 +54,7 @@ describe('hideOnBlurPlugin', () => {
     });
 
     popoverInstance.popper.dispatchEvent(focusOutEvent);
+    expect(popoverInstance.shouldFocusTrigger).toEqual(false);
 
     expect(popoverInstance.hide).toHaveBeenCalled();
   });
@@ -70,6 +71,7 @@ describe('hideOnBlurPlugin', () => {
     });
 
     popoverInstance.popper.dispatchEvent(focusOutEvent);
+    expect(popoverInstance.shouldFocusTrigger).toEqual(false);
 
     expect(popoverInstance.hide).not.toHaveBeenCalled();
   });
@@ -88,11 +90,12 @@ describe('hideOnBlurPlugin', () => {
     });
 
     popoverInstance.popper.dispatchEvent(focusOutEvent);
+    expect(popoverInstance.shouldFocusTrigger).toEqual(false);
 
     expect(popoverInstance.hide).not.toHaveBeenCalled();
   });
 
-  it('should not hide popover if relatedTarget is null', async () => {
+  it('should hide popover if relatedTarget is null', async () => {
     const { hideOnBlurPlugin } = await import('./hideOnBlurPlugin');
     const popoverInstance = createPopoverInstance();
     const plugin = hideOnBlurPlugin.fn(popoverInstance);
@@ -103,7 +106,8 @@ describe('hideOnBlurPlugin', () => {
     });
 
     popoverInstance.popper.dispatchEvent(focusOutEvent);
+    expect(popoverInstance.shouldFocusTrigger).toEqual(true);
 
-    expect(popoverInstance.hide).not.toHaveBeenCalled();
+    expect(popoverInstance.hide).toHaveBeenCalled();
   });
 });

src/components/Popover/tippy-plugins/hideOnBlurPlugin.ts

@@ -6,20 +6,16 @@ export interface PopperBlurPluginProps extends Props {
   isChildPopoverOpen?: boolean;
 }
 
-type CustomInstance = PopoverInstance & { props: PopperBlurPluginProps } & {
-  hasRelatedTarget?: boolean;
-};
-
 export const hideOnBlurPlugin: Plugin = {
   name: 'isChildPopoverOpen',
   defaultValue: false,
-  fn(instance: CustomInstance) {
+  fn(instance: PopoverInstance & { props: PopperBlurPluginProps }) {
     const focusOutHandler = (event) => {
-      instance.hasRelatedTarget = !!event.relatedTarget;
+      // if it doesn't have a related target (ie: Esc, or click, should focus back on trigger)
+      instance.shouldFocusTrigger = !event.relatedTarget;
 
       if (
         !instance.props.isChildPopoverOpen &&
-        event.relatedTarget &&
         !instance.popper.contains(event.relatedTarget as Element)
       ) {
         instance.hide();

src/components/Popover/tippy-plugins/hideOnEscPlugin.ts

@@ -7,7 +7,6 @@ const openedTippyInstances: TippyInstance[] = [];
 // hide the last opened popover when Escape key is pressed
 function onKeyDown(event: KeyboardEvent) {
   if (event.key === 'Escape' && openedTippyInstances.length !== 0) {
-    // RS_TODO: i think the better thing to do would be set custom attri on instance here
     const lastIdx = openedTippyInstances.length - 1;
     openedTippyInstances[lastIdx].hide();
   }