diff --git a/static/app/components/searchQueryBuilder/index.spec.tsx b/static/app/components/searchQueryBuilder/index.spec.tsx index 4a53226d2807aa..03be8592b94927 100644 --- a/static/app/components/searchQueryBuilder/index.spec.tsx +++ b/static/app/components/searchQueryBuilder/index.spec.tsx @@ -2320,6 +2320,38 @@ describe('SearchQueryBuilder', () => { }); }); + it('sorts multi-selected values after re-opening the combobox', async () => { + const user = userEvent.setup(); + render(); + + 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( } ) => void; onKeyUp?: (e: KeyboardEvent) => void; - onOpenChange?: (newOpenState: boolean) => void; + onOpenChange?: (newOpenState: boolean, state?: any) => void; onPaste?: (e: React.ClipboardEvent) => void; openOnFocus?: boolean; placeholder?: string; + preserveFocusOnInputChange?: boolean; ref?: React.Ref; /** * Function to determine whether the menu should close when interacting with @@ -365,6 +366,7 @@ export function SearchQueryBuilderCombobox< onClick, customMenu, isOpen: incomingIsOpen, + preserveFocusOnInputChange = false, ['data-test-id']: dataTestId, ref, }: SearchQueryBuilderComboboxProps) { @@ -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), @@ -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, diff --git a/static/app/components/searchQueryBuilder/tokens/filter/valueCombobox.tsx b/static/app/components/searchQueryBuilder/tokens/filter/valueCombobox.tsx index ca21492a1a957d..3af7636756e7de 100644 --- a/static/app/components/searchQueryBuilder/tokens/filter/valueCombobox.tsx +++ b/static/app/components/searchQueryBuilder/tokens/filter/valueCombobox.tsx @@ -144,10 +144,9 @@ 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., "artifact_bundle.assemble" inside "artifact_bundle.assemble.find_projects") + const selected = text.charAt(item.value?.location.end.offset ?? 0) === ','; return {value, selected}; }); @@ -323,10 +322,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; }) { @@ -381,8 +382,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, @@ -395,6 +401,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 ( (() => { + 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)), ]), }, @@ -463,12 +495,21 @@ function useFilterSuggestions({ 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(() => { @@ -479,6 +520,7 @@ function useFilterSuggestions({ items, suggestionSectionItems, isFetching: isFetching || isDebouncing, + suggestionGroups, }; } @@ -546,6 +588,7 @@ export function SearchQueryBuilderValueCombobox({ }: SearchQueryValueBuilderProps) { const ref = useRef(null); const inputRef = useRef(null); + const comboboxStateRef = useRef(null); const organization = useOrganization(); const { getFieldDefinition, @@ -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( () => ({ @@ -933,14 +1014,30 @@ export function SearchQueryBuilderValueCombobox({ token={token} inputLabel={t('Edit filter value')} onInputChange={e => setInputValue(e.target.value)} - onKeyDown={onKeyDown} + onKeyDown={(e, {state}) => { + comboboxStateRef.current = state; + 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(); + } + }} + onOpenChange={(_isOpen, state) => { + // Capture the combobox state so we can restore focus after selections + if (state) { + comboboxStateRef.current = state; + } + }} autoFocus maxOptions={50} openOnFocus customMenu={customMenu} shouldCloseOnInteractOutside={shouldCloseOnInteractOutside} + preserveFocusOnInputChange={canSelectMultipleValues} > {suggestionSectionItems.map(section => (
diff --git a/static/app/components/searchQueryBuilder/tokens/filter/valueListBox.tsx b/static/app/components/searchQueryBuilder/tokens/filter/valueListBox.tsx index 930bc2a2a6e6df..c8fb28a0d0bf94 100644 --- a/static/app/components/searchQueryBuilder/tokens/filter/valueListBox.tsx +++ b/static/app/components/searchQueryBuilder/tokens/filter/valueListBox.tsx @@ -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'; @@ -118,6 +120,46 @@ export function ValueListBox>({ token, wrapperRef, }: ValueListBoxProps) { + // Track and restore focused option if react-aria clears it during multi-select updates + const lastFocusedKeyRef = useRef(null); + + const centerKeyInView = useCallback( + (key: Key | null) => { + if (key === null) return; + const container = listBoxRef.current; + if (!container) return; + const elId = getItemId(state as any, key as any); + if (!elId) return; + const el = document.getElementById(elId); + if (!el) return; + + const containerRect = container.getBoundingClientRect(); + const elRect = el.getBoundingClientRect(); + const offsetTopWithinContainer = + container.scrollTop + (elRect.top - containerRect.top); + const targetScrollTop = + offsetTopWithinContainer - container.clientHeight / 2 + elRect.height / 2; + + container.scrollTop = Math.max(0, targetScrollTop); + }, + [listBoxRef, state] + ); + + useEffect(() => { + lastFocusedKeyRef.current = state.selectionManager.focusedKey; + }, [state.selectionManager.focusedKey]); + + useLayoutEffect(() => { + if (!isOpen) return; + if ( + lastFocusedKeyRef.current !== null && + state.selectionManager.focusedKey === null + ) { + const keyToRestore = lastFocusedKeyRef.current; + state.selectionManager.setFocusedKey(keyToRestore); + centerKeyInView(keyToRestore); + } + }, [isOpen, state, centerKeyInView]); const totalOptions = items.reduce( (acc, item) => acc + (itemIsSection(item) ? item.options.length : 1), 0