Exposed close function on Popover

[AndrewMusgrave]

WHY are these changes introduced?

Fixes #12336
Fixes #12249

WHAT is this pull request doing?

This PR exposes a close function on popovers imperative handle to handle accessibility when closing the popover from an outside source.

WHY did you take this approach?

Originally I thought about using an effect to watch the active status, and automatically manage focus when the popover is closed. However, we can't guarantee that the user will always want popover to manage focus. Perhaps, they'll self manage focus to another area of the page, or navigate entirely.

Exposing a close function allows us to:

• Release this in a minor version, rather than a major one
• There's less work required to migrate as you won't need to audit all instance of popover
• Allows for accessible focus, while allowing popover to contain the business logic
• It allows for the flexibility of how you'll manage the user experience when the popover is closed

Giphy

Monosnap.screencast.2024-07-04.17-52-19.mp4

How to 🎩

Playground code

import React, {useRef, useState, useEffect} from 'react';

import type {PopoverPublicAPI} from '../src';
import {Page, Button, Popover, ActionList} from '../src';

console.log('Logging active element every 15 seconds');

export const Playground = {
  tags: ['skip-tests'],
  render() {
    useEffect(() => {
      const interval = setInterval(() => {
        console.log(document.activeElement);
      }, 15000);

      return () => {
        clearInterval(interval);
      };
    }, []);

    const popoverRef = useRef<PopoverPublicAPI>(null);
    const [popoverActive, setPopoverActive] = useState(true);

    const togglePopoverActive = () =>
      setPopoverActive((popoverActive) => !popoverActive);

    const activator = (
      <Button onClick={togglePopoverActive} disclosure>
        More actions
      </Button>
    );

    return (
      <Page title="Playground">
        <Popover
          active={popoverActive}
          activator={activator}
          autofocusTarget="first-node"
          onClose={togglePopoverActive}
          ref={popoverRef}
        >
          <ActionList
            actionRole="menuitem"
            items={[
              {
                content: 'Focus activator',
                onAction() {
                  popoverRef.current?.close();
                },
              },
              {
                content: 'Focus next node',
                onAction() {
                  popoverRef.current?.close('next-node');
                },
              },
            ]}
          />
        </Popover>
        <Button>Next focusable node</Button>
      </Page>
    );
  },
};

🎩 checklist

• Tested a snapshot
• Tested on mobile
• Tested on multiple browsers
• Tested for accessibility
• Updated the component's README.md with documentation changes
• Tophatted documentation changes in the style guide

[AndrewMusgrave]

This pattern is what's used above to test the other function in the imperative handle

[AndrewMusgrave]

@kellydanma @zaquille-oneil Before I reach out to Polaris for a review, what do you think about the approach?

[kellydanma]

✅ Approach looks good to me! Seems like a much better way to manage focus than the approach in https://github.com/Shopify/web/pull/131141; I'm going to close my PR and wait for the changes in Polaris to ship before resolving the bug. 😸

[zaquille-oneil]

LGTM

[zaquille-oneil]

/snapit

[github-actions]

🫰✨ Thanks @zaquille-oneil! Your snapshot has been published to npm.

Test the snapshot by updating your package.json with the newly published version:

[dleroux]

🐑 🇮🇹 🎩 👍

[kimhawthorne-shopify]

(Deleted comment)