Skip to content
Open
32 changes: 32 additions & 0 deletions static/app/components/searchQueryBuilder/index.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2320,6 +2320,38 @@ describe('SearchQueryBuilder', () => {
});
});

it('sorts multi-selected values after re-opening the combobox', async () => {
const user = userEvent.setup();
render(<SearchQueryBuilder {...defaultProps} initialQuery='browser.name:""' />);

await userEvent.click(
screen.getByRole('button', {name: 'Edit value for filter: browser.name'})
);

const checkboxOptions = await screen.findAllByRole('checkbox');
expect(checkboxOptions).toHaveLength(4);
expect(checkboxOptions[0]).toHaveAccessibleName('Toggle Chrome');
expect(checkboxOptions[1]).toHaveAccessibleName('Toggle Firefox');

await user.keyboard('{Control>}');
await user.click(checkboxOptions[1]!);

expect(checkboxOptions[1]).toHaveAccessibleName('Toggle Firefox');
expect(checkboxOptions[1]).toBeChecked();

await user.keyboard('{Escape}');
await userEvent.click(
screen.getByRole('button', {name: 'Edit value for filter: browser.name'})
);

const updatedCheckboxOptions = await screen.findAllByRole('checkbox');
expect(updatedCheckboxOptions).toHaveLength(4);
expect(updatedCheckboxOptions[0]).toHaveAccessibleName('Toggle Firefox');
expect(updatedCheckboxOptions[0]).toBeChecked();
expect(updatedCheckboxOptions[1]).toHaveAccessibleName('Toggle Chrome');
expect(updatedCheckboxOptions[1]).not.toBeChecked();
});

it('collapses many selected options', async () => {
render(
<SearchQueryBuilder
Expand Down
18 changes: 13 additions & 5 deletions static/app/components/searchQueryBuilder/tokens/combobox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,11 @@ type SearchQueryBuilderComboboxProps<T extends SelectOptionOrSectionWithKey<stri
extra: {state: ComboBoxState<T>}
) => void;
onKeyUp?: (e: KeyboardEvent) => void;
onOpenChange?: (newOpenState: boolean) => void;
onOpenChange?: (newOpenState: boolean, state?: any) => void;
onPaste?: (e: React.ClipboardEvent<HTMLInputElement>) => void;
openOnFocus?: boolean;
placeholder?: string;
preserveFocusOnInputChange?: boolean;
ref?: React.Ref<HTMLInputElement>;
/**
* Function to determine whether the menu should close when interacting with
Expand Down Expand Up @@ -365,6 +366,7 @@ export function SearchQueryBuilderCombobox<
onClick,
customMenu,
isOpen: incomingIsOpen,
preserveFocusOnInputChange = false,
['data-test-id']: dataTestId,
ref,
}: SearchQueryBuilderComboboxProps<T>) {
Expand Down Expand Up @@ -462,10 +464,15 @@ export function SearchQueryBuilderCombobox<

const previousInputValue = usePrevious(inputValue);
useEffect(() => {
if (inputValue !== previousInputValue) {
if (!preserveFocusOnInputChange && inputValue !== previousInputValue) {
state.selectionManager.setFocusedKey(null);
}
}, [inputValue, previousInputValue, state.selectionManager]);
}, [
inputValue,
previousInputValue,
state.selectionManager,
preserveFocusOnInputChange,
]);

const totalOptions = items.reduce(
(acc, item) => acc + (itemIsSection(item) ? item.options.length : 1),
Expand All @@ -482,9 +489,10 @@ export function SearchQueryBuilderCombobox<
isOpen: incomingIsOpen,
});

// Always notify parent of state so they can maintain up-to-date references
useEffect(() => {
onOpenChange?.(isOpen);
}, [onOpenChange, isOpen]);
onOpenChange?.(isOpen, state);
}, [onOpenChange, isOpen, state]);

const {
overlayProps,
Expand Down
128 changes: 109 additions & 19 deletions static/app/components/searchQueryBuilder/tokens/filter/valueCombobox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -144,10 +144,10 @@ function getSelectedValuesFromText(

// Check if this value is selected by looking at the character after the value in
// the text. If there's a comma after the value, it means this value is selected.
// We need to check the text content to ensure that we account for any quotes the
// user may have added.
const valueText = item.value?.text ?? '';
const selected = text.charAt(text.indexOf(valueText) + valueText.length) === ',';
// Use the actual token location from the parser instead of indexOf to avoid
// matching substrings (e.g., "cool.property" inside "cool.property.thing")
const selected =
text.charAt(item.value?.location.end.offset ?? text.length - 1) === ',';

return {value, selected};
});
Expand Down Expand Up @@ -323,10 +323,12 @@ function useFilterSuggestions({
token,
filterValue,
selectedValues,
initialSelectedValues,
ctrlKeyPressed,
}: {
ctrlKeyPressed: boolean;
filterValue: string;
initialSelectedValues: Array<{selected: boolean; value: string}>;
selectedValues: Array<{selected: boolean; value: string}>;
token: TokenResult<Token.FILTER>;
}) {
Expand Down Expand Up @@ -381,8 +383,13 @@ function useFilterSuggestions({
enabled: shouldFetchValues,
});

// Create a ref to hold current selected values for the checkbox to access
// without causing item recreation
const selectedValuesRef = useRef(selectedValues);
selectedValuesRef.current = selectedValues;

const createItem = useCallback(
(suggestion: SuggestionItem, selected = false) => {
(suggestion: SuggestionItem) => {
return {
label: suggestion.label ?? suggestion.value,
value: suggestion.value,
Expand All @@ -395,6 +402,11 @@ function useFilterSuggestions({
return null;
}

// Look up selected state dynamically from ref to avoid recreating items
const selected = selectedValuesRef.current.some(
v => v.value === suggestion.value && v.selected
);

return (
<ItemCheckbox
isFocused={isFocused}
Expand Down Expand Up @@ -434,41 +446,71 @@ function useFilterSuggestions({
}, [data, predefinedValues, shouldFetchValues, key?.key]);

// Grouped sections for rendering purposes
// Use initialSelectedValues for ordering (to avoid re-ordering during selection)
// Selected state is looked up dynamically in the checkbox render to avoid recreating items
const suggestionSectionItems = useMemo<SuggestionSectionItem[]>(() => {
const allSuggestionValues = suggestionGroups
.flatMap(group => group.suggestions)
.map(s => s.value);

// Build list with initialSelectedValues at top, followed by all other suggestions
const initialValueSet = new Set(initialSelectedValues.map(v => v.value));

const itemsWithoutSection = suggestionGroups
.filter(group => group.sectionText === '')
.flatMap(group => group.suggestions)
.filter(suggestion => !selectedValues.some(v => v.value === suggestion.value));
.filter(suggestion => !initialValueSet.has(suggestion.value));
const sections = suggestionGroups.filter(group => group.sectionText !== '');

// Add the current filterValue as an option if it's not empty and not already in the list
const trimmedFilterValue = filterValue.trim();
const shouldShowFilterValue =
canSelectMultipleValues &&
trimmedFilterValue.length > 0 &&
!allSuggestionValues.includes(trimmedFilterValue) &&
!initialValueSet.has(trimmedFilterValue);

return [
{
sectionText: '',
items: getItemsWithKeys([
...selectedValues.map(value => {
// Show initial selected values at top (these were added via typing+comma)
...initialSelectedValues.map(value => {
const matchingSuggestion = suggestionGroups
.flatMap(group => group.suggestions)
.find(suggestion => suggestion.value === value.value);

if (matchingSuggestion) {
return createItem(matchingSuggestion, value.selected);
return createItem(matchingSuggestion);
}

return createItem({value: value.value}, value.selected);
return createItem({value: value.value});
}),
// Show currently typed value if it's new
...(shouldShowFilterValue ? [createItem({value: filterValue.trim()})] : []),
// Then all other suggestions in their original order
...itemsWithoutSection.map(suggestion => createItem(suggestion)),
]),
},
...sections.map(group => ({
sectionText: group.sectionText,
items: getItemsWithKeys(
group.suggestions
.filter(suggestion => !selectedValues.some(v => v.value === suggestion.value))
.filter(suggestion => !initialValueSet.has(suggestion.value))
.map(suggestion => createItem(suggestion))
),
})),
];
}, [createItem, selectedValues, suggestionGroups]);
// selectedValues is needed to trigger re-renders when checkboxes are toggled
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [
createItem,
suggestionGroups,
canSelectMultipleValues,
filterValue,
selectedValues,
initialSelectedValues,
]);

// Flat list used for state management
const items = useMemo(() => {
Expand All @@ -479,6 +521,7 @@ function useFilterSuggestions({
items,
suggestionSectionItems,
isFetching: isFetching || isDebouncing,
suggestionGroups,
};
}

Expand Down Expand Up @@ -613,12 +656,50 @@ export function SearchQueryBuilderValueCombobox({
}
}, []);

const {items, suggestionSectionItems, isFetching} = useFilterSuggestions({
token,
filterValue,
selectedValues: selectedValuesUnescaped,
ctrlKeyPressed,
});
// Track the initial selected values when the combobox is mounted to preserve list order during selection
const initialSelectedValues = useRef(selectedValuesUnescaped);
const prevInputValue = useRef(inputValue);

const {items, suggestionSectionItems, isFetching, suggestionGroups} =
useFilterSuggestions({
token,
filterValue,
selectedValues: selectedValuesUnescaped,
initialSelectedValues: initialSelectedValues.current,
ctrlKeyPressed,
});

// Update initialSelectedValues only for custom typed values (not existing suggestions)
useEffect(() => {
if (!canSelectMultipleValues) {
return;
}

// Only process if inputValue actually changed
if (prevInputValue.current === inputValue) {
return;
}
prevInputValue.current = inputValue;

// Get all values that exist in suggestions
const allSuggestionValues = new Set(
suggestionGroups.flatMap(group => group.suggestions).map(s => s.value)
);

// Find new selected values that are NOT in suggestions (i.e., custom typed values)
const currentInitialSet = new Set(initialSelectedValues.current.map(v => v.value));
const newCustomValues = selectedValuesUnescaped.filter(
v =>
v.selected && !currentInitialSet.has(v.value) && !allSuggestionValues.has(v.value)
);

if (newCustomValues.length > 0) {
initialSelectedValues.current = [
...initialSelectedValues.current,
...newCustomValues,
];
}
}, [inputValue, canSelectMultipleValues, selectedValuesUnescaped, suggestionGroups]);

const analyticsData = useMemo(
() => ({
Expand Down Expand Up @@ -933,14 +1014,23 @@ export function SearchQueryBuilderValueCombobox({
token={token}
inputLabel={t('Edit filter value')}
onInputChange={e => setInputValue(e.target.value)}
onKeyDown={onKeyDown}
onKeyDown={e => {
onKeyDown(e);
}}
onKeyUp={updateSelectionIndex}
onClick={updateSelectionIndex}
onClick={() => {
updateSelectionIndex();
// Also capture state on click to ensure it's available for mouse selections
if (inputRef.current) {
inputRef.current.focus();
}
}}
autoFocus
maxOptions={50}
openOnFocus
customMenu={customMenu}
shouldCloseOnInteractOutside={shouldCloseOnInteractOutside}
preserveFocusOnInputChange={canSelectMultipleValues}
>
{suggestionSectionItems.map(section => (
<Section key={section.sectionText} title={section.sectionText}>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import {Fragment, useCallback} from 'react';
import {Fragment, useCallback, useEffect, useLayoutEffect, useRef} from 'react';
import {createPortal} from 'react-dom';
import styled from '@emotion/styled';
import {getItemId} from '@react-aria/listbox';
import {isMac} from '@react-aria/utils';
import type {Key} from '@react-types/shared';

import {ListBox} from 'sentry/components/core/compactSelect/listBox';
import type {SelectOptionOrSectionWithKey} from 'sentry/components/core/compactSelect/types';
Expand Down Expand Up @@ -118,6 +120,45 @@ export function ValueListBox<T extends SelectOptionOrSectionWithKey<string>>({
token,
wrapperRef,
}: ValueListBoxProps<T>) {
// Track and restore focused option if react-aria clears it during multi-select updates
const lastFocusedKeyRef = useRef<Key | null>(null);

const centerKeyInView = useCallback(
(key: Key | null) => {
if (key === null || !listBoxRef.current) return;

const el = document.getElementById(getItemId(state as any, key as any));
if (!el) return;

const elRect = el.getBoundingClientRect();
const containerRect = listBoxRef.current.getBoundingClientRect();

const offsetTopWithinContainer =
listBoxRef.current.scrollTop + (elRect.top - containerRect.top);

const targetScrollTop =
offsetTopWithinContainer -
listBoxRef.current.clientHeight / 2 +
elRect.height / 2;

listBoxRef.current.scrollTop = Math.max(0, targetScrollTop);
},
[listBoxRef, state]
);

useEffect(() => {
lastFocusedKeyRef.current = state.selectionManager.focusedKey;
}, [state.selectionManager.focusedKey]);

useLayoutEffect(() => {
if (!isOpen) return;
if (!lastFocusedKeyRef.current) return;
if (state.selectionManager.focusedKey !== null) return;

state.selectionManager.setFocusedKey(lastFocusedKeyRef.current);
centerKeyInView(lastFocusedKeyRef.current);
}, [isOpen, state, centerKeyInView]);

const totalOptions = items.reduce(
(acc, item) => acc + (itemIsSection(item) ? item.options.length : 1),
0
Expand Down
Loading