feedback round 1

packages/react-core/src/components/Select/Select.tsx

@@ -5,9 +5,9 @@ import { Popper, PopperProps } from '../../helpers/Popper/Popper';
 import { getOUIAProps, OUIAProps, getDefaultOUIAId } from '../../helpers';
 
 export interface SelectPopperProps extends PopperProps {
-  /** popper direction */
+  /** Vertical direction of the popper. If enableFlip is set to true, this will set the initial direction before the popper flips. */
   direction?: 'up' | 'down';
-  /** popper position */
+  /** Horizontal position of the popper */
   position?: 'right' | 'left' | 'center';
   /** Custom width of the popper. If the value is "trigger", it will set the width to the select toggle's width */
   width?: string | 'trigger';
@@ -41,8 +41,10 @@ export interface SelectProps extends MenuProps, OUIAProps {
     toggleRef?: React.RefObject<any>
   ) => void;
   /** Callback to allow the select component to change the open state of the menu.
-   * Triggered by clicking outside of the menu, or by pressing either tab or escape. */
+   * Triggered by clicking outside of the menu, or by pressing either tab or escape (or specificed in onOpenChangeKeys). */
   onOpenChange?: (isOpen: boolean) => void;
+  /** Keys that trigger onOpenChange, defaults to tab and escape. */
+  onOpenChangeKeys?: string[];
   /** Indicates if the select should be without the outer box-shadow */
   isPlain?: boolean;
   /** @hide Forwarded ref */
@@ -63,6 +65,7 @@ const SelectBase: React.FunctionComponent<SelectProps & OUIAProps> = ({
   selected,
   toggle,
   onOpenChange,
+  onOpenChangeKeys = ['Escape', 'Tab'],
   isPlain,
   innerRef,
   zIndex = 9999,
@@ -83,7 +86,7 @@ const SelectBase: React.FunctionComponent<SelectProps & OUIAProps> = ({
         onOpenChange &&
         (menuRef.current?.contains(event.target as Node) || toggleRef.current?.contains(event.target as Node))
       ) {
-        if (event.key === 'Escape' || event.key === 'Tab') {
+        if (onOpenChangeKeys.includes(event.key)) {
           event.preventDefault();
           onOpenChange(false);
           toggleRef.current?.focus();
@@ -115,7 +118,7 @@ const SelectBase: React.FunctionComponent<SelectProps & OUIAProps> = ({
       window.removeEventListener('keydown', handleMenuKeys);
       window.removeEventListener('click', handleClick);
     };
-  }, [isOpen, menuRef, onOpenChange]);
+  }, [isOpen, menuRef, onOpenChange, onOpenChangeKeys]);
 
   const menu = (
     <Menu

packages/react-core/src/components/Select/examples/Select.md

@@ -12,6 +12,8 @@ import BellIcon from '@patternfly/react-icons/dist/esm/icons/bell-icon';
 
 ## Examples
 
+`Select` builds off of the Menu component suite to wrap commonly used properties and functions for a select menu. See the [Menu documentation](/components/menus/menu) for a full list of properties that may be passed through Select to further customize the select menu, or the [custom menu examples](/components/menus/custom-menus) for additional examples of fully functional menus.
+
 ### Single
 
 ```ts file="./SelectBasic.tsx"
@@ -20,6 +22,8 @@ import BellIcon from '@patternfly/react-icons/dist/esm/icons/bell-icon';
 
 ### Option variations
 
+Showcases different option variants and customizations that are commonly used in a select menu. For a more complete list, see the [Menu documentation](/components/menus/menu).
+
 ```ts file="./SelectOptionVariations.tsx"
 
 ```
@@ -71,3 +75,9 @@ import BellIcon from '@patternfly/react-icons/dist/esm/icons/bell-icon';
 ```ts file="./SelectViewMore.tsx"
 
 ```
+
+### Footer
+
+```ts file="./SelectFooter.tsx"
+
+```

packages/react-core/src/components/Select/examples/SelectFooter.tsx

@@ -0,0 +1,64 @@
+import React from 'react';
+import { MenuToggle, MenuFooter, Select, SelectList, SelectOption, Button } from '@patternfly/react-core';
+
+export const SelectFooter: React.FunctionComponent = () => {
+  const [isOpen, setIsOpen] = React.useState<boolean>(false);
+  const [selected, setSelected] = React.useState<string>('Select a value');
+  const menuRef = React.useRef<HTMLDivElement>(null);
+
+  const onToggleClick = (ev: React.MouseEvent) => {
+    ev.stopPropagation(); // Stop handleClickOutside from handling
+    setTimeout(() => {
+      if (menuRef.current) {
+        const firstElement = menuRef.current.querySelector('li > button:not(:disabled), li > a:not(:disabled)');
+        firstElement && (firstElement as HTMLElement).focus();
+      }
+    }, 0);
+    setIsOpen(!isOpen);
+  };
+
+  const toggle = (toggleRef) => (
+    <MenuToggle
+      ref={toggleRef}
+      onClick={onToggleClick}
+      isExpanded={isOpen}
+      style={
+        {
+          width: '200px'
+        } as React.CSSProperties
+      }
+    >
+      {selected}
+    </MenuToggle>
+  );
+
+  function onSelect(event: React.MouseEvent | undefined, itemId: string | number | undefined) {
+    if (typeof itemId === 'undefined') {
+      return;
+    }
+
+    setSelected(itemId.toString());
+  }
+
+  return (
+    <Select
+      isOpen={isOpen}
+      onOpenChange={(isOpen) => setIsOpen(isOpen)}
+      onOpenChangeKeys={['Escape']}
+      toggle={toggle}
+      ref={menuRef}
+      id="menu-with-footer"
+      onSelect={onSelect}
+      selected={selected}
+    >
+      <SelectList>
+        <SelectOption itemId="Option 1">Option 1</SelectOption>
+        <SelectOption itemId="Option 2">Option 2</SelectOption>
+        <SelectOption itemId="Option 3">Option 3</SelectOption>
+      </SelectList>
+      <MenuFooter>
+        <Button>Footer action</Button>
+      </MenuFooter>
+    </Select>
+  );
+};

packages/react-core/src/components/Select/examples/SelectMultiTypeaheadCheckbox.tsx

@@ -179,6 +179,7 @@ export const SelectMultiTypeaheadCheckbox: React.FunctionComponent = () => {
 
   return (
     <Select
+      role="menu"
       id="multi-typeahead-checkbox-select"
       ref={menuRef}
       isOpen={isOpen}
@@ -187,7 +188,7 @@ export const SelectMultiTypeaheadCheckbox: React.FunctionComponent = () => {
       onOpenChange={() => setIsOpen(false)}
       toggle={toggle}
     >
-      <SelectList isAriaMultiselectable id="select-multi-typeahead-checkbox-listbox">
+      <SelectList id="select-multi-typeahead-checkbox-listbox">
         {selectOptions.map((option, index) => (
           <SelectOption
             {...(!option.isDisabled && { hasCheckbox: true })}

packages/react-core/src/components/Select/examples/SelectOptionVariations.tsx

@@ -54,7 +54,13 @@ export const SelectOptionVariations: React.FunctionComponent = () => {
         <SelectOption itemId="Option with description" description="This is a description">
           Option with description
         </SelectOption>
-        <SelectOption itemId="Option with link" isExternalLink>
+        <SelectOption
+          to="#"
+          // Prevent default clicking functionality for example purposes only
+          onClick={(event) => event.preventDefault()}
+          itemId="Option with link"
+          isExternalLink
+        >
           Option with link
         </SelectOption>
         <SelectOption itemId="Option with icon" icon={<BellIcon />}>

packages/react-core/src/components/Select/examples/SelectTypeahead.tsx

@@ -26,6 +26,7 @@ export const SelectBasic: React.FunctionComponent = () => {
   const [isOpen, setIsOpen] = React.useState(false);
   const [selected, setSelected] = React.useState<string>('');
   const [inputValue, setInputValue] = React.useState<string>('');
+  const [filterValue, setFilterValue] = React.useState<string>('');
   const [selectOptions, setSelectOptions] = React.useState<SelectOptionProps[]>(initialSelectOptions);
   const [focusedItemIndex, setFocusedItemIndex] = React.useState<number | null>(null);
   const [activeItem, setActiveItem] = React.useState<string | null>(null);
@@ -37,17 +38,15 @@ export const SelectBasic: React.FunctionComponent = () => {
     let newSelectOptions: SelectOptionProps[] = initialSelectOptions;
 
     // Filter menu items based on the text input value when one exists
-    if (inputValue) {
-      newSelectOptions = initialSelectOptions.filter(menuItem =>
-        String(menuItem.children)
-          .toLowerCase()
-          .includes(inputValue.toLowerCase())
+    if (filterValue) {
+      newSelectOptions = initialSelectOptions.filter((menuItem) =>
+        String(menuItem.children).toLowerCase().includes(filterValue.toLowerCase())
       );
 
       // When no options are found after filtering, display 'No results found'
       if (!newSelectOptions.length) {
         newSelectOptions = [
-          { isDisabled: false, children: `No results found for "${inputValue}"`, itemId: 'no results' }
+          { isDisabled: false, children: `No results found for "${filterValue}"`, itemId: 'no results' }
         ];
       }
 
@@ -60,7 +59,7 @@ export const SelectBasic: React.FunctionComponent = () => {
     setSelectOptions(newSelectOptions);
     setActiveItem(null);
     setFocusedItemIndex(null);
-  }, [inputValue]);
+  }, [filterValue]);
 
   const onToggleClick = () => {
     setIsOpen(!isOpen);
@@ -72,6 +71,7 @@ export const SelectBasic: React.FunctionComponent = () => {
 
     if (itemId && itemId !== 'no results') {
       setInputValue(itemId as string);
+      setFilterValue('');
       setSelected(itemId as string);
     }
     setIsOpen(false);
@@ -81,6 +81,7 @@ export const SelectBasic: React.FunctionComponent = () => {
 
   const onTextInputChange = (_event: React.FormEvent<HTMLInputElement>, value: string) => {
     setInputValue(value);
+    setFilterValue(value);
   };
 
   const handleMenuArrowKeys = (key: string) => {
@@ -106,13 +107,13 @@ export const SelectBasic: React.FunctionComponent = () => {
       }
 
       setFocusedItemIndex(indexToFocus);
-      const focusedItem = selectOptions.filter(option => !option.isDisabled)[indexToFocus];
+      const focusedItem = selectOptions.filter((option) => !option.isDisabled)[indexToFocus];
       setActiveItem(`select-typeahead-${focusedItem.itemId.replace(' ', '-')}`);
     }
   };
 
   const onInputKeyDown = (event: React.KeyboardEvent<HTMLInputElement>) => {
-    const enabledMenuItems = selectOptions.filter(option => !option.isDisabled);
+    const enabledMenuItems = selectOptions.filter((option) => !option.isDisabled);
     const [firstMenuItem] = enabledMenuItems;
     const focusedItem = focusedItemIndex ? enabledMenuItems[focusedItemIndex] : firstMenuItem;
 
@@ -121,10 +122,11 @@ export const SelectBasic: React.FunctionComponent = () => {
       case 'Enter':
         if (isOpen && focusedItem.itemId !== 'no results') {
           setInputValue(String(focusedItem.children));
+          setFilterValue('');
           setSelected(String(focusedItem.children));
         }
 
-        setIsOpen(prevIsOpen => !prevIsOpen);
+        setIsOpen((prevIsOpen) => !prevIsOpen);
         setFocusedItemIndex(null);
         setActiveItem(null);
 
@@ -167,6 +169,7 @@ export const SelectBasic: React.FunctionComponent = () => {
               onClick={() => {
                 setSelected('');
                 setInputValue('');
+                setFilterValue('');
                 textInputRef?.current?.focus();
               }}
               aria-label="Clear input value"

packages/react-core/src/components/Select/examples/SelectTypeaheadCreatable.tsx

@@ -26,6 +26,7 @@ export const SelectTypeaheadCreatable: React.FunctionComponent = () => {
   const [isOpen, setIsOpen] = React.useState(false);
   const [selected, setSelected] = React.useState<string>('');
   const [inputValue, setInputValue] = React.useState<string>('');
+  const [filterValue, setFilterValue] = React.useState<string>('');
   const [selectOptions, setSelectOptions] = React.useState<SelectOptionProps[]>(initialSelectOptions);
   const [focusedItemIndex, setFocusedItemIndex] = React.useState<number | null>(null);
   const [activeItem, setActiveItem] = React.useState<string | null>(null);
@@ -38,14 +39,14 @@ export const SelectTypeaheadCreatable: React.FunctionComponent = () => {
     let newSelectOptions: SelectOptionProps[] = initialSelectOptions;
 
     // Filter menu items based on the text input value when one exists
-    if (inputValue) {
+    if (filterValue) {
       newSelectOptions = initialSelectOptions.filter((menuItem) =>
-        String(menuItem.children).toLowerCase().includes(inputValue.toLowerCase())
+        String(menuItem.children).toLowerCase().includes(filterValue.toLowerCase())
       );
 
       // When no options are found after filtering, display creation option
       if (!newSelectOptions.length) {
-        newSelectOptions = [{ isDisabled: false, children: `Create new option "${inputValue}"`, itemId: 'create' }];
+        newSelectOptions = [{ isDisabled: false, children: `Create new option "${filterValue}"`, itemId: 'create' }];
       }
 
       // Open the menu when the input value changes and the new value is not empty
@@ -57,7 +58,7 @@ export const SelectTypeaheadCreatable: React.FunctionComponent = () => {
     setSelectOptions(newSelectOptions);
     setActiveItem(null);
     setFocusedItemIndex(null);
-  }, [inputValue, onCreation]);
+  }, [filterValue, onCreation]);
 
   const onToggleClick = () => {
     setIsOpen(!isOpen);
@@ -68,15 +69,17 @@ export const SelectTypeaheadCreatable: React.FunctionComponent = () => {
 
     if (itemId) {
       if (itemId === 'create') {
-        if (!initialSelectOptions.some((item) => item.itemId === inputValue)) {
-          initialSelectOptions = [...initialSelectOptions, { itemId: inputValue, children: inputValue }];
+        if (!initialSelectOptions.some((item) => item.itemId === filterValue)) {
+          initialSelectOptions = [...initialSelectOptions, { itemId: filterValue, children: filterValue }];
         }
-        setSelected(inputValue);
+        setSelected(filterValue);
         setOnCreation(!onCreation);
+        setFilterValue('');
       } else {
         // eslint-disable-next-line no-console
         console.log('selected', itemId);
         setInputValue(itemId as string);
+        setFilterValue('');
         setSelected(itemId as string);
       }
     }
@@ -88,6 +91,7 @@ export const SelectTypeaheadCreatable: React.FunctionComponent = () => {
 
   const onTextInputChange = (_event: React.FormEvent<HTMLInputElement>, value: string) => {
     setInputValue(value);
+    setFilterValue(value);
   };
 
   const handleMenuArrowKeys = (key: string) => {
@@ -126,15 +130,17 @@ export const SelectTypeaheadCreatable: React.FunctionComponent = () => {
     switch (event.key) {
       // Select the first available option
       case 'Enter':
-        if (!isOpen) {
-          setIsOpen((prevIsOpen) => !prevIsOpen);
-        } else if (isOpen) {
+        if (isOpen) {
           onSelect(undefined, focusedItem.itemId as string);
           setIsOpen((prevIsOpen) => !prevIsOpen);
           setFocusedItemIndex(null);
           setActiveItem(null);
         }
 
+        setIsOpen((prevIsOpen) => !prevIsOpen);
+        setFocusedItemIndex(null);
+        setActiveItem(null);
+
         break;
       case 'Tab':
       case 'Escape':
@@ -174,6 +180,7 @@ export const SelectTypeaheadCreatable: React.FunctionComponent = () => {
               onClick={() => {
                 setSelected('');
                 setInputValue('');
+                setFilterValue('');
                 textInputRef?.current?.focus();
               }}
               aria-label="Clear input value"

packages/react-core/src/demos/ComposableMenu/ComposableMenu.md

@@ -116,9 +116,3 @@ The flyout will automatically position to the left or top if it would otherwise
 ```ts file="./examples/ComposableDateSelect.tsx"
 
 ```
-
-### Select with footer
-
-```ts file="./examples/MenuWithFooter.tsx"
-
-```