[Picker] Refresh options on options change (#11890)

Currently when you add a new option it won't show up until the next render cycle, i.e. when the list is closed and re-opened. This fixes that issue but requires options to be memoized.
Shopify · Apr 18, 2024 · b36ba8a · b36ba8a
1 parent 5b9f4b8
commit b36ba8a
Show file tree

Hide file tree

Showing 2 changed files with 106 additions and 37 deletions.
diff --git a/polaris-react/playground/DetailsPage.tsx b/polaris-react/playground/DetailsPage.tsx
@@ -1,4 +1,4 @@
-import React, {useCallback, useRef, useState} from 'react';
+import React, {useMemo, useCallback, useRef, useState} from 'react';
 import {
   ChartVerticalIcon,
   AppsIcon,
@@ -76,6 +76,80 @@ export function DetailsPage() {
   const [emailFieldValue, setEmailFieldValue] = useState(
     defaultState.current.emailFieldValue,
   );
+  const [addedTags, setAddedTags] = useState<
+    {value: string; children: string}[]
+  >([]);
+  const [addedVendors, setAddedVendors] = useState<
+    {value: string; children: string}[]
+  >([]);
+
+  const tags = useMemo(
+    () => [
+      {value: 'Outdoors', children: 'Outdoors'},
+      {value: 'Adventure', children: 'Adventure'},
+      {value: 'Hiking', children: 'Hiking'},
+      {value: 'Camping', children: 'Camping'},
+      {value: 'Backpacking', children: 'Backpacking'},
+      {value: 'Mountaineering', children: 'Mountaineering'},
+      {value: 'Skiing', children: 'Skiing'},
+      {value: 'Snowboarding', children: 'Snowboarding'},
+      ...addedTags,
+    ],
+    [addedTags],
+  );
+
+  const vendors = useMemo(
+    () => [
+      {value: 'The North Face', children: 'The North Face'},
+      {value: 'Patagonia', children: 'Patagonia'},
+      {value: 'Arc’teryx', children: 'Arc’teryx'},
+      {value: 'Marmot', children: 'Marmot'},
+      {value: 'Black Diamond', children: 'Black Diamond'},
+      {value: 'Mountain Hardwear', children: 'Mountain Hardwear'},
+      {value: 'Columbia', children: 'Columbia'},
+      {value: 'Canada Goose', children: 'Canada Goose'},
+      {value: 'Merrell', children: 'Merrell'},
+      {value: 'Salomon', children: 'Salomon'},
+      {value: 'Burton', children: 'Burton'},
+      ...addedVendors,
+    ],
+    [addedVendors],
+  );
+
+  const handleTagSelect = useCallback(
+    (newTag: string) => {
+      if (
+        tags.some((tag) => tag.value === newTag) ||
+        addedTags.some((tag) => tag.value === newTag)
+      ) {
+        return;
+      }
+      setAddedTags((addedTags) => [
+        ...addedTags,
+        {value: newTag, children: newTag},
+      ]);
+      setQuery('');
+    },
+    [addedTags, tags],
+  );
+
+  const handleVendorSelect = useCallback(
+    (newVendor: string) => {
+      if (
+        vendors.some((vendor) => vendor.value === newVendor) ||
+        addedVendors.some((vendor) => vendor.value === newVendor)
+      ) {
+        return;
+      }
+      setAddedVendors((addedVendors) => [
+        ...addedVendors,
+        {value: newVendor, children: newVendor},
+      ]);
+      setQuery('');
+    },
+    [addedVendors, vendors],
+  );
+
   const [storeName, setStoreName] = useState(
     defaultState.current.nameFieldValue,
   );
@@ -651,16 +725,8 @@ export function DetailsPage() {
                     value: query,
                     onChange: (value) => setQuery(value),
                   }}
-                  options={[
-                    {value: 'Outdoors', children: 'Outdoors'},
-                    {value: 'Adventure', children: 'Adventure'},
-                    {value: 'Hiking', children: 'Hiking'},
-                    {value: 'Camping', children: 'Camping'},
-                    {value: 'Backpacking', children: 'Backpacking'},
-                    {value: 'Mountaineering', children: 'Mountaineering'},
-                    {value: 'Skiing', children: 'Skiing'},
-                    {value: 'Snowboarding', children: 'Snowboarding'},
-                  ]}
+                  options={tags}
+                  onSelect={handleTagSelect}
                   addAction={{
                     value: query,
                     children: `Add ${query}`,
@@ -678,19 +744,8 @@ export function DetailsPage() {
                     value: query,
                     onChange: (value) => setQuery(value),
                   }}
-                  options={[
-                    {value: 'The North Face', children: 'The North Face'},
-                    {value: 'Patagonia', children: 'Patagonia'},
-                    {value: 'Arc’teryx', children: 'Arc’teryx'},
-                    {value: 'Marmot', children: 'Marmot'},
-                    {value: 'Black Diamond', children: 'Black Diamond'},
-                    {value: 'Mountain Hardwear', children: 'Mountain Hardwear'},
-                    {value: 'Columbia', children: 'Columbia'},
-                    {value: 'Canada Goose', children: 'Canada Goose'},
-                    {value: 'Merrell', children: 'Merrell'},
-                    {value: 'Salomon', children: 'Salomon'},
-                    {value: 'Burton', children: 'Burton'},
-                  ]}
+                  options={vendors}
+                  onSelect={handleVendorSelect}
                   addAction={{
                     value: query,
                     children: `Add ${query}`,

diff --git a/polaris-react/src/components/Picker/Picker.tsx b/polaris-react/src/components/Picker/Picker.tsx
@@ -1,4 +1,11 @@
-import React, {useState, useMemo, useCallback, isValidElement} from 'react';
+import React, {
+  createRef,
+  isValidElement,
+  useCallback,
+  useMemo,
+  useRef,
+  useState,
+} from 'react';
 import {SearchIcon} from '@shopify/polaris-icons';
 
 import {Popover} from '../Popover';
@@ -59,18 +66,16 @@ export function Picker({
   onClose,
   ...listboxProps
 }: PickerProps) {
-  const activatorRef = React.createRef<HTMLButtonElement>();
+  const activatorRef = createRef<HTMLButtonElement>();
   const [activeItems, setActiveItems] = useState<string[]>([]);
   const [popoverActive, setPopoverActive] = useState(false);
   const [activeOptionId, setActiveOptionId] = useState<string>();
   const [textFieldLabelId, setTextFieldLabelId] = useState<string>();
   const [listboxId, setListboxId] = useState<string>();
   const [query, setQuery] = useState<string>('');
-  const [filteredOptions, setFilteredOptions] = useState<OptionProps[] | null>(
-    options,
-  );
-
+  const filteredOptions = useRef(options);
   const shouldOpen = !popoverActive;
+
   const handleClose = useCallback(() => {
     setPopoverActive(false);
     onClose?.();
@@ -79,7 +84,8 @@ export function Picker({
 
   const handleOpen = useCallback(() => {
     setPopoverActive(true);
-  }, []);
+    filteredOptions.current = options;
+  }, [options]);
 
   const handleFocus = useCallback(() => {
     if (shouldOpen) handleOpen();
@@ -93,9 +99,8 @@ export function Picker({
     if (popoverActive) {
       handleClose();
       setQuery('');
-      setFilteredOptions(options);
     }
-  }, [popoverActive, handleClose, options]);
+  }, [popoverActive, handleClose]);
 
   const textFieldContextValue: ComboboxTextFieldType = useMemo(
     () => ({
@@ -151,14 +156,14 @@ export function Picker({
       setQuery(value);
 
       if (value === '') {
-        setFilteredOptions(options);
+        filteredOptions.current = options;
         return;
       }
 
       const resultOptions = options?.filter((option) =>
         FILTER_REGEX(value).exec(reactChildrenText(option.children)),
       );
-      setFilteredOptions(resultOptions ?? []);
+      filteredOptions.current = resultOptions ?? [];
     },
     [options],
   );
@@ -169,6 +174,15 @@ export function Picker({
       updateText('');
       listboxProps.onSelect?.(selected);
 
+      if (
+        !filteredOptions.current.some((option) => option.value === selected)
+      ) {
+        filteredOptions.current = [
+          ...filteredOptions.current,
+          {value: selected, children: selected},
+        ];
+      }
+
       if (!allowMultiple) {
         handleClose();
         setActiveItems([selected]);
@@ -181,7 +195,7 @@ export function Picker({
           : [...selectedOptions, selected];
       });
     },
-    [updateText, listboxProps, allowMultiple, activeItems, handleClose],
+    [updateText, listboxProps, allowMultiple, handleClose, activeItems],
   );
 
   const firstSelectedOption = reactChildrenText(
@@ -241,7 +255,7 @@ export function Picker({
           >
             <Box paddingBlock="200">
               <Listbox {...listboxProps} onSelect={handleSelect}>
-                {filteredOptions?.map((option) => (
+                {filteredOptions.current?.map((option) => (
                   <Listbox.Option
                     key={option.value}
                     selected={activeItems.some((item) => item === option.value)}