From ecdc9b1f6728369ae6f0948764dfd66d1d8fed68 Mon Sep 17 00:00:00 2001 From: TkDodo Date: Wed, 19 Nov 2025 14:24:10 +0100 Subject: [PATCH 01/14] perf: do not render inside CompactSelect when the overlay is closed --- .../core/compactSelect/compactSelect.tsx | 6 +- .../core/compactSelect/composite.tsx | 11 +- .../components/core/compactSelect/control.tsx | 183 ++++++++---------- .../components/core/compactSelect/list.tsx | 16 +- 4 files changed, 95 insertions(+), 121 deletions(-) diff --git a/static/app/components/core/compactSelect/compactSelect.tsx b/static/app/components/core/compactSelect/compactSelect.tsx index 0bbdd7d117ecd6..6a10bbf97b43e4 100644 --- a/static/app/components/core/compactSelect/compactSelect.tsx +++ b/static/app/components/core/compactSelect/compactSelect.tsx @@ -114,6 +114,8 @@ export function CompactSelect({ disabled={controlDisabled} grid={grid} size={size} + items={itemsWithKey} + value={value ?? defaultValue} > ({ if ('options' in item) { return (
- {item.options.map(opt => ( + {item.options?.map(opt => ( {opt.label} - ))} + )) ?? null}
); } diff --git a/static/app/components/core/compactSelect/composite.tsx b/static/app/components/core/compactSelect/composite.tsx index 8ef5073a923611..a5f011d2047a8b 100644 --- a/static/app/components/core/compactSelect/composite.tsx +++ b/static/app/components/core/compactSelect/composite.tsx @@ -65,7 +65,7 @@ type CompositeSelectChild = | null | undefined; -export interface CompositeSelectProps extends ControlProps { +export interface CompositeSelectProps extends Omit { /** * The "regions" inside this composite selector. Each region functions as a separated, * self-contained selectable list (each renders as a `ul` with its own list state) @@ -87,7 +87,14 @@ function CompositeSelect({ ...controlProps }: CompositeSelectProps) { return ( - + {Children.map(children, (child, index) => { diff --git a/static/app/components/core/compactSelect/control.tsx b/static/app/components/core/compactSelect/control.tsx index c27a28e2f3ffdc..1fd842b0f8243f 100644 --- a/static/app/components/core/compactSelect/control.tsx +++ b/static/app/components/core/compactSelect/control.tsx @@ -32,7 +32,7 @@ import useOverlay from 'sentry/utils/useOverlay'; import usePrevious from 'sentry/utils/usePrevious'; import type {SingleListProps} from './list'; -import type {SelectKey, SelectOption} from './types'; +import type {SelectKey, SelectOptionOrSection} from './types'; // autoFocus react attribute is sync called on render, this causes // layout thrashing and is bad for performance. This thin wrapper function @@ -57,15 +57,6 @@ interface SelectContextValue { * non-composite selectors, where there's only one list, that list's index is 0. */ registerListState: (index: number, listState: ListState) => void; - /** - * Function to be called when a list's selection state changes. We need a complete - * list of all selected options to label the trigger button. The `index` parameter - * indentifies the list, in the same way as in `registerListState`. - */ - saveSelectedOptions: ( - index: number, - newSelectedOptions: SelectOption | Array> - ) => void; /** * Search string to determine whether an option should be rendered in the select list. */ @@ -79,7 +70,6 @@ interface SelectContextValue { export const SelectContext = createContext({ registerListState: () => {}, - saveSelectedOptions: () => {}, overlayIsOpen: false, search: '', }); @@ -194,6 +184,7 @@ export interface ControlProps */ searchable?: boolean; size?: FormSize; + /** * Optional replacement for the default trigger button. Note that the replacement must * forward `props` and `ref` its outer wrap, otherwise many accessibility features @@ -240,6 +231,8 @@ export function Control({ menuBody, menuFooter, onOpenChange, + items, + value, // Select props size = 'md', @@ -253,7 +246,10 @@ export function Control({ grid = false, children, ...wrapperProps -}: ControlProps) { +}: ControlProps & { + items: Array>; + value: SelectKey | SelectKey[] | undefined; +}) { const wrapperRef = useRef(null); // Set up list states (in composite selects, each region has its own state, that way // selection values are contained within each region). @@ -423,24 +419,6 @@ export function Control({ [search] ); - /** - * A list of selected options across all select regions, to be used to generate the - * trigger label. - */ - const [selectedOptions, setSelectedOptions] = useState< - Array | Array>> - >([]); - const saveSelectedOptions = useCallback( - (index, newSelectedOptions) => { - setSelectedOptions(current => [ - ...current.slice(0, index), - newSelectedOptions, - ...current.slice(index + 1), - ]); - }, - [] - ); - /** * Trigger label, generated from current selection values. If more than one option is * selected, then a count badge will appear. @@ -450,7 +428,13 @@ export function Control({ return triggerLabelProp; } - const options = selectedOptions.flat().filter(Boolean); + const values = Array.isArray(value) ? value : [value]; + const options = items + .flatMap(item => { + if ('options' in item) return item.options.flat(); + return item; + }) + .filter(item => values.includes(item.value)); if (options.length === 0) { return {t('None')}; @@ -464,7 +448,7 @@ export function Control({ )} ); - }, [triggerLabelProp, selectedOptions]); + }, [triggerLabelProp, value, items]); const {keyboardProps: triggerKeyboardProps} = useKeyboard({ onKeyDown: e => { @@ -478,10 +462,7 @@ export function Control({ }, }); - const showClearButton = useMemo( - () => selectedOptions.flat().length > 0, - [selectedOptions] - ); + const showClearButton = useMemo(() => items.flat().length > 0, [items]); const contextValue = useMemo(() => { const registerListState: SelectContextValue['registerListState'] = ( @@ -497,12 +478,11 @@ export function Control({ return { registerListState, - saveSelectedOptions, overlayState, overlayIsOpen, search, }; - }, [saveSelectedOptions, overlayState, overlayIsOpen, search]); + }, [overlayState, overlayIsOpen, search]); const theme = useTheme(); @@ -521,69 +501,67 @@ export function Control({ {triggerLabel} )} - - - - {(menuTitle || - menuHeaderTrailingItems || - (clearable && showClearButton)) && ( - - {menuTitle} - - {loading && } - {typeof menuHeaderTrailingItems === 'function' - ? menuHeaderTrailingItems({closeOverlay: overlayState.close}) - : menuHeaderTrailingItems} - {clearable && showClearButton && ( - - {t('Clear')} - - )} - - - )} - {searchable && ( - updateSearch(e.target.value)} - size="xs" - {...searchKeyboardProps} - /> - )} - {typeof menuBody === 'function' - ? menuBody({closeOverlay: overlayState.close}) - : menuBody} - {!hideOptions && {children}} - {menuFooter && ( - - {typeof menuFooter === 'function' - ? menuFooter({ - closeOverlay: overlayState.close, - resetSearch: () => updateSearch(''), - }) - : menuFooter} - - )} - - - + {overlayIsOpen && ( + + + + {(menuTitle || + menuHeaderTrailingItems || + (clearable && showClearButton)) && ( + + {menuTitle} + + {loading && } + {typeof menuHeaderTrailingItems === 'function' + ? menuHeaderTrailingItems({closeOverlay: overlayState.close}) + : menuHeaderTrailingItems} + {clearable && showClearButton && ( + + {t('Clear')} + + )} + + + )} + {searchable && ( + updateSearch(e.target.value)} + size="xs" + {...searchKeyboardProps} + /> + )} + {typeof menuBody === 'function' + ? menuBody({closeOverlay: overlayState.close}) + : menuBody} + {!hideOptions && {children}} + {menuFooter && ( + + {typeof menuFooter === 'function' + ? menuFooter({ + closeOverlay: overlayState.close, + resetSearch: () => updateSearch(''), + }) + : menuFooter} + + )} + + + + )} ); @@ -698,9 +676,8 @@ const StyledOverlay = styled(Overlay, { const StyledPositionWrapper = styled(PositionWrapper, { shouldForwardProp: prop => isPropValid(prop), -})<{visible?: boolean; zIndex?: number}>` +})<{zIndex?: number}>` min-width: 100%; - display: ${p => (p.visible ? 'block' : 'none')}; z-index: ${p => p?.zIndex}; `; diff --git a/static/app/components/core/compactSelect/list.tsx b/static/app/components/core/compactSelect/list.tsx index 6eca0cd4042892..c50213a1ad3309 100644 --- a/static/app/components/core/compactSelect/list.tsx +++ b/static/app/components/core/compactSelect/list.tsx @@ -130,7 +130,7 @@ export interface MultipleListProps extends BaseListProp * In composite selectors, there may be multiple self-contained lists, each * representing a select "region". */ -function List({ +export function List({ items, value, defaultValue, @@ -147,7 +147,7 @@ function List({ closeOnSelect, ...props }: SingleListProps | MultipleListProps) { - const {overlayState, registerListState, saveSelectedOptions, search, overlayIsOpen} = + const {overlayState, registerListState, search, overlayIsOpen} = useContext(SelectContext); const hiddenOptions = useMemo( @@ -175,8 +175,6 @@ function List({ allowDuplicateSelectionEvents: true, onSelectionChange: selection => { const selectedOptions = getSelectedOptions(items, selection); - // Save selected options in SelectContext, to update the trigger label - saveSelectedOptions(compositeIndex, selectedOptions); onChange?.(selectedOptions); // Close menu if closeOnSelect is true @@ -203,8 +201,6 @@ function List({ allowDuplicateSelectionEvents: true, onSelectionChange: selection => { const selectedOption = getSelectedOptions(items, selection)[0]!; - // Save selected options in SelectContext, to update the trigger label - saveSelectedOptions(compositeIndex, selectedOption ?? null); onChange?.(selectedOption ?? null); // Close menu if closeOnSelect is true or undefined (by default single-selection @@ -228,8 +224,6 @@ function List({ hiddenOptions, multiple, disallowEmptySelection, - compositeIndex, - saveSelectedOptions, closeOnSelect, overlayState, ]); @@ -243,10 +237,6 @@ function List({ // Register the initialized list state once on mount useLayoutEffect(() => { registerListState(compositeIndex, listState); - saveSelectedOptions( - compositeIndex, - getSelectedOptions(items, listState.selectionManager.selectedKeys) - ); // eslint-disable-next-line react-hooks/exhaustive-deps }, [listState.collection]); @@ -385,5 +375,3 @@ function List({ ); } - -export {List}; From 562b528a9ba175be8a105ed3abfef2bd56676fa1 Mon Sep 17 00:00:00 2001 From: TkDodo Date: Wed, 19 Nov 2025 16:34:07 +0100 Subject: [PATCH 02/14] ref: defaultValue is no more --- static/app/components/core/compactSelect/compactSelect.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/static/app/components/core/compactSelect/compactSelect.tsx b/static/app/components/core/compactSelect/compactSelect.tsx index d0214d55f8fc5e..25f78645d2a646 100644 --- a/static/app/components/core/compactSelect/compactSelect.tsx +++ b/static/app/components/core/compactSelect/compactSelect.tsx @@ -3,8 +3,7 @@ import {Item, Section} from '@react-stately/collections'; import {t} from 'sentry/locale'; -import type {ControlProps} from './control'; -import {Control} from './control'; +import {Control, type ControlProps} from './control'; import type {MultipleListProps, SingleListProps} from './list'; import {List} from './list'; import {EmptyMessage} from './styles'; @@ -112,7 +111,7 @@ export function CompactSelect({ grid={grid} size={size} items={itemsWithKey} - value={value ?? defaultValue} + value={value} > Date: Wed, 19 Nov 2025 16:49:09 +0100 Subject: [PATCH 03/14] ref: make items and value optional for `control` --- static/app/components/core/compactSelect/composite.tsx | 9 +-------- static/app/components/core/compactSelect/control.tsx | 6 +++--- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/static/app/components/core/compactSelect/composite.tsx b/static/app/components/core/compactSelect/composite.tsx index d62e09dd95322b..0951364123b0e2 100644 --- a/static/app/components/core/compactSelect/composite.tsx +++ b/static/app/components/core/compactSelect/composite.tsx @@ -87,14 +87,7 @@ function CompositeSelect({ ...controlProps }: CompositeSelectProps) { return ( - + {Children.map(children, (child, index) => { diff --git a/static/app/components/core/compactSelect/control.tsx b/static/app/components/core/compactSelect/control.tsx index f3b2ec05f3604a..dcdb5f8a634a33 100644 --- a/static/app/components/core/compactSelect/control.tsx +++ b/static/app/components/core/compactSelect/control.tsx @@ -232,7 +232,7 @@ export function Control({ menuBody, menuFooter, onOpenChange, - items, + items = [], value, // Select props @@ -248,8 +248,8 @@ export function Control({ children, ...wrapperProps }: ControlProps & { - items: Array>; - value: SelectKey | SelectKey[] | undefined; + items?: Array>; + value?: SelectKey | SelectKey[] | undefined; }) { const wrapperRef = useRef(null); // Set up list states (in composite selects, each region has its own state, that way From 65eca0f25de9046a20a916fbe468c3a3774d37fc Mon Sep 17 00:00:00 2001 From: TkDodo Date: Wed, 19 Nov 2025 18:27:15 +0100 Subject: [PATCH 04/14] ref: move conditional rendering further down --- .../components/core/compactSelect/control.tsx | 131 +++++++++--------- 1 file changed, 69 insertions(+), 62 deletions(-) diff --git a/static/app/components/core/compactSelect/control.tsx b/static/app/components/core/compactSelect/control.tsx index dcdb5f8a634a33..ec05bafaf5526a 100644 --- a/static/app/components/core/compactSelect/control.tsx +++ b/static/app/components/core/compactSelect/control.tsx @@ -502,67 +502,73 @@ export function Control({ {triggerLabel} )} - {overlayIsOpen && ( - - - - {(menuTitle || - menuHeaderTrailingItems || - (clearable && showClearButton)) && ( - - {menuTitle} - - {loading && } - {typeof menuHeaderTrailingItems === 'function' - ? menuHeaderTrailingItems({closeOverlay: overlayState.close}) - : menuHeaderTrailingItems} - {clearable && showClearButton && ( - - {t('Clear')} - - )} - - - )} - {searchable && ( - updateSearch(e.target.value)} - size="xs" - {...searchKeyboardProps} - /> - )} - {typeof menuBody === 'function' - ? menuBody({closeOverlay: overlayState.close}) - : menuBody} - {!hideOptions && {children}} - {menuFooter && ( - - {typeof menuFooter === 'function' - ? menuFooter({ - closeOverlay: overlayState.close, - resetSearch: () => updateSearch(''), - }) - : menuFooter} - - )} - - - - )} + + + + {overlayIsOpen && ( + + {(menuTitle || + menuHeaderTrailingItems || + (clearable && showClearButton)) && ( + + {menuTitle} + + {loading && } + {typeof menuHeaderTrailingItems === 'function' + ? menuHeaderTrailingItems({closeOverlay: overlayState.close}) + : menuHeaderTrailingItems} + {clearable && showClearButton && ( + + {t('Clear')} + + )} + + + )} + {searchable && ( + updateSearch(e.target.value)} + size="xs" + {...searchKeyboardProps} + /> + )} + {typeof menuBody === 'function' + ? menuBody({closeOverlay: overlayState.close}) + : menuBody} + {!hideOptions && {children}} + {menuFooter && ( + + {typeof menuFooter === 'function' + ? menuFooter({ + closeOverlay: overlayState.close, + resetSearch: () => updateSearch(''), + }) + : menuFooter} + + )} + + )} + + + ); @@ -677,8 +683,9 @@ const StyledOverlay = styled(Overlay, { const StyledPositionWrapper = styled(PositionWrapper, { shouldForwardProp: prop => isPropValid(prop), -})<{zIndex?: number}>` +})<{visible?: boolean; zIndex?: number}>` min-width: 100%; + display: ${p => (p.visible ? 'block' : 'none')}; z-index: ${p => p?.zIndex}; `; From a3b60f74b09863199dd6fb05987e3a0d910931ba Mon Sep 17 00:00:00 2001 From: TkDodo Date: Thu, 20 Nov 2025 10:37:27 +0100 Subject: [PATCH 05/14] ref: compositeSelect gets a required trigger --- .../core/compactSelect/composite.spec.tsx | 21 ++++++++++------ .../core/compactSelect/composite.stories.tsx | 24 +++++++++---------- .../core/compactSelect/composite.tsx | 4 +++- .../overviewTimeline/sortSelector.tsx | 17 ++++++++----- .../components/membersFilter.tsx | 7 +++++- 5 files changed, 46 insertions(+), 27 deletions(-) diff --git a/static/app/components/core/compactSelect/composite.spec.tsx b/static/app/components/core/compactSelect/composite.spec.tsx index c1a55a70f630bd..c7012cc10d54a3 100644 --- a/static/app/components/core/compactSelect/composite.spec.tsx +++ b/static/app/components/core/compactSelect/composite.spec.tsx @@ -5,7 +5,10 @@ import {CompositeSelect} from './composite'; describe('CompactSelect', () => { it('renders', async () => { render( - + } + > { it('renders disabled trigger button', async () => { render( - + }> { // focus state. This test ensures that focus moves seamlessly between regions. it('manages focus between regions', async () => { render( - + }> { const region1Mock = jest.fn(); const region2Mock = jest.fn(); render( - + }> { it('can search', async () => { render( - + } + > { it('works with grid lists', async () => { render( - + }> { it('can use numbers as values', async () => { const onChange = jest.fn(); render( - + }> { options.

, - size: 'sm', - }} + trigger={props => ( + + )} > { , - children: 'Composite Select Single Select', - showChevron: false, - }} + trigger={props => ( + + )} > { /** * The "regions" inside this composite selector. Each region functions as a separated, * self-contained selectable list (each renders as a `ul` with its own list state) * whose values don't interfere with one another. */ children: CompositeSelectChild | CompositeSelectChild[]; + trigger: NonNullable; } /** diff --git a/static/app/views/insights/crons/components/overviewTimeline/sortSelector.tsx b/static/app/views/insights/crons/components/overviewTimeline/sortSelector.tsx index 1a084508a403a6..ec92c2f818ed1f 100644 --- a/static/app/views/insights/crons/components/overviewTimeline/sortSelector.tsx +++ b/static/app/views/insights/crons/components/overviewTimeline/sortSelector.tsx @@ -3,6 +3,7 @@ import { type CompositeSelectProps, } from 'sentry/components/core/compactSelect/composite'; import type {SelectOption} from 'sentry/components/core/compactSelect/types'; +import DropdownButton from 'sentry/components/dropdownButton'; import {IconSort} from 'sentry/icons'; import {t} from 'sentry/locale'; import {useLocation} from 'sentry/utils/useLocation'; @@ -77,12 +78,16 @@ export function SortSelector({onChangeOrder, onChangeSort, order, sort, size}: P return ( , - children: `${label} \u2014 ${orderLabel}`, - }} + trigger={props => ( + } + prefix={t('Sort By')} + {...props} + > + {`${label} \u2014 ${orderLabel}`} + + )} > , size: 'md', children: t('Filter')}} + trigger={props => ( + }> + {t('Filter')} + + )} maxMenuHeight="22rem" size="sm" > From 8055756de205597506743d122ab168b3f60106c4 Mon Sep 17 00:00:00 2001 From: TkDodo Date: Thu, 20 Nov 2025 10:43:10 +0100 Subject: [PATCH 06/14] ref: fix tests we no longer do trigger label updating --- static/app/components/core/compactSelect/composite.spec.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/static/app/components/core/compactSelect/composite.spec.tsx b/static/app/components/core/compactSelect/composite.spec.tsx index c7012cc10d54a3..20a67debd506e8 100644 --- a/static/app/components/core/compactSelect/composite.spec.tsx +++ b/static/app/components/core/compactSelect/composite.spec.tsx @@ -174,9 +174,8 @@ describe('CompactSelect', () => { // select Choice One await userEvent.click(screen.getByRole('option', {name: 'Choice One'})); - // Region 1's callback is called, and trigger label is updated + // Region 1's callback is called expect(region1Mock).toHaveBeenCalledWith({value: 'choice_one', label: 'Choice One'}); - expect(screen.getByRole('button', {name: 'Choice One'})).toBeInTheDocument(); // open the menu again await userEvent.click(screen.getByRole('button')); @@ -212,7 +211,6 @@ describe('CompactSelect', () => { expect(region2Mock).toHaveBeenCalledWith([ {value: 'choice_three', label: 'Choice Three'}, ]); - expect(screen.getByRole('button', {name: 'Choice One +1'})).toBeInTheDocument(); }); it('can search', async () => { From 5c5c864026b79329a0ce6730a460afaeb5604b45 Mon Sep 17 00:00:00 2001 From: TkDodo Date: Thu, 20 Nov 2025 11:23:02 +0100 Subject: [PATCH 07/14] fix: move conditional rendering to the highest possible point where auto focussing still works --- .../components/core/compactSelect/control.tsx | 120 +++++++++--------- 1 file changed, 59 insertions(+), 61 deletions(-) diff --git a/static/app/components/core/compactSelect/control.tsx b/static/app/components/core/compactSelect/control.tsx index ec05bafaf5526a..eac6bc464c1a0b 100644 --- a/static/app/components/core/compactSelect/control.tsx +++ b/static/app/components/core/compactSelect/control.tsx @@ -507,67 +507,65 @@ export function Control({ zIndex={theme.zIndex?.dropdown} {...overlayProps} > - - - {overlayIsOpen && ( - - {(menuTitle || - menuHeaderTrailingItems || - (clearable && showClearButton)) && ( - - {menuTitle} - - {loading && } - {typeof menuHeaderTrailingItems === 'function' - ? menuHeaderTrailingItems({closeOverlay: overlayState.close}) - : menuHeaderTrailingItems} - {clearable && showClearButton && ( - - {t('Clear')} - - )} - - - )} - {searchable && ( - updateSearch(e.target.value)} - size="xs" - {...searchKeyboardProps} - /> - )} - {typeof menuBody === 'function' - ? menuBody({closeOverlay: overlayState.close}) - : menuBody} - {!hideOptions && {children}} - {menuFooter && ( - - {typeof menuFooter === 'function' - ? menuFooter({ - closeOverlay: overlayState.close, - resetSearch: () => updateSearch(''), - }) - : menuFooter} - - )} - - )} - - + {overlayIsOpen && ( + + + {(menuTitle || + menuHeaderTrailingItems || + (clearable && showClearButton)) && ( + + {menuTitle} + + {loading && } + {typeof menuHeaderTrailingItems === 'function' + ? menuHeaderTrailingItems({closeOverlay: overlayState.close}) + : menuHeaderTrailingItems} + {clearable && showClearButton && ( + + {t('Clear')} + + )} + + + )} + {searchable && ( + updateSearch(e.target.value)} + size="xs" + {...searchKeyboardProps} + /> + )} + {typeof menuBody === 'function' + ? menuBody({closeOverlay: overlayState.close}) + : menuBody} + {!hideOptions && {children}} + {menuFooter && ( + + {typeof menuFooter === 'function' + ? menuFooter({ + closeOverlay: overlayState.close, + resetSearch: () => updateSearch(''), + }) + : menuFooter} + + )} + + + )} From 31c671e42af0a45772c8bb7695529538aa681024 Mon Sep 17 00:00:00 2001 From: TkDodo Date: Thu, 20 Nov 2025 11:23:39 +0100 Subject: [PATCH 08/14] fix: we need proper state management to make selection work now as all selects need to be fully controlled --- .../core/compactSelect/compactSelect.spec.tsx | 127 +++++++++++------- .../core/compactSelect/composite.spec.tsx | 63 +++++---- 2 files changed, 121 insertions(+), 69 deletions(-) diff --git a/static/app/components/core/compactSelect/compactSelect.spec.tsx b/static/app/components/core/compactSelect/compactSelect.spec.tsx index 4ad0b031c45d7e..5f0685793e4f6d 100644 --- a/static/app/components/core/compactSelect/compactSelect.spec.tsx +++ b/static/app/components/core/compactSelect/compactSelect.spec.tsx @@ -1,4 +1,4 @@ -import {Fragment} from 'react'; +import {Fragment, useState} from 'react'; import {render, screen, userEvent, waitFor} from 'sentry-test/reactTestingLibrary'; @@ -116,16 +116,25 @@ describe('CompactSelect', () => { describe('ListBox', () => { it('updates trigger label on selection', async () => { const mock = jest.fn(); - render( - - ); + + function Component() { + const [state, setState] = useState(); + return ( + { + mock(selection); + setState(selection.value); + }} + /> + ); + } + + render(); // click on the trigger button await userEvent.click(screen.getByRole('button')); @@ -139,17 +148,26 @@ describe('CompactSelect', () => { it('can select multiple options', async () => { const mock = jest.fn(); - render( - - ); + + function Component() { + const [state, setState] = useState([]); + return ( + { + mock(selection); + setState(selection.map(opt => opt.value)); + }} + value={state} + /> + ); + } + + render(); // click on the trigger button await userEvent.click(screen.getByRole('button')); @@ -468,17 +486,27 @@ describe('CompactSelect', () => { describe('GridList', () => { it('updates trigger label on selection', async () => { const mock = jest.fn(); - render( - - ); + + function Component() { + const [state, setState] = useState(); + + return ( + { + mock(selection); + setState(selection.value); + }} + /> + ); + } + + render(); // click on the trigger button await userEvent.click(screen.getByRole('button')); @@ -492,18 +520,27 @@ describe('CompactSelect', () => { it('can select multiple options', async () => { const mock = jest.fn(); - render( - - ); + + function Component() { + const [state, setState] = useState([]); + return ( + { + mock(selection); + setState(selection.map(opt => opt.value)); + }} + value={state} + /> + ); + } + + render(); // click on the trigger button await userEvent.click(screen.getByRole('button')); diff --git a/static/app/components/core/compactSelect/composite.spec.tsx b/static/app/components/core/compactSelect/composite.spec.tsx index 20a67debd506e8..662cb9699cf357 100644 --- a/static/app/components/core/compactSelect/composite.spec.tsx +++ b/static/app/components/core/compactSelect/composite.spec.tsx @@ -1,8 +1,10 @@ +import {useState} from 'react'; + import {render, screen, userEvent, waitFor} from 'sentry-test/reactTestingLibrary'; import {CompositeSelect} from './composite'; -describe('CompactSelect', () => { +describe('CompositeSelect', () => { it('renders', async () => { render( { it('has separate, async self-contained select regions', async () => { const region1Mock = jest.fn(); const region2Mock = jest.fn(); - render( - }> - - - - ); + + function Component() { + const [region1, setRegion1] = useState(); + const [region2, setRegion2] = useState([]); + return ( + }> + { + region1Mock(selection); + setRegion1(selection.value); + }} + value={region1} + options={[ + {value: 'choice_one', label: 'Choice One'}, + {value: 'choice_two', label: 'Choice Two'}, + ]} + /> + { + region2Mock(selection); + setRegion2(selection.map(s => s.value)); + }} + value={region2} + options={[ + {value: 'choice_three', label: 'Choice Three'}, + {value: 'choice_four', label: 'Choice Four'}, + ]} + /> + + ); + } + + render(); // click on the trigger button await userEvent.click(screen.getByRole('button')); From 988d9977154136a613f7ef7bdfc7dcd254867066 Mon Sep 17 00:00:00 2001 From: TkDodo Date: Fri, 21 Nov 2025 14:16:27 +0100 Subject: [PATCH 09/14] ref: remove compositeIndex as it's no longer needed --- .../core/compactSelect/compactSelect.tsx | 10 ++-------- .../core/compactSelect/composite.tsx | 19 ++++--------------- .../components/core/compactSelect/control.tsx | 5 +---- .../components/core/compactSelect/list.tsx | 5 ----- 4 files changed, 7 insertions(+), 32 deletions(-) diff --git a/static/app/components/core/compactSelect/compactSelect.tsx b/static/app/components/core/compactSelect/compactSelect.tsx index 3c0a09180dda04..c7f79d9c9c4b08 100644 --- a/static/app/components/core/compactSelect/compactSelect.tsx +++ b/static/app/components/core/compactSelect/compactSelect.tsx @@ -26,16 +26,10 @@ interface BaseSelectProps } export type SingleSelectProps = BaseSelectProps & - DistributiveOmit< - SingleListProps, - 'children' | 'items' | 'grid' | 'compositeIndex' | 'label' - >; + DistributiveOmit, 'children' | 'items' | 'grid' | 'label'>; export type MultipleSelectProps = BaseSelectProps & - DistributiveOmit< - MultipleListProps, - 'children' | 'items' | 'grid' | 'compositeIndex' | 'label' - >; + DistributiveOmit, 'children' | 'items' | 'grid' | 'label'>; export type SelectProps = | SingleSelectProps diff --git a/static/app/components/core/compactSelect/composite.tsx b/static/app/components/core/compactSelect/composite.tsx index 4bc6dd4dd4d30c..471bc44cc8f7f8 100644 --- a/static/app/components/core/compactSelect/composite.tsx +++ b/static/app/components/core/compactSelect/composite.tsx @@ -30,10 +30,7 @@ interface BaseCompositeSelectRegion { */ type SingleCompositeSelectRegion = BaseCompositeSelectRegion & - DistributiveOmit< - SingleListProps, - 'children' | 'items' | 'grid' | 'compositeIndex' | 'size' - >; + DistributiveOmit, 'children' | 'items' | 'grid' | 'size'>; /** * A multiple-selection (multiple options can be selected at the same time) "region" @@ -43,10 +40,7 @@ type SingleCompositeSelectRegion = */ type MultipleCompositeSelectRegion = BaseCompositeSelectRegion & - DistributiveOmit< - MultipleListProps, - 'children' | 'items' | 'grid' | 'compositeIndex' | 'size' - >; + DistributiveOmit, 'children' | 'items' | 'grid' | 'size'>; /** * A "region" inside a composite select. Each "region" is a separated, self-contained @@ -94,14 +88,12 @@ function CompositeSelect({ - {Children.map(children, (child, index) => { + {Children.map(children, child => { if (!child) { return null; } - return ( - - ); + return ; })} {/* Only displayed when all lists (regions) are empty */} @@ -129,7 +121,6 @@ CompositeSelect.Region = function ( export {CompositeSelect}; type RegionProps = CompositeSelectRegion & { - compositeIndex: SingleListProps['compositeIndex']; grid: SingleListProps['grid']; size: SingleListProps['size']; }; @@ -138,7 +129,6 @@ function Region({ options, isOptionDisabled, size, - compositeIndex, label, ...props }: RegionProps) { @@ -150,7 +140,6 @@ function Region({ items={itemsWithKey} isOptionDisabled={isOptionDisabled} shouldFocusWrap={false} - compositeIndex={compositeIndex} size={size} label={label} > diff --git a/static/app/components/core/compactSelect/control.tsx b/static/app/components/core/compactSelect/control.tsx index 7ac54ff8073abb..c8669d1872a7c8 100644 --- a/static/app/components/core/compactSelect/control.tsx +++ b/static/app/components/core/compactSelect/control.tsx @@ -69,10 +69,7 @@ export interface ControlProps extends Omit< React.BaseHTMLAttributes, // omit keys from SingleListProps because those will be passed to instead - | keyof Omit< - SingleListProps, - 'children' | 'items' | 'grid' | 'compositeIndex' | 'label' - > + | keyof Omit, 'children' | 'items' | 'grid' | 'label'> | 'defaultValue' >, Pick< diff --git a/static/app/components/core/compactSelect/list.tsx b/static/app/components/core/compactSelect/list.tsx index 414d3a8418fa95..21d3c8ccfc9ab5 100644 --- a/static/app/components/core/compactSelect/list.tsx +++ b/static/app/components/core/compactSelect/list.tsx @@ -52,10 +52,6 @@ interface BaseListProps | 'shouldUseVirtualFocus' > { items: Array>; - /** - * This list's index number inside composite select menus. - */ - compositeIndex?: number; /** * Whether to render a grid list rather than a list box. * @@ -164,7 +160,6 @@ export function List({ isOptionDisabled, shouldFocusWrap = true, shouldFocusOnHover = true, - compositeIndex = 0, sizeLimit, sizeLimitMessage, closeOnSelect, From b18d6e0c500197d25b7ee38e5e75ed35e11b7b7c Mon Sep 17 00:00:00 2001 From: TkDodo Date: Fri, 21 Nov 2025 14:18:40 +0100 Subject: [PATCH 10/14] fix: inline onClear and remove call to non-existing setSelectedOptions --- .../app/components/core/compactSelect/control.tsx | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/static/app/components/core/compactSelect/control.tsx b/static/app/components/core/compactSelect/control.tsx index c8669d1872a7c8..5814946fc85e18 100644 --- a/static/app/components/core/compactSelect/control.tsx +++ b/static/app/components/core/compactSelect/control.tsx @@ -278,14 +278,6 @@ export function Control({ }, }); - /** - * Clears selection values - */ - const clearSelection = () => { - setSelectedOptions([]); - onClear?.({overlayState}); - }; - // Manage overlay position const { isOpen: overlayIsOpen, @@ -503,7 +495,11 @@ export function Control({ ? menuHeaderTrailingItems({closeOverlay: overlayState.close}) : menuHeaderTrailingItems} {clearable && showClearButton && ( - + onClear?.({overlayState})} + size="zero" + borderless + > {t('Clear')} )} From 93f5e34a5efd6cc7217543bc3592e65c6dda9c72 Mon Sep 17 00:00:00 2001 From: TkDodo Date: Fri, 21 Nov 2025 14:34:59 +0100 Subject: [PATCH 11/14] fix: hasSelection logic --- .../app/components/core/compactSelect/control.tsx | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/static/app/components/core/compactSelect/control.tsx b/static/app/components/core/compactSelect/control.tsx index 5814946fc85e18..9be50c8ff04deb 100644 --- a/static/app/components/core/compactSelect/control.tsx +++ b/static/app/components/core/compactSelect/control.tsx @@ -440,7 +440,14 @@ export function Control({ }, }); - const showClearButton = useMemo(() => items.flat().length > 0, [items]); + const hasSelection = useMemo(() => { + const selectedOptions = Array.isArray(value) + ? value + : value === undefined + ? [] + : [value]; + return selectedOptions.flat().length > 0; + }, [value]); const contextValue = useMemo(() => { return { @@ -486,7 +493,7 @@ export function Control({ {(menuTitle || menuHeaderTrailingItems || - (clearable && showClearButton)) && ( + (clearable && hasSelection)) && ( {menuTitle} @@ -494,7 +501,7 @@ export function Control({ {typeof menuHeaderTrailingItems === 'function' ? menuHeaderTrailingItems({closeOverlay: overlayState.close}) : menuHeaderTrailingItems} - {clearable && showClearButton && ( + {clearable && hasSelection && ( onClear?.({overlayState})} size="zero" From 55180594ecb1c27ed194efaafd1371d7189e0572 Mon Sep 17 00:00:00 2001 From: TkDodo Date: Fri, 21 Nov 2025 16:48:00 +0100 Subject: [PATCH 12/14] test: adapt tests --- .../form/environmentSelector.spec.tsx | 18 ++++++++++++++- .../app/views/explore/spans/spansTab.spec.tsx | 5 ++++- .../aggregateColumnEditorModal.spec.tsx | 7 +++++- .../explore/tables/columnEditorModal.spec.tsx | 22 ++++++++++++++++--- .../app/views/explore/toolbar/index.spec.tsx | 15 +++++++++---- .../spans/selectors/domainSelector.spec.tsx | 12 +--------- .../databaseSystemSelector.spec.tsx | 1 - .../auditLogView.spec.tsx | 5 +++-- .../settings/project/projectTeams.spec.tsx | 2 +- 9 files changed, 62 insertions(+), 25 deletions(-) diff --git a/static/app/components/workflowEngine/form/environmentSelector.spec.tsx b/static/app/components/workflowEngine/form/environmentSelector.spec.tsx index bb2be65be0e8af..0151e6de833b8e 100644 --- a/static/app/components/workflowEngine/form/environmentSelector.spec.tsx +++ b/static/app/components/workflowEngine/form/environmentSelector.spec.tsx @@ -1,3 +1,5 @@ +import {useState} from 'react'; + import {initializeOrg} from 'sentry-test/initializeOrg'; import {render, screen, userEvent, within} from 'sentry-test/reactTestingLibrary'; @@ -16,7 +18,21 @@ describe('EnvironmentSelector', () => { const mockOnChange = jest.fn(); - render(); + function Component() { + const [environment, setEnvironment] = useState(''); + + return ( + { + setEnvironment(value); + mockOnChange(value); + }} + /> + ); + } + + render(); // Open list await userEvent.click(screen.getByRole('button', {name: 'All Environments'})); diff --git a/static/app/views/explore/spans/spansTab.spec.tsx b/static/app/views/explore/spans/spansTab.spec.tsx index d743cad308eca4..a57216993a49c7 100644 --- a/static/app/views/explore/spans/spansTab.spec.tsx +++ b/static/app/views/explore/spans/spansTab.spec.tsx @@ -191,7 +191,10 @@ describe('SpansTabContent', () => { // Add a group by, and leave one unselected await userEvent.click(aggregates); - await userEvent.click(within(groupBy).getByRole('button', {name: '\u2014'})); + + const editorColumn = screen.getAllByTestId('editor-column')[0]!; + + await userEvent.click(within(editorColumn).getByRole('button', {name: '\u2014'})); await userEvent.click(within(groupBy).getByRole('option', {name: 'project'})); expect(groupBys).toEqual(['project']); diff --git a/static/app/views/explore/tables/aggregateColumnEditorModal.spec.tsx b/static/app/views/explore/tables/aggregateColumnEditorModal.spec.tsx index 3cf05bf6106d7a..741b45328c49ed 100644 --- a/static/app/views/explore/tables/aggregateColumnEditorModal.spec.tsx +++ b/static/app/views/explore/tables/aggregateColumnEditorModal.spec.tsx @@ -263,7 +263,12 @@ describe('AggregateColumnEditorModal', () => { 'span.op', 'span.self_time', ]; - await userEvent.click(screen.getByRole('button', {name: 'Group By geo.country'})); + + const row = screen.getAllByTestId('editor-row')[0]!; + + await userEvent.click( + within(row).getByRole('button', {name: 'Group By geo.country'}) + ); const groupByOptions = await screen.findAllByRole('option'); groupByOptions.forEach((option, i) => { expect(option).toHaveTextContent(options[i]!); diff --git a/static/app/views/explore/tables/columnEditorModal.spec.tsx b/static/app/views/explore/tables/columnEditorModal.spec.tsx index 4428cd690f3f57..0ca360cc133797 100644 --- a/static/app/views/explore/tables/columnEditorModal.spec.tsx +++ b/static/app/views/explore/tables/columnEditorModal.spec.tsx @@ -1,4 +1,10 @@ -import {act, renderGlobalModal, screen, userEvent} from 'sentry-test/reactTestingLibrary'; +import { + act, + renderGlobalModal, + screen, + userEvent, + within, +} from 'sentry-test/reactTestingLibrary'; import {openModal} from 'sentry/actionCreators/modal'; import type {TagCollection} from 'sentry/types/group'; @@ -134,7 +140,12 @@ describe('ColumnEditorModal', () => { ['span.duration', 'number'], ['span.op', 'string'], ]; - await userEvent.click(screen.getByRole('button', {name: 'Column \u2014'})); + + const projectColumn = screen.getAllByTestId('editor-column')[2]!; + + await userEvent.click( + within(projectColumn).getByRole('button', {name: 'Column \u2014'}) + ); const columnOptions = await screen.findAllByRole('option'); columnOptions.forEach((option, i) => { expect(option).toHaveTextContent(options[i]![0]); @@ -182,7 +193,12 @@ describe('ColumnEditorModal', () => { ['span.duration', 'number'], ['span.op', 'string'], ]; - await userEvent.click(screen.getByRole('button', {name: 'Column project string'})); + + const projectColumn = screen.getAllByTestId('editor-column')[1]!; + + await userEvent.click( + within(projectColumn).getByRole('button', {name: 'Column project string'}) + ); const columnOptions = await screen.findAllByRole('option'); columnOptions.forEach((option, i) => { expect(option).toHaveTextContent(options[i]![0]); diff --git a/static/app/views/explore/toolbar/index.spec.tsx b/static/app/views/explore/toolbar/index.spec.tsx index 0a7b5d787dc8fc..b7f3ec3bbc4fc1 100644 --- a/static/app/views/explore/toolbar/index.spec.tsx +++ b/static/app/views/explore/toolbar/index.spec.tsx @@ -304,16 +304,17 @@ describe('ExploreToolbar', () => { let options: HTMLElement[]; const section = screen.getByTestId('section-group-by'); + const spanOpColumn = screen.getAllByTestId('editor-column')[0]!; expect(groupBys).toEqual(['']); - await userEvent.click(within(section).getByRole('button', {name: '\u2014'})); + await userEvent.click(within(spanOpColumn).getByRole('button', {name: '\u2014'})); options = await within(section).findAllByRole('option'); expect(options.length).toBeGreaterThan(0); await userEvent.click(within(section).getByRole('option', {name: 'span.op'})); expect(groupBys).toEqual(['span.op']); - await userEvent.click(within(section).getByRole('button', {name: 'span.op'})); + await userEvent.click(within(spanOpColumn).getByRole('button', {name: 'span.op'})); options = await within(section).findAllByRole('option'); expect(options.length).toBeGreaterThan(0); await userEvent.click(within(section).getByRole('option', {name: 'project'})); @@ -322,7 +323,12 @@ describe('ExploreToolbar', () => { await userEvent.click(within(section).getByRole('button', {name: 'Add Group'})); expect(groupBys).toEqual(['project', '']); - await userEvent.click(within(section).getByRole('button', {name: '\u2014'})); + const projectColumn = screen.getAllByTestId('editor-column')[1]!; + await userEvent.click( + within(projectColumn).getByRole('button', { + name: '\u2014', + }) + ); options = await within(section).findAllByRole('option'); expect(options.length).toBeGreaterThan(0); await userEvent.click( @@ -356,8 +362,9 @@ describe('ExploreToolbar', () => { expect(groupBys).toEqual(['']); const section = screen.getByTestId('section-group-by'); + const editorColumn = screen.getAllByTestId('editor-column')[0]!; - await userEvent.click(within(section).getByRole('button', {name: '\u2014'})); + await userEvent.click(within(editorColumn).getByRole('button', {name: '\u2014'})); await userEvent.click(within(section).getByRole('option', {name: 'span.op'})); expect(mode).toEqual(Mode.AGGREGATE); diff --git a/static/app/views/insights/common/views/spans/selectors/domainSelector.spec.tsx b/static/app/views/insights/common/views/spans/selectors/domainSelector.spec.tsx index b3692e99304b6e..47c8b1dd1de4e2 100644 --- a/static/app/views/insights/common/views/spans/selectors/domainSelector.spec.tsx +++ b/static/app/views/insights/common/views/spans/selectors/domainSelector.spec.tsx @@ -1,13 +1,7 @@ import {OrganizationFixture} from 'sentry-fixture/organization'; import {PageFilterStateFixture} from 'sentry-fixture/pageFilters'; -import { - render, - screen, - userEvent, - waitFor, - waitForElementToBeRemoved, -} from 'sentry-test/reactTestingLibrary'; +import {render, screen, userEvent, waitFor} from 'sentry-test/reactTestingLibrary'; import selectEvent from 'sentry-test/selectEvent'; import usePageFilters from 'sentry/utils/usePageFilters'; @@ -55,8 +49,6 @@ describe('DomainSelector', () => { it('allows selecting a domain', async () => { render(); - await waitForElementToBeRemoved(() => screen.queryByTestId('loading-indicator')); - await selectEvent.openMenu(screen.getByText('All')); expect(screen.getByText('sentry_user')).toBeInTheDocument(); @@ -85,8 +77,6 @@ describe('DomainSelector', () => { render(); expect(fetchMoreResponse).not.toHaveBeenCalled(); - await waitForElementToBeRemoved(() => screen.queryByTestId('loading-indicator')); - await selectEvent.openMenu(screen.getByText('All')); await userEvent.type(screen.getByRole('textbox'), 'p'); diff --git a/static/app/views/insights/database/components/databaseSystemSelector.spec.tsx b/static/app/views/insights/database/components/databaseSystemSelector.spec.tsx index c94d0896dfad36..64acea1f0be98a 100644 --- a/static/app/views/insights/database/components/databaseSystemSelector.spec.tsx +++ b/static/app/views/insights/database/components/databaseSystemSelector.spec.tsx @@ -219,7 +219,6 @@ describe('DatabaseSystemSelector', () => { expect(dropdownOptionLabels[1]).toHaveTextContent('MongoDB'); await userEvent.click(dropdownOptionLabels[0]!); - expect(dropdownSelector).toHaveTextContent('SystemPostgreSQL'); expect(mockSetState).toHaveBeenCalledWith('postgresql'); expect(mockNavigate).toHaveBeenCalledWith({ action: 'POP', diff --git a/static/app/views/settings/organizationAuditLog/auditLogView.spec.tsx b/static/app/views/settings/organizationAuditLog/auditLogView.spec.tsx index 957e6e23372a7d..0849ce0f14a413 100644 --- a/static/app/views/settings/organizationAuditLog/auditLogView.spec.tsx +++ b/static/app/views/settings/organizationAuditLog/auditLogView.spec.tsx @@ -30,8 +30,9 @@ describe('OrganizationAuditLog', () => { render(); expect(await screen.findByRole('heading')).toHaveTextContent('Audit Log'); - // Check that both textboxes are present (date selector search and event selector) - expect(screen.getAllByRole('textbox')).toHaveLength(2); + // Check that both date selector search and event selector are present + expect(screen.getByText('All time')).toBeInTheDocument(); + expect(screen.getByText('Select Action:')).toBeInTheDocument(); expect(screen.getByText('Member')).toBeInTheDocument(); expect(screen.getByText('Action')).toBeInTheDocument(); expect(screen.getByText('IP')).toBeInTheDocument(); diff --git a/static/app/views/settings/project/projectTeams.spec.tsx b/static/app/views/settings/project/projectTeams.spec.tsx index b4f429e887756f..99add240f33830 100644 --- a/static/app/views/settings/project/projectTeams.spec.tsx +++ b/static/app/views/settings/project/projectTeams.spec.tsx @@ -294,7 +294,7 @@ describe('ProjectTeams', () => { expect(await screen.findByText('Project Teams for project-slug')).toBeInTheDocument(); // Add new team - await userEvent.click(screen.getAllByRole('button', {name: 'Add Team'})[1]!); + await userEvent.click(screen.getByRole('button', {name: 'Add Team'})); await userEvent.click(screen.getByRole('button', {name: 'Create Team'})); From dc6b4d76a3fb0d1c8ec2f49f0c8fa6ced3522cc2 Mon Sep 17 00:00:00 2001 From: TkDodo Date: Fri, 21 Nov 2025 18:31:07 +0100 Subject: [PATCH 13/14] test: more test fixes --- static/app/views/dashboards/detail.spec.tsx | 2 +- .../views/dashboards/manage/dashboardTable.spec.tsx | 2 +- static/app/views/explore/logs/content.spec.tsx | 12 ++++++++++-- static/app/views/explore/logs/logsToolbar.spec.tsx | 10 ++++++---- 4 files changed, 18 insertions(+), 8 deletions(-) diff --git a/static/app/views/dashboards/detail.spec.tsx b/static/app/views/dashboards/detail.spec.tsx index a8da89b4830899..552ea5c708e5a8 100644 --- a/static/app/views/dashboards/detail.spec.tsx +++ b/static/app/views/dashboards/detail.spec.tsx @@ -1809,7 +1809,7 @@ describe('Dashboards > Detail', () => { ); await userEvent.click(await screen.findByText('All Releases')); - await userEvent.type(screen.getAllByPlaceholderText('Search\u2026')[2]!, 's'); + await userEvent.type(screen.getByPlaceholderText('Search\u2026'), 's'); await userEvent.click(await screen.findByRole('option', {name: 'search-result'})); // Validate that after search is cleared, search result still appears diff --git a/static/app/views/dashboards/manage/dashboardTable.spec.tsx b/static/app/views/dashboards/manage/dashboardTable.spec.tsx index 37cacddd04431a..0865b2092c30be 100644 --- a/static/app/views/dashboards/manage/dashboardTable.spec.tsx +++ b/static/app/views/dashboards/manage/dashboardTable.spec.tsx @@ -294,7 +294,7 @@ describe('Dashboards - DashboardTable', () => { expect(await screen.findAllByTestId('grid-head-cell')).toHaveLength(5); expect(screen.getByText('Access')).toBeInTheDocument(); - await userEvent.click((await screen.findAllByTestId('edit-access-dropdown'))[0]!); + await userEvent.click(screen.getByText('All')); expect(screen.getAllByPlaceholderText('Search Teams')[0]).toBeInTheDocument(); }); diff --git a/static/app/views/explore/logs/content.spec.tsx b/static/app/views/explore/logs/content.spec.tsx index 2a8c99358996e0..718eaa85ff6693 100644 --- a/static/app/views/explore/logs/content.spec.tsx +++ b/static/app/views/explore/logs/content.spec.tsx @@ -2,7 +2,13 @@ import {createLogFixtures, initializeLogsTest} from 'sentry-fixture/log'; import {ProjectKeysFixture} from 'sentry-fixture/projectKeys'; import {TeamFixture} from 'sentry-fixture/team'; -import {render, screen, userEvent, waitFor} from 'sentry-test/reactTestingLibrary'; +import { + render, + screen, + userEvent, + waitFor, + within, +} from 'sentry-test/reactTestingLibrary'; import * as useRecentCreatedProjectHook from 'sentry/components/onboarding/useRecentCreatedProject'; import ProjectsStore from 'sentry/stores/projectsStore'; @@ -179,7 +185,9 @@ describe('LogsPage', () => { eventStatsMock.mockClear(); await userEvent.click(screen.getByRole('button', {name: 'Expand sidebar'})); - await userEvent.click(screen.getByRole('button', {name: '\u2014'})); + + const editorColumn = screen.getAllByTestId('editor-column')[0]!; + await userEvent.click(within(editorColumn).getByRole('button', {name: '\u2014'})); await userEvent.click(screen.getByRole('option', {name: 'severity'})); await userEvent.click(screen.getByRole('tab', {name: 'Aggregates'})); diff --git a/static/app/views/explore/logs/logsToolbar.spec.tsx b/static/app/views/explore/logs/logsToolbar.spec.tsx index 40a68283710377..e06d7b9b69fe05 100644 --- a/static/app/views/explore/logs/logsToolbar.spec.tsx +++ b/static/app/views/explore/logs/logsToolbar.spec.tsx @@ -1,6 +1,6 @@ import type {ReactNode} from 'react'; -import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary'; +import {render, screen, userEvent, within} from 'sentry-test/reactTestingLibrary'; import {LogsAnalyticsPageSource} from 'sentry/utils/analytics/logsAnalyticsEvent'; import {LogsQueryParamsProvider} from 'sentry/views/explore/logs/logsQueryParamsProvider'; @@ -200,7 +200,8 @@ describe('LogsToolbar', () => { expect(mode).toEqual(Mode.SAMPLES); - await userEvent.click(screen.getByRole('button', {name: '\u2014'})); + const editorColumn = screen.getAllByTestId('editor-column')[0]!; + await userEvent.click(within(editorColumn).getByRole('button', {name: '\u2014'})); await userEvent.click(screen.getByRole('option', {name: 'message'})); expect(router.location.query.aggregateField).toEqual( [{groupBy: 'message'}, {yAxes: ['count(message)']}].map(aggregateField => @@ -210,7 +211,7 @@ describe('LogsToolbar', () => { expect(mode).toEqual(Mode.AGGREGATE); - await userEvent.click(screen.getByRole('button', {name: 'message'})); + await userEvent.click(within(editorColumn).getByRole('button', {name: 'message'})); await userEvent.click(screen.getByRole('option', {name: 'severity'})); expect(router.location.query.aggregateField).toEqual( [{groupBy: 'severity'}, {yAxes: ['count(message)']}].map(aggregateField => @@ -233,7 +234,8 @@ describe('LogsToolbar', () => { ); - await userEvent.click(screen.getByRole('button', {name: '\u2014'})); + const editorColumn = screen.getAllByTestId('editor-column')[0]!; + await userEvent.click(within(editorColumn).getByRole('button', {name: '\u2014'})); await userEvent.click(screen.getByRole('option', {name: 'message'})); await userEvent.click(screen.getByRole('button', {name: 'Add Group'})); From 4b1bf6275e1105593917593ad1c8aad31d88579a Mon Sep 17 00:00:00 2001 From: TkDodo Date: Fri, 21 Nov 2025 19:08:29 +0100 Subject: [PATCH 14/14] options?.flat --- static/app/components/core/compactSelect/control.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/static/app/components/core/compactSelect/control.tsx b/static/app/components/core/compactSelect/control.tsx index 9be50c8ff04deb..f1679b2ea9d3bb 100644 --- a/static/app/components/core/compactSelect/control.tsx +++ b/static/app/components/core/compactSelect/control.tsx @@ -409,7 +409,7 @@ export function Control({ const values = Array.isArray(value) ? value : [value]; const options = items .flatMap(item => { - if ('options' in item) return item.options.flat(); + if ('options' in item) return item.options?.flat(); return item; }) .filter(item => values.includes(item.value));