From 4fac38ce01f39a276c82467925d6aadbbdf1cbe4 Mon Sep 17 00:00:00 2001
From: Thomas Heartman <thomas@getunleash.io>
Date: Wed, 7 Aug 2024 14:46:37 +0200
Subject: [PATCH] fix: select an item only from the filtered list of options
 (#7789)

Fixes a bug where the `handleSelection` function would select the
wrong item under certain conditions.

Because we always sent the unfiltered list of options to the function,
but took the index of the filtered items, the index would be off when
you have filtered the list and items before the selected items were
hidden.

This addresses that and also ports in some improvements I made when
setting up the config buttons for the new dialogs:

1. You can now use the space bar to select items that you have
focused (this is consistent with regular form interactions for
checkboxes)

2. When you have added text to the search field, pressing Enter will
select the top-most item (this is consistent with how these fields
work in linear, for instance) as long as your focus is still in the
search field. If you have moved it to the list, enter will still
select an item on that list as expected.

Potential other addition: if you press "Enter" with an empty search
field, we could close the box but keep your selection the same. Again,
this is how Linear does it, but I don't personally know what I'd
expect to happen there, so I'm happy to leave it as is.
---
 .../filter/FilterItem/FilterItem.tsx          | 104 ++++++++++--------
 1 file changed, 58 insertions(+), 46 deletions(-)

diff --git a/frontend/src/component/filter/FilterItem/FilterItem.tsx b/frontend/src/component/filter/FilterItem/FilterItem.tsx
index 9f5bbce18d..76ba1ced0c 100644
--- a/frontend/src/component/filter/FilterItem/FilterItem.tsx
+++ b/frontend/src/component/filter/FilterItem/FilterItem.tsx
@@ -45,7 +45,21 @@ const useSelectionManagement = ({
         } else if (event.key === 'ArrowUp' && index > 0) {
             event.preventDefault();
             listRefs.current[index - 1]?.focus();
-        } else if (event.key === 'Enter') {
+        } else if (
+            event.key === 'Enter' &&
+            index === 0 &&
+            listRefs.current[0]?.value &&
+            options.length > 0
+        ) {
+            // if the search field is not empty and the user presses
+            // enter from the search field, toggle the topmost item in
+            // the filtered list event.preventDefault();
+            handleToggle(options[0].value)();
+        } else if (
+            event.key === 'Enter' ||
+            // allow selection with space when not in the search field
+            (index !== 0 && event.key === ' ')
+        ) {
             event.preventDefault();
             if (index > 0) {
                 const listItemIndex = index - 1;
@@ -100,6 +114,10 @@ export const FilterItem: FC<IFilterItemProps> = ({
         onChipClose();
     };
 
+    const filteredOptions = options.filter((option) =>
+        option.label.toLowerCase().includes(searchText.toLowerCase()),
+    );
+
     const handleToggle = (value: string) => () => {
         if (
             selectedOptions?.some((selectedOption) => selectedOption === value)
@@ -123,7 +141,7 @@ export const FilterItem: FC<IFilterItemProps> = ({
     };
 
     const { listRefs, handleSelection } = useSelectionManagement({
-        options,
+        options: filteredOptions,
         handleToggle,
     });
 
@@ -185,52 +203,46 @@ export const FilterItem: FC<IFilterItemProps> = ({
                         onKeyDown={(event) => handleSelection(event, 0)}
                     />
                     <List sx={{ overflowY: 'auto' }} disablePadding>
-                        {options
-                            ?.filter((option) =>
-                                option.label
-                                    .toLowerCase()
-                                    .includes(searchText.toLowerCase()),
-                            )
-                            .map((option, index) => {
-                                const labelId = `checkbox-list-label-${option.value}`;
+                        {filteredOptions.map((option, index) => {
+                            const labelId = `checkbox-list-label-${option.value}`;
 
-                                return (
-                                    <StyledListItem
-                                        key={option.value}
-                                        dense
-                                        disablePadding
-                                        tabIndex={0}
-                                        onClick={handleToggle(option.value)}
-                                        ref={(el) => {
-                                            listRefs.current[index + 1] = el;
-                                        }}
-                                        onKeyDown={(event) =>
-                                            handleSelection(event, index + 1)
+                            return (
+                                <StyledListItem
+                                    key={option.value}
+                                    dense
+                                    disablePadding
+                                    tabIndex={0}
+                                    onClick={handleToggle(option.value)}
+                                    ref={(el) => {
+                                        listRefs.current[index + 1] = el;
+                                    }}
+                                    onKeyDown={(event) =>
+                                        handleSelection(event, index + 1)
+                                    }
+                                >
+                                    <StyledCheckbox
+                                        edge='start'
+                                        checked={
+                                            selectedOptions?.some(
+                                                (selectedOption) =>
+                                                    selectedOption ===
+                                                    option.value,
+                                            ) ?? false
                                         }
-                                    >
-                                        <StyledCheckbox
-                                            edge='start'
-                                            checked={
-                                                selectedOptions?.some(
-                                                    (selectedOption) =>
-                                                        selectedOption ===
-                                                        option.value,
-                                                ) ?? false
-                                            }
-                                            tabIndex={-1}
-                                            inputProps={{
-                                                'aria-labelledby': labelId,
-                                            }}
-                                            size='small'
-                                            disableRipple
-                                        />
-                                        <ListItemText
-                                            id={labelId}
-                                            primary={option.label}
-                                        />
-                                    </StyledListItem>
-                                );
-                            })}
+                                        tabIndex={-1}
+                                        inputProps={{
+                                            'aria-labelledby': labelId,
+                                        }}
+                                        size='small'
+                                        disableRipple
+                                    />
+                                    <ListItemText
+                                        id={labelId}
+                                        primary={option.label}
+                                    />
+                                </StyledListItem>
+                            );
+                        })}
                     </List>
                 </StyledDropdown>
             </StyledPopover>