From d1c9a716bd6a80c48bedd64b76d6d9dc31f0c70f Mon Sep 17 00:00:00 2001
From: Kyle Durand <kyle.durand@shopify.com>
Date: Wed, 27 Mar 2024 09:24:33 -0400
Subject: [PATCH] manage focus manually

---
 polaris-react/playground/DetailsPage.tsx      | 22 +-----
 .../components/Combobox/Combobox.stories.tsx  |  1 -
 .../src/components/Picker/Picker.tsx          | 29 ++++----
 .../Picker/components/Activator/Activator.tsx | 71 ++++++++++---------
 .../Activator/tests/Activator.test.tsx        |  3 +-
 .../components/SearchField/SearchField.tsx    | 50 ++++++-------
 .../components/Picker/tests/Picker.test.tsx   |  9 ++-
 7 files changed, 85 insertions(+), 100 deletions(-)

diff --git a/polaris-react/playground/DetailsPage.tsx b/polaris-react/playground/DetailsPage.tsx
index 1b414f8b25d..409a2fba949 100644
--- a/polaris-react/playground/DetailsPage.tsx
+++ b/polaris-react/playground/DetailsPage.tsx
@@ -45,8 +45,6 @@ import {
   FooterHelp,
   Link,
   AlphaPicker,
-  Box,
-  Popover,
 } from '../src';
 import type {DropZoneProps, PageProps} from '../src';
 
@@ -644,7 +642,6 @@ export function DetailsPage() {
           </LegacyCard>
         </Layout.Section>
         <Layout.Section variant="oneThird">
-          <Box minHeight="75vmin" />
           <LegacyCard title="Organization">
             <LegacyCard.Section>
               <Select
@@ -654,22 +651,15 @@ export function DetailsPage() {
                 value={selected}
               />
               <br />
-              <Select
-                label="Vendor"
-                options={options}
-                onChange={setSelected}
-                value={selected}
-              />
-              <br />
               <AlphaPicker
                 onSelect={handleSelect}
                 activator={{
                   label: 'Vendor',
-                  placeholder: 'Search vendors',
+                  placeholder: 'None selected',
                 }}
                 searchField={{
                   label: 'Search vendors',
-                  placeholder: 'Search vendors',
+                  placeholder: 'Search or add new vendor',
                   autoComplete: 'off',
                   value: query,
                   onChange: (value) => setQuery(value),
@@ -680,14 +670,6 @@ export function DetailsPage() {
                   children: `Add ${query}`,
                 }}
               />
-              <button>yohooo</button>
-              <Popover
-                active
-                activator={<button>click me</button>}
-                onClose={() => {}}
-              >
-                <p>hello</p>
-              </Popover>
             </LegacyCard.Section>
             <LegacyCard.Section title="Collections" />
             <LegacyCard.Section title="Tags" />
diff --git a/polaris-react/src/components/Combobox/Combobox.stories.tsx b/polaris-react/src/components/Combobox/Combobox.stories.tsx
index c46f7925624..4bbd0c0bb30 100644
--- a/polaris-react/src/components/Combobox/Combobox.stories.tsx
+++ b/polaris-react/src/components/Combobox/Combobox.stories.tsx
@@ -98,7 +98,6 @@ export function Default() {
           <Listbox onSelect={updateSelection}>{optionsMarkup}</Listbox>
         ) : null}
       </Combobox>
-      <button>test</button>
     </div>
   );
 }
diff --git a/polaris-react/src/components/Picker/Picker.tsx b/polaris-react/src/components/Picker/Picker.tsx
index ec79ec3bf82..8f6981af1fa 100644
--- a/polaris-react/src/components/Picker/Picker.tsx
+++ b/polaris-react/src/components/Picker/Picker.tsx
@@ -58,21 +58,23 @@ export function Picker({
   onClose,
   ...listboxProps
 }: PickerProps) {
-  const [query, setQuery] = useState<string>('');
-  const [filteredOptions, setFilteredOptions] = useState<OptionProps[] | null>(
-    options,
-  );
+  const activatorRef = React.createRef<HTMLButtonElement>();
+  const [activeItem, setActiveItem] = useState<string>();
   const [popoverActive, setPopoverActive] = useState(false);
   const [activeOptionId, setActiveOptionId] = useState<string>();
-  const [activeItem, setActiveItem] = useState<string>();
   const [textFieldLabelId, setTextFieldLabelId] = useState<string>();
   const [listboxId, setListboxId] = useState<string>();
-  const shouldOpen = !popoverActive;
+  const [query, setQuery] = useState<string>('');
+  const [filteredOptions, setFilteredOptions] = useState<OptionProps[] | null>(
+    options,
+  );
 
+  const shouldOpen = !popoverActive;
   const handleClose = useCallback(() => {
     setPopoverActive(false);
     onClose?.();
-  }, [onClose]);
+    activatorRef.current?.focus();
+  }, [activatorRef, onClose]);
 
   const handleOpen = useCallback(() => {
     setPopoverActive(true);
@@ -164,27 +166,26 @@ export function Picker({
     options.find((option) => option.value === activeItem)?.children,
   );
 
-  const firstSelectedLabel = firstSelectedOption
-    ? firstSelectedOption?.toString()
-    : activator.placeholder;
-
   const queryMatchesExistingOption = options.some((option) =>
     QUERY_REGEX(query).exec(reactChildrenText(option.children)),
   );
 
   return (
     <Popover
-      active={popoverActive}
       activator={
         <Activator
           {...activator}
           onClick={handleOpen}
-          placeholder={firstSelectedLabel}
+          selected={firstSelectedOption || ''}
+          placeholder={activator.placeholder}
+          ref={activatorRef}
         />
       }
+      active={popoverActive}
       autofocusTarget="none"
-      preferredPosition="cover"
       onClose={handleClose}
+      preferredPosition="cover"
+      preventFocusOnClose
     >
       <Popover.Pane onScrolledToBottom={onScrolledToBottom} height={height}>
         {searchField ? (
diff --git a/polaris-react/src/components/Picker/components/Activator/Activator.tsx b/polaris-react/src/components/Picker/components/Activator/Activator.tsx
index 3a29ab500ec..ecafab7cd83 100644
--- a/polaris-react/src/components/Picker/components/Activator/Activator.tsx
+++ b/polaris-react/src/components/Picker/components/Activator/Activator.tsx
@@ -1,49 +1,54 @@
 import {SelectIcon} from '@shopify/polaris-icons';
-import React from 'react';
+import React, {forwardRef} from 'react';
 
 import {BlockStack} from '../../../BlockStack';
 import {Icon} from '../../../Icon';
 import {Text} from '../../../Text';
-import {UnstyledButton} from '../../../UnstyledButton';
 import {classNames} from '../../../../utilities/css';
 
 import styles from './Activator.module.css';
 
 export interface ActivatorProps {
+  disabled?: boolean;
   label?: string;
   placeholder?: string;
-  disabled?: boolean;
+  selected?: string;
   onClick?(): void;
 }
 
-export function Activator({
-  disabled,
-  label,
-  placeholder,
-  onClick,
-}: ActivatorProps) {
-  return (
-    <UnstyledButton
-      className={classNames(styles.Activator, disabled && styles.disabled)}
-      disabled={disabled}
-      onClick={onClick}
-    >
-      <BlockStack as="span" gap="100">
-        {label && (
-          <Text as="span" variant="bodySm" alignment="start" tone="subdued">
-            {label}
-          </Text>
-        )}
+export const Activator = forwardRef<HTMLButtonElement, ActivatorProps>(
+  ({disabled, label, placeholder, selected, onClick}, ref) => {
+    return (
+      <button
+        ref={ref}
+        disabled={disabled}
+        onClick={onClick}
+        className={classNames(styles.Activator, disabled && styles.disabled)}
+      >
+        <BlockStack as="span" gap="100">
+          {label && (
+            <Text as="span" variant="bodySm" alignment="start" tone="subdued">
+              {label}
+            </Text>
+          )}
 
-        {placeholder && (
-          <Text as="span" variant="bodyMd" alignment="start">
-            {placeholder}
-          </Text>
-        )}
-      </BlockStack>
-      <span>
-        <Icon tone="subdued" source={SelectIcon} />
-      </span>
-    </UnstyledButton>
-  );
-}
+          {(selected !== '' || placeholder) && (
+            <Text
+              as="span"
+              variant="bodyMd"
+              alignment="start"
+              tone={selected ? undefined : 'subdued'}
+            >
+              {selected || placeholder}
+            </Text>
+          )}
+        </BlockStack>
+        <span>
+          <Icon tone="subdued" source={SelectIcon} />
+        </span>
+      </button>
+    );
+  },
+);
+
+Activator.displayName = 'Activator';
diff --git a/polaris-react/src/components/Picker/components/Activator/tests/Activator.test.tsx b/polaris-react/src/components/Picker/components/Activator/tests/Activator.test.tsx
index 4455e55f1e0..8da2160efea 100644
--- a/polaris-react/src/components/Picker/components/Activator/tests/Activator.test.tsx
+++ b/polaris-react/src/components/Picker/components/Activator/tests/Activator.test.tsx
@@ -2,7 +2,6 @@ import React from 'react';
 import {mountWithApp} from 'tests/utilities';
 
 import {Activator} from '../Activator';
-import {UnstyledButton} from '../../../../UnstyledButton';
 
 describe('<Activator />', () => {
   it('renders a label', () => {
@@ -20,6 +19,6 @@ describe('<Activator />', () => {
   it('renders a disabled activator', () => {
     const activator = mountWithApp(<Activator disabled />);
 
-    expect(activator).toContainReactComponent(UnstyledButton, {disabled: true});
+    expect(activator).toContainReactComponent('button', {disabled: true});
   });
 });
diff --git a/polaris-react/src/components/Picker/components/SearchField/SearchField.tsx b/polaris-react/src/components/Picker/components/SearchField/SearchField.tsx
index 4082a8fbdb4..e0b7ee4fe32 100644
--- a/polaris-react/src/components/Picker/components/SearchField/SearchField.tsx
+++ b/polaris-react/src/components/Picker/components/SearchField/SearchField.tsx
@@ -4,6 +4,7 @@ import {Label, labelID} from '../../../Label';
 import type {TextFieldProps} from '../../../TextField';
 import {useComboboxTextField} from '../../../../utilities/combobox';
 import {InlineStack} from '../../../InlineStack';
+import {Text} from '../../../Text';
 
 import styles from './SearchField.module.css';
 
@@ -15,7 +16,6 @@ export function SearchField({
   onBlur,
   onChange,
   label,
-  labelHidden,
   prefix,
   placeholder,
   focused,
@@ -76,30 +76,30 @@ export function SearchField({
   }
 
   return (
-    <>
-      <Label id={textFieldId} hidden={labelHidden}>
-        {label}
-      </Label>
-      <InlineStack gap="100" blockAlign="center">
+    <InlineStack gap="100" blockAlign="center">
+      <Label id={textFieldId}>
+        <Text as="span" visuallyHidden>
+          {label}
+        </Text>
         <span>{prefix}</span>
-        <input
-          ref={inputRef}
-          className={styles.SearchField}
-          value={value}
-          id={textFieldId}
-          type={type}
-          aria-activedescendant={activeOptionId}
-          role="combobox"
-          aria-haspopup="listbox"
-          aria-autocomplete="list"
-          aria-expanded="true"
-          placeholder={placeholder}
-          aria-controls={listboxId}
-          onFocus={handleFocus}
-          onBlur={handleBlur}
-          onChange={({target}) => handleChange(target.value, textFieldId)}
-        />
-      </InlineStack>
-    </>
+      </Label>
+      <input
+        ref={inputRef}
+        id={textFieldId}
+        className={styles.SearchField}
+        value={value}
+        type={type}
+        aria-activedescendant={activeOptionId}
+        role="combobox"
+        aria-haspopup="listbox"
+        aria-autocomplete="list"
+        aria-expanded="true"
+        placeholder={placeholder}
+        aria-controls={listboxId}
+        onFocus={handleFocus}
+        onBlur={handleBlur}
+        onChange={({target}) => handleChange(target.value, textFieldId)}
+      />
+    </InlineStack>
   );
 }
diff --git a/polaris-react/src/components/Picker/tests/Picker.test.tsx b/polaris-react/src/components/Picker/tests/Picker.test.tsx
index a25e8671b79..21403990284 100644
--- a/polaris-react/src/components/Picker/tests/Picker.test.tsx
+++ b/polaris-react/src/components/Picker/tests/Picker.test.tsx
@@ -3,7 +3,6 @@ import {mountWithApp} from 'tests/utilities';
 
 import {Picker} from '../Picker';
 import {Activator, SearchField} from '../components';
-import {UnstyledButton} from '../../UnstyledButton';
 import {Listbox} from '../../Listbox';
 
 describe('<Picker />', () => {
@@ -26,7 +25,7 @@ describe('<Picker />', () => {
       />,
     );
 
-    picker.find(UnstyledButton)!.trigger('onClick');
+    picker.find('button')!.trigger('onClick');
 
     expect(picker).toContainReactComponent(SearchField);
   });
@@ -39,7 +38,7 @@ describe('<Picker />', () => {
 
     const picker = mountWithApp(<Picker activator={{}} options={options} />);
 
-    picker.find(UnstyledButton)!.trigger('onClick');
+    picker.find('button')!.trigger('onClick');
 
     expect(picker).toContainReactComponent(Listbox);
     expect(picker).toContainReactComponentTimes(Listbox.Option, options.length);
@@ -56,7 +55,7 @@ describe('<Picker />', () => {
       />,
     );
 
-    picker.find(UnstyledButton)!.trigger('onClick');
+    picker.find('button')!.trigger('onClick');
     picker.find(SearchField)!.trigger('onChange', 'Add');
 
     expect(picker).toContainReactComponent(Listbox.Action);
@@ -76,7 +75,7 @@ describe('<Picker />', () => {
       />,
     );
 
-    picker.find(UnstyledButton)!.trigger('onClick');
+    picker.find('button')!.trigger('onClick');
     expect(picker).toContainReactComponentTimes(Listbox.Option, options.length);
 
     picker.find(SearchField)!.trigger('onChange', 'One');