Skip to content

Commit

Permalink
fix: select an item only from the filtered list of options (#7789)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
thomasheartman authored Aug 7, 2024
1 parent 22d97ff commit 4fac38c
Showing 1 changed file with 59 additions and 47 deletions.
106 changes: 59 additions & 47 deletions frontend/src/component/filter/FilterItem/FilterItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand All @@ -123,7 +141,7 @@ export const FilterItem: FC<IFilterItemProps> = ({
};

const { listRefs, handleSelection } = useSelectionManagement({
options,
options: filteredOptions,
handleToggle,
});

Expand Down Expand Up @@ -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}`;

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)
{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)
}
>
<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>
Expand Down

0 comments on commit 4fac38c

Please sign in to comment.