lib: Remove custom key navigation from TypeaheadSelect

pkg/lib/cockpit-components-multi-typeahead-select.tsx

@@ -189,7 +189,8 @@ export const MultiTypeaheadSelectBase: React.FunctionComponent<MultiTypeaheadSel
     }
   };
 
-  const selectOption = (option: string | number) => {
+    const selectOption = (option: string | number) => {
+    console.log("SEL", option, selected, selected.includes(option));
     if (selected.includes(option))
       onRemove(option);
     else

pkg/lib/cockpit-components-typeahead-select.tsx

@@ -76,6 +76,10 @@ SOFTWARE.
      done. And there is no visual nesting going on anyway. Keeping the
      options a flat list is just all around easier.
 
+   - Remove the custom menu key navigation. It wasn't doing anything,
+     actually, except duplicating what the menu already does and
+     producing a second focus item that was mostly out of sync with
+     the one already maintained by the Select component.
 */
 
 /* eslint-disable */
@@ -215,8 +219,6 @@ export const TypeaheadSelectBase: React.FunctionComponent<TypeaheadSelectProps>
   const [isOpen, setIsOpen] = React.useState(false);
   const [filterValue, setFilterValue] = React.useState<string>('');
   const [isFiltering, setIsFiltering] = React.useState<boolean>(false);
-  const [focusedItemIndex, setFocusedItemIndex] = React.useState<number | null>(null);
-  const [activeItemId, setActiveItemId] = React.useState<string | null>(null);
   const textInputRef = React.useRef<HTMLInputElement>();
 
   const NO_RESULTS = 'no results';
@@ -308,31 +310,21 @@ export const TypeaheadSelectBase: React.FunctionComponent<TypeaheadSelectProps>
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [isFiltering]);
 
-  const setActiveAndFocusedItem = (itemIndex: number) => {
-    setFocusedItemIndex(itemIndex);
-    const focusedItem = selectOptions[itemIndex] as TypeaheadSelectMenuOption;
-    setActiveItemId(String(focusedItem.value));
-  };
-
-  const resetActiveAndFocusedItem = () => {
-    setFocusedItemIndex(null);
-    setActiveItemId(null);
-  };
-
-  const openMenu = () => {
+  const openMenu = (menuFocus: boolean = false) => {
     if (!isOpen) {
       onToggle && onToggle(true);
       setIsOpen(true);
-      setTimeout(() => {
-        textInputRef.current?.focus();
-      }, 100);
+      if (!menuFocus) {
+        setTimeout(() => {
+          textInputRef.current?.focus();
+        }, 100);
+      }
     }
   };
 
   const closeMenu = () => {
     onToggle && onToggle(false);
     setIsOpen(false);
-    resetActiveAndFocusedItem();
     setIsFiltering(false);
     setFilterValue(String(selected?.content ?? ''));
   };
@@ -369,72 +361,16 @@ export const TypeaheadSelectBase: React.FunctionComponent<TypeaheadSelectProps>
     setIsFiltering(true);
     setFilterValue(value || '');
     onInputChange && onInputChange(value);
-
-    resetActiveAndFocusedItem();
-  };
-
-  const handleMenuArrowKeys = (key: string) => {
-    let indexToFocus = 0;
-
-    openMenu();
-
-    if (filteredSelections.every(o => !isMenu(o))) {
-      return;
-    }
-
-    if (key === 'ArrowUp') {
-      // When no index is set or at the first index, focus to the last, otherwise decrement focus index
-      if (focusedItemIndex === null || focusedItemIndex === 0) {
-        indexToFocus = filteredSelections.length - 1;
-      } else {
-        indexToFocus = focusedItemIndex - 1;
-      }
-
-      // Skip non-items
-      while (!isEnabledMenu(filteredSelections[indexToFocus])) {
-        indexToFocus--;
-        if (indexToFocus === -1) {
-          indexToFocus = filteredSelections.length - 1;
-        }
-      }
-    }
-
-    if (key === 'ArrowDown') {
-      // When no index is set or at the last index, focus to the first, otherwise increment focus index
-      if (focusedItemIndex === null || focusedItemIndex === filteredSelections.length - 1) {
-        indexToFocus = 0;
-      } else {
-        indexToFocus = focusedItemIndex + 1;
-      }
-
-      // Skip non-items
-      while (!isEnabledMenu(filteredSelections[indexToFocus])) {
-        indexToFocus++;
-        if (indexToFocus === filteredSelections.length) {
-          indexToFocus = 0;
-        }
-      }
-    }
-
-    setActiveAndFocusedItem(indexToFocus);
   };
 
   const onInputKeyDown = (event: React.KeyboardEvent<HTMLInputElement>) => {
-    const focusedItem = (focusedItemIndex !== null ? filteredSelections[focusedItemIndex] : null) as TypeaheadSelectMenuOption | null;
-
     switch (event.key) {
       case 'Enter':
-        if (isOpen && focusedItem && focusedItem.value !== NO_RESULTS && !focusedItem.isAriaDisabled) {
-          selectOption(event, focusedItem);
-        }
-
         openMenu();
-
         break;
       case 'ArrowUp':
       case 'ArrowDown':
-        event.preventDefault();
-        handleMenuArrowKeys(event.key);
+        openMenu(true);
         break;
     }
   };
@@ -455,7 +391,6 @@ export const TypeaheadSelectBase: React.FunctionComponent<TypeaheadSelectProps>
     setFilterValue('');
     onInputChange && onInputChange('');
     setIsFiltering(false);
-    resetActiveAndFocusedItem();
     textInputRef.current?.focus();
     onClearSelection && onClearSelection();
   };
@@ -485,7 +420,6 @@ export const TypeaheadSelectBase: React.FunctionComponent<TypeaheadSelectProps>
           autoComplete="off"
           innerRef={textInputRef}
           placeholder={placeholder}
-          {...(activeItemId && { 'aria-activedescendant': activeItemId })}
           role="combobox"
           isExpanded={isOpen}
           aria-controls="select-typeahead-listbox"
@@ -529,7 +463,7 @@ export const TypeaheadSelectBase: React.FunctionComponent<TypeaheadSelectProps>
 
           const { content, value, ...props } = option;
           return (
-            <SelectOption key={value} value={value} isFocused={focusedItemIndex === index} {...props}>
+            <SelectOption key={value} value={value} {...props}>
               {content}
             </SelectOption>
           );

pkg/playground/react-demo-multi-typeahead.tsx

@@ -0,0 +1,164 @@
+/*
+ * This file is part of Cockpit.
+ *
+ * Copyright (C) 2024 Red Hat, Inc.
+ *
+ * Cockpit is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; either version 2.1 of the License, or
+ * (at your option) any later version.
+ *
+ * Cockpit is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with Cockpit; If not, see <https://www.gnu.org/licenses/>.
+ */
+
+import cockpit from "cockpit";
+
+import React, { useState } from "react";
+import { createRoot, Container } from 'react-dom/client';
+
+import { Checkbox } from '@patternfly/react-core';
+import { MultiTypeaheadSelect, MultiTypeaheadSelectOption } from "cockpit-components-multi-typeahead-select";
+
+const MultiTypeaheadDemo = ({ options } : { options: MultiTypeaheadSelectOption[] }) => {
+    const [notFoundIsString, setNotFoundIsString] = useState(false);
+    const [selected, setSelected] = useState<(string | number)[]>([]);
+    const [toggles, setToggles] = useState(0);
+    const [changes, setChanges] = useState(0);
+
+    function add(val: string | number) {
+        setSelected(selected.concat([val]));
+    }
+
+    function rem(val: string | number) {
+        setSelected(selected.filter(v => v != val));
+    }
+
+    return (
+        <div>
+            <MultiTypeaheadSelect
+                id='multi-typeahead-widget'
+                placeholder="Select flavors"
+                isScrollable
+                noOptionsFoundMessage={notFoundIsString ? "Not found" : val => cockpit.format("'$0' not found", val) }
+                options={options}
+                selected={selected}
+                onAdd={add}
+                onRemove={rem}
+                onToggle={() => setToggles(val => val + 1)}
+                onInputChange={() => setChanges(val => val + 1)}
+            />
+            <div>Selected: <span id="multi-value">{JSON.stringify(selected)}</span></div>
+            <div>Toggles: <span id="multi-toggles">{toggles}</span></div>
+            <div>Changes: <span id="multi-changes">{changes}</span></div>
+            <Checkbox
+                id="notFoundIsStringMulti"
+                label="notFoundIsString"
+                isChecked={notFoundIsString}
+                onChange={(_event, checked) => setNotFoundIsString(checked)}
+            />
+        </div>
+    );
+};
+
+export function showMultiTypeaheadDemo(rootElement: Container) {
+    const flavors: string[] = [
+        "Alumni Swirl",
+        "Apple Cobbler Crunch",
+        "Arboretum Breeze",
+        "August Pie",
+        "Autumn Delight",
+        "Bavarian Raspberry Crunch",
+        "Berkey Brickle",
+        "Birthday Bash",
+        "Bittersweet Mint",
+        "Black Cow",
+        "Black Raspberry",
+        "Blueberry Cheesecake",
+        "Butter Pecan",
+        "Candy Bar/Snickers",
+        "Caramel Critters",
+        "Centennial Vanilla Bean",
+        "Cherry Cheesecake",
+        "Cherry Chip",
+        "Cherry Quist",
+        "Cherry Sherbet",
+        "Chocolate",
+        "Chocolate Cherry Cordia",
+        "Chocolate Chip",
+        "Chocolate Chip Cheesecake",
+        "Chocolate Chip Cookie Dough",
+        "Chocolate Chocolate Nut",
+        "Chocolate Marble",
+        "Chocolate Marshmallow",
+        "Chocolate Pretzel Crunch",
+        "Chunky Chocolate",
+        "Chunky Chocolate- Vanilla",
+        "Coconut Chip",
+        "Coffee Mocha Fudge",
+        "Coffee w/Cream and Sugar",
+        "Crazy Charlie Sundae Swirl",
+        "Death By Chocolate",
+        "Egg Nog",
+        "Espresso Fudge Pie",
+        "German Chocolate Cake",
+        "Golden Chocolate Pecan",
+        "Goo Goo Cluster",
+        "Grape Sherbet",
+        "Happy Happy Joy Joy",
+        "Heath Bar Candy",
+        "Just Fudge",
+        "Kenney Beany Chocolate",
+        "Lion Tracks",
+        "LionS'more",
+        "Mallo Cup",
+        "Maple Nut",
+        "Mint Nittany",
+        "Monster Mash",
+        "Orange Vanilla Sundae",
+        "Palmer Mousseum With Almonds",
+        "Peachy Paterno",
+        "Peanut Butter Cup",
+        "Peanut Butter Fudge Cluster",
+        "Peanut Butter Marshmallow",
+        "Peanut Butter Swirl",
+        "Pecan Apple Danish",
+        "Peppermint Stick",
+        "Pistachio",
+        "Pralines N Cream",
+        "Pumpkin Pie",
+        "Raspberry Fudge Torte",
+        "Raspberry Parfait",
+        "Rum Raisin",
+        "Russ 'Digs' Roseberry",
+        "Santa Fe Banana",
+        "Scholar's Chip",
+        "Sea Salt Chocolate Caramel",
+        "Somerset Shortcake",
+        "Southern Chocolate Pie",
+        "Southern Pecan Cheesecake",
+        "Strawberry",
+        "Strawberry Ice Cream",
+        "Strawberry Cheesecake",
+        "Teaberry",
+        "Tin Roof Sundae",
+        "Toasted Almond",
+        "Toasted Almond Fudge",
+        "Turtle Creek",
+        "Vanilla",
+        "Vanilla ice cream",
+        "White House",
+        "Wicked Caramel Sundae",
+        "WPSU Coffee Break",
+    ];
+
+    const options: MultiTypeaheadSelectOption[] = flavors.map((f, i) => ({ value: i + 1, content: f }));
+
+    const root = createRoot(rootElement);
+    root.render(<MultiTypeaheadDemo options={options} />);
+}

pkg/playground/react-patterns.html

@@ -23,6 +23,11 @@ <h3>Typeahead</h3>
                 <div id="demo-typeahead"></div>
             </section>
 
+            <section class="pf-v5-c-page__main-section pf-m-light">
+                <h3>Multi Typeahead</h3>
+                <div id="demo-multi-typeahead"></div>
+            </section>
+
             <section class="pf-v5-c-page__main-section pf-m-light">
                 <h3>Dialogs</h3>
                 <button id="demo-show-dialog" class="pf-v5-c-button pf-m-secondary">Show Dialog</button>

pkg/playground/react-patterns.js

@@ -31,6 +31,7 @@ import { showCardsDemo } from "./react-demo-cards.jsx";
 import { showUploadDemo } from "./react-demo-file-upload.jsx";
 import { showFileAcDemo, showFileAcDemoPreselected } from "./react-demo-file-autocomplete.jsx";
 import { showTypeaheadDemo } from "./react-demo-typeahead.jsx";
+import { showMultiTypeaheadDemo } from "./react-demo-multi-typeahead.jsx";
 
 /* -----------------------------------------------------------------------------
   Modal Dialog
@@ -130,6 +131,9 @@ document.addEventListener("DOMContentLoaded", function() {
     // Plain typeahead select with headers and dividers
     showTypeaheadDemo(document.getElementById('demo-typeahead'));
 
+    // Multi typeahead
+    showMultiTypeaheadDemo(document.getElementById('demo-multi-typeahead'));
+
     // Cards
     showCardsDemo(document.getElementById('demo-cards'));
 

test/verify/check-lib

@@ -18,6 +18,7 @@
 # along with Cockpit; If not, see <https://www.gnu.org/licenses/>.
 
 import testlib
+import time
 
 
 @testlib.nondestructive
@@ -70,14 +71,27 @@ class TestLib(testlib.MachineCase):
         # Select with keys, verify that dividers and headers are skipped
 
         b.click("#demo-typeahead .pf-v5-c-menu-toggle__button")
+        # The PF typeahead template asynchronously moves the focus to
+        # the text input, and navigation would be yanked to the start
+        # or end of the menu.  Let's wait for this to be over.
+        time.sleep(1)
+
         b.wait_visible("#typeahead-widget")
+        b.wait_visible("#demo-typeahead input:focus")
         b.key("ArrowDown")
+        b.wait_text("#demo-typeahead :focus", "The Start")
         b.key("ArrowUp")     # wraps around
+        b.wait_text("#demo-typeahead :focus", "The End")
         b.key("ArrowUp")     # skips over divider
+        b.wait_text("#demo-typeahead :focus", "Wyoming")
         b.key("ArrowDown")   # skips over divider
+        b.wait_text("#demo-typeahead :focus", "The End")
         b.key("ArrowDown")   # wraps around
+        b.wait_text("#demo-typeahead :focus", "The Start")
         b.key("ArrowDown")   # skips over divider and header
+        b.wait_text("#demo-typeahead :focus", "Alaska")
         b.key("ArrowDown")
+        b.wait_text("#demo-typeahead :focus", "Alabama")
         b.key("Enter")
         b.wait_not_present("#typeahead-widget")
         b.wait_text("#value", "AL")