diff --git a/static/app/components/assigneeSelectorDropdown.tsx b/static/app/components/assigneeSelectorDropdown.tsx index 656313f8e31ec4..b04a1338b617a5 100644 --- a/static/app/components/assigneeSelectorDropdown.tsx +++ b/static/app/components/assigneeSelectorDropdown.tsx @@ -311,7 +311,7 @@ export default function AssigneeSelectorDropdown({ })); }; - const handleSelect = (selectedOption: SelectOption | null) => { + const handleSelect = (selectedOption: SelectOption | undefined) => { // selectedOption is falsey when the option selected is already selected, or when the clear button is clicked if (!selectedOption) { if (onClear && group.assignedTo) { @@ -565,7 +565,6 @@ export default function AssigneeSelectorDropdown({ className={className} menuWidth={275} position="bottom-end" - disallowEmptySelection={false} onClick={e => e.stopPropagation()} value={ group.assignedTo diff --git a/static/app/components/charts/optionSelector.tsx b/static/app/components/charts/optionSelector.tsx index c7a2c7e8e41aa2..e8152a4f50d097 100644 --- a/static/app/components/charts/optionSelector.tsx +++ b/static/app/components/charts/optionSelector.tsx @@ -1,6 +1,8 @@ import {Fragment, useMemo} from 'react'; import styled from '@emotion/styled'; +import type {DistributiveOmit} from '@sentry/scraps/types'; + import {FeatureBadge} from 'sentry/components/core/badge'; import type { MultipleSelectProps, @@ -17,28 +19,39 @@ type BaseProps = { featureType?: 'alpha' | 'beta' | 'new'; }; -interface SingleProps - extends Omit< - SingleSelectProps, - 'onChange' | 'defaultValue' | 'multiple' | 'title' | 'value' - >, - BaseProps { - onChange: (value: string) => void; - selected: string; - multiple?: false; -} +type SingleUnClearableProps = DistributiveOmit< + SingleSelectProps, + 'onChange' | 'multiple' | 'title' | 'value' +> & + BaseProps & { + onChange: (value: string) => void; + selected: string; + clearable?: false; + multiple?: false; + }; -interface MultipleProps - extends Omit< - MultipleSelectProps, - 'onChange' | 'defaultValue' | 'multiple' | 'title' | 'value' - >, - BaseProps { - multiple: true; - onChange: (value: string[]) => void; - selected: string[]; - defaultValue?: string[]; -} +type SingleClearableProps = DistributiveOmit< + SingleSelectProps, + 'onChange' | 'multiple' | 'title' | 'value' +> & + BaseProps & { + clearable: true; + onChange: (value: string | undefined) => void; + selected: string; + multiple?: false; + }; + +type SingleProps = SingleClearableProps | SingleUnClearableProps; + +type MultipleProps = DistributiveOmit< + MultipleSelectProps, + 'onChange' | 'multiple' | 'title' | 'value' +> & + BaseProps & { + multiple: true; + onChange: (value: string[]) => void; + selected: string[]; + }; function OptionSelector({ options, @@ -48,6 +61,7 @@ function OptionSelector({ featureType, multiple, closeOnSelect, + clearable, ...rest }: SingleProps | MultipleProps) { const mappedOptions = useMemo(() => { @@ -65,6 +79,7 @@ function OptionSelector({ if (multiple) { return { multiple, + clearable, value: selected, onChange: (sel: Array>) => { onChange?.(sel.map(o => o.value)); @@ -73,13 +88,24 @@ function OptionSelector({ }; } + if (clearable) { + return { + multiple, + clearable, + value: selected, + onChange: (opt: SelectOption | undefined) => onChange?.(opt?.value), + closeOnSelect, + }; + } + return { multiple, + clearable, value: selected, - onChange: (opt: any) => onChange?.(opt.value), + onChange: (opt: SelectOption) => onChange?.(opt.value), closeOnSelect, }; - }, [multiple, selected, onChange, closeOnSelect]); + }, [clearable, multiple, selected, onChange, closeOnSelect]); function isOptionDisabled(option: SelectOptionWithKey) { return Boolean( diff --git a/static/app/components/core/compactSelect/compactSelect.spec.tsx b/static/app/components/core/compactSelect/compactSelect.spec.tsx index 4ad0b031c45d7e..53f14e64fa20cc 100644 --- a/static/app/components/core/compactSelect/compactSelect.spec.tsx +++ b/static/app/components/core/compactSelect/compactSelect.spec.tsx @@ -1,10 +1,104 @@ -import {Fragment} from 'react'; +import {Fragment, useState} from 'react'; +import {expectTypeOf} from 'expect-type'; import {render, screen, userEvent, waitFor} from 'sentry-test/reactTestingLibrary'; -import {CompactSelect} from './'; +import {CompactSelect, type SelectOption} from './'; describe('CompactSelect', () => { + describe('types', () => { + it('should enforce correct types for onChange for SingleSelect', () => { + void ( + { + expectTypeOf(option).toEqualTypeOf>(); + }} + closeOnSelect={option => { + expectTypeOf(option).toEqualTypeOf>(); + return true; + }} + options={[ + {value: 'opt_one', label: 'Option One'}, + {value: 'opt_two', label: 'Option Two'}, + ]} + /> + ); + }); + + it('should add undefined to onChange when clearable for SingleSelect', () => { + void ( + { + expectTypeOf(option).toEqualTypeOf< + SelectOption<'opt_one' | 'opt_two'> | undefined + >(); + }} + closeOnSelect={option => { + expectTypeOf(option).toEqualTypeOf< + SelectOption<'opt_one' | 'opt_two'> | undefined + >(); + return true; + }} + options={[ + {value: 'opt_one', label: 'Option One'}, + {value: 'opt_two', label: 'Option Two'}, + ]} + /> + ); + }); + + it('should always use arrays for MultiSelect', () => { + const values: Array<'opt_one' | 'opt_two'> = ['opt_one']; + void ( + { + expectTypeOf(option).toEqualTypeOf< + Array> + >(); + }} + closeOnSelect={option => { + expectTypeOf(option).toEqualTypeOf< + Array> + >(); + return true; + }} + options={[ + {value: 'opt_one', label: 'Option One'}, + {value: 'opt_two', label: 'Option Two'}, + ]} + /> + ); + + void ( + { + expectTypeOf(option).toEqualTypeOf< + Array> + >(); + }} + closeOnSelect={option => { + expectTypeOf(option).toEqualTypeOf< + Array> + >(); + return true; + }} + options={[ + {value: 'opt_one', label: 'Option One'}, + {value: 'opt_two', label: 'Option Two'}, + ]} + /> + ); + }); + }); + it('renders', async () => { render( { expect(await screen.findByRole('option', {name: 'Option Three'})).toBeInTheDocument(); }); + it('closes menu when clear is clicked', async () => { + render( + + ); + + // open the menu + await userEvent.click(screen.getByRole('button', {name: 'Option One'})); + expect(screen.getByRole('option', {name: 'Option One'})).toBeInTheDocument(); + + // click the clear button + await userEvent.click(screen.getByRole('button', {name: 'Clear'})); + + // menu is closed + await waitFor(() => { + expect(screen.queryByRole('option', {name: 'Option One'})).not.toBeInTheDocument(); + }); + }); + 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 +268,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')); @@ -167,17 +305,26 @@ describe('CompactSelect', () => { it('can select options with values containing quotes', 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')); @@ -330,34 +477,44 @@ describe('CompactSelect', () => { it('can toggle sections', async () => { const mock = jest.fn(); - render( - - ); + + function Component() { + const [state, setState] = useState([]); + + return ( + { + mock(selection); + setState(selection.map(opt => opt.value)); + }} + options={[ + { + key: 'section-1', + label: 'Section 1', + showToggleAllButton: true, + options: [ + {value: 'opt_one', label: 'Option One'}, + {value: 'opt_two', label: 'Option Two'}, + ], + }, + { + key: 'section-2', + label: 'Section 2', + showToggleAllButton: true, + options: [ + {value: 'opt_three', label: 'Option Three'}, + {value: 'opt_four', label: 'Option Four'}, + ], + }, + ]} + /> + ); + } + + render(); // click on the trigger button await userEvent.click(screen.getByRole('button', {expanded: false})); @@ -468,17 +625,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 +659,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')); @@ -521,18 +697,27 @@ describe('CompactSelect', () => { it('can select options with values containing quotes', 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')); @@ -639,35 +824,45 @@ describe('CompactSelect', () => { it('can toggle sections', async () => { const mock = jest.fn(); - render( - - ); + + function Component() { + const [state, setState] = useState([]); + + return ( + { + mock(selection); + setState(selection.map(opt => opt.value)); + }} + options={[ + { + key: 'section-1', + label: 'Section 1', + showToggleAllButton: true, + options: [ + {value: 'opt_one', label: 'Option One'}, + {value: 'opt_two', label: 'Option Two'}, + ], + }, + { + key: 'section-2', + label: 'Section 2', + showToggleAllButton: true, + options: [ + {value: 'opt_three', label: 'Option Three'}, + {value: 'opt_four', label: 'Option Four'}, + ], + }, + ]} + /> + ); + } + + render(); // click on the trigger button await userEvent.click(screen.getByRole('button', {expanded: false})); diff --git a/static/app/components/core/compactSelect/compactSelect.stories.tsx b/static/app/components/core/compactSelect/compactSelect.stories.tsx index 7ad5aeeec20518..1ee062e0914a39 100644 --- a/static/app/components/core/compactSelect/compactSelect.stories.tsx +++ b/static/app/components/core/compactSelect/compactSelect.stories.tsx @@ -284,7 +284,7 @@ export default Storybook.story('CompactSelect', story => { }); story('Simple', () => { - const [value, setValue] = useState(''); + const [value, setValue] = useState(); const options = [ {value: '', label: 'All'}, {value: '2', label: '2XX', details: 'Optional'}, diff --git a/static/app/components/core/compactSelect/compactSelect.tsx b/static/app/components/core/compactSelect/compactSelect.tsx index 93e167f8aedae0..ffdbaa4899503d 100644 --- a/static/app/components/core/compactSelect/compactSelect.tsx +++ b/static/app/components/core/compactSelect/compactSelect.tsx @@ -1,10 +1,11 @@ import {useId, useMemo} from 'react'; import {Item, Section} from '@react-stately/collections'; +import type {DistributiveOmit} from '@sentry/scraps/types'; + 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'; @@ -15,27 +16,26 @@ import type { SelectOptionOrSectionWithKey, SelectSection, } from './types'; -import {getItemsWithKeys} from './utils'; +import {getItemsWithKeys, shouldCloseOnSelect} from './utils'; export type {SelectOption, SelectOptionOrSection, SelectSection, SelectKey}; -interface BaseSelectProps extends ControlProps { +interface BaseSelectProps + extends Omit { options: Array>; } -export interface SingleSelectProps - extends BaseSelectProps, - Omit< - SingleListProps, - 'children' | 'items' | 'grid' | 'compositeIndex' | 'label' - > {} +export type SingleSelectProps = BaseSelectProps & + DistributiveOmit< + SingleListProps, + 'children' | 'items' | 'grid' | 'compositeIndex' | 'label' + >; -export interface MultipleSelectProps - extends BaseSelectProps, - Omit< - MultipleListProps, - 'children' | 'items' | 'grid' | 'compositeIndex' | 'label' - > {} +export type MultipleSelectProps = BaseSelectProps & + DistributiveOmit< + MultipleListProps, + 'children' | 'items' | 'grid' | 'compositeIndex' | 'label' + >; export type SelectProps = | SingleSelectProps @@ -63,7 +63,7 @@ export function CompactSelect({ onChange, onSectionToggle, multiple, - disallowEmptySelection, + clearable, isOptionDisabled, sizeLimit, sizeLimitMessage, @@ -81,9 +81,11 @@ export function CompactSelect({ // Combine list props into an object with two clearly separated types, one where // `multiple` is true and the other where it's not. Necessary to avoid TS errors. + // also multiple:false must be split into clearable true/false to satisfy TS const listProps = useMemo(() => { if (multiple) { return { + clearable, multiple, value, onChange, @@ -91,14 +93,27 @@ export function CompactSelect({ grid, }; } + + if (clearable) { + return { + clearable, + multiple, + value, + onChange, + closeOnSelect, + grid, + }; + } + return { + clearable, multiple, value, onChange, closeOnSelect, grid, }; - }, [multiple, value, onChange, closeOnSelect, grid]); + }, [multiple, clearable, value, onChange, closeOnSelect, grid]); const itemsWithKey = useMemo(() => getItemsWithKeys(options), [options]); @@ -111,12 +126,24 @@ export function CompactSelect({ disabled={controlDisabled} grid={grid} size={size} + clearable={clearable} + onClear={({overlayState}) => { + if (clearable) { + if (multiple) { + onChange([]); + } else { + onChange(undefined); + } + if (shouldCloseOnSelect?.({...listProps, selectedOptions: []})) { + overlayState?.close(); + } + } + }} > { +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')); @@ -260,29 +275,39 @@ describe('CompactSelect', () => { }); it('works with grid lists', async () => { - render( - }> - {}} - options={[ - {value: 'choice_one', label: 'Choice One'}, - {value: 'choice_two', label: 'Choice Two'}, - ]} - /> - - - ); + function Component() { + const [region1, setRegion1] = useState('choice_one'); + const [region2, setRegion2] = useState([]); + return ( + }> + { + setRegion1(selection.value); + }} + value={region1} + options={[ + {value: 'choice_one', label: 'Choice One'}, + {value: 'choice_two', label: 'Choice Two'}, + ]} + /> + { + 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')); diff --git a/static/app/components/core/compactSelect/composite.tsx b/static/app/components/core/compactSelect/composite.tsx index 68d140ef4d4752..cc13a37bcdafbd 100644 --- a/static/app/components/core/compactSelect/composite.tsx +++ b/static/app/components/core/compactSelect/composite.tsx @@ -3,6 +3,8 @@ import styled from '@emotion/styled'; import {FocusScope} from '@react-aria/focus'; import {Item} from '@react-stately/collections'; +import type {DistributiveOmit} from '@sentry/scraps/types'; + import {t} from 'sentry/locale'; import {space} from 'sentry/styles/space'; @@ -26,12 +28,12 @@ interface BaseCompositeSelectRegion { * renders as a `ul` with its own list state) whose selection values don't interfere * with one another. */ -interface SingleCompositeSelectRegion - extends BaseCompositeSelectRegion, - Omit< +type SingleCompositeSelectRegion = + BaseCompositeSelectRegion & + DistributiveOmit< SingleListProps, - 'children' | 'items' | 'grid' | 'compositeIndex' | 'size' | 'limitOptions' - > {} + 'children' | 'items' | 'grid' | 'compositeIndex' | 'size' + >; /** * A multiple-selection (multiple options can be selected at the same time) "region" @@ -39,12 +41,12 @@ interface SingleCompositeSelectRegion * list (each renders as a `ul` with its own list state) whose selection values don't * interfere with one another. */ -interface MultipleCompositeSelectRegion - extends BaseCompositeSelectRegion, - Omit< +type MultipleCompositeSelectRegion = + BaseCompositeSelectRegion & + DistributiveOmit< MultipleListProps, - 'children' | 'items' | 'grid' | 'compositeIndex' | 'size' | 'limitOptions' - > {} + 'children' | 'items' | 'grid' | 'compositeIndex' | 'size' + >; /** * A "region" inside a composite select. Each "region" is a separated, self-contained @@ -134,44 +136,18 @@ type RegionProps = CompositeSelectRegion & { function Region({ options, - value, - onChange, - multiple, - disallowEmptySelection, isOptionDisabled, - closeOnSelect, size, compositeIndex, label, ...props }: RegionProps) { - // Combine list props into an object with two clearly separated types, one where - // `multiple` is true and the other where it's not. Necessary to avoid TS errors. - const listProps = useMemo(() => { - if (multiple) { - return { - multiple, - value, - closeOnSelect, - onChange, - }; - } - return { - multiple, - value, - closeOnSelect, - onChange, - }; - }, [multiple, value, onChange, closeOnSelect]); - const itemsWithKey = useMemo(() => getItemsWithKeys(options), [options]); return ( void) { interface SelectContextValue { overlayIsOpen: boolean; - /** - * Function to be called once when a list is initialized, to register its state in - * SelectContext. In composite selectors, where there can be multiple lists, the - * `index` parameter is the list's index number (the order in which it appears). In - * 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`. + * identifies the list. */ saveSelectedOptions: ( index: number, @@ -78,7 +70,6 @@ interface SelectContextValue { } export const SelectContext = createContext({ - registerListState: () => {}, saveSelectedOptions: () => {}, overlayIsOpen: false, search: '', @@ -175,7 +166,7 @@ export interface ControlProps * Called when the clear button is clicked (applicable only when `clearable` is * true). */ - onClear?: () => void; + onClear?: (props: {overlayState: OverlayTriggerState}) => void; /** * Called when the menu is opened or closed. */ @@ -206,6 +197,7 @@ export interface ControlProps }, isOpen: boolean ) => React.ReactNode; + /** * Props to be passed to the default trigger button. */ @@ -256,10 +248,6 @@ export function Control({ ...wrapperProps }: ControlProps) { 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). - const [listStates, setListStates] = useState>>([]); - /** * Search/filter value, used to filter out the list of displayed elements */ @@ -298,11 +286,11 @@ export function Control({ }); /** - * Clears selection values across all list states + * Clears selection values */ const clearSelection = () => { - listStates.forEach(listState => listState.selectionManager.clearSelection()); - onClear?.(); + setSelectedOptions([]); + onClear?.({overlayState}); }; // Manage overlay position @@ -485,19 +473,7 @@ export function Control({ ); const contextValue = useMemo(() => { - const registerListState: SelectContextValue['registerListState'] = ( - index, - listState - ) => { - setListStates(current => [ - ...current.slice(0, index), - listState, - ...current.slice(index + 1), - ]); - }; - return { - registerListState, saveSelectedOptions, overlayState, overlayIsOpen, diff --git a/static/app/components/core/compactSelect/list.tsx b/static/app/components/core/compactSelect/list.tsx index b667faee6b9ea6..7e21328130e5d5 100644 --- a/static/app/components/core/compactSelect/list.tsx +++ b/static/app/components/core/compactSelect/list.tsx @@ -31,14 +31,16 @@ import { getHiddenOptions, getSelectedOptions, HiddenSectionToggle, + shouldCloseOnSelect, } from './utils'; export const SelectFilterContext = createContext(new Set()); interface BaseListProps - extends ListProps, + extends Omit, 'disallowEmptySelection'>, Omit< AriaListBoxOptions, + | 'disallowEmptySelection' | 'disabledKeys' | 'selectedKeys' | 'defaultSelectedKeys' @@ -48,6 +50,7 @@ interface BaseListProps >, Omit< AriaGridListOptions, + | 'disallowEmptySelection' | 'disabledKeys' | 'selectedKeys' | 'defaultSelectedKeys' @@ -99,9 +102,37 @@ interface BaseListProps sizeLimitMessage?: string; } -export interface SingleListProps extends BaseListProps { +/** + * A single-selection (only one option can be selected at a time) list that allows + * clearing the selection. + * `value` can be `undefined` to represent no selection. + */ +interface SingleClearableListProps extends BaseListProps { + /** + * If true, there will be a "Clear" button in the menu header. + */ + clearable: true; + onChange: (selectedOption: SelectOption | undefined) => void; + value: Value | undefined; + /** + * Whether to close the menu. Accepts either a boolean value or a callback function + * that receives the newly selected option and returns whether to close the menu. + */ + closeOnSelect?: + | boolean + | ((selectedOption: SelectOption | undefined) => boolean); + multiple?: false; +} + +export type SingleListProps = + | SingleClearableListProps + | SingleUnclearableListProps; + +interface SingleUnclearableListProps + extends BaseListProps { onChange: (selectedOption: SelectOption) => void; value: Value | undefined; + clearable?: false; /** * Whether to close the menu. Accepts either a boolean value or a callback function * that receives the newly selected option and returns whether to close the menu. @@ -114,6 +145,8 @@ export interface MultipleListProps extends BaseListProp multiple: true; onChange: (selectedOptions: Array>) => void; value: Value[] | undefined; + clearable?: boolean; // set to a regular boolean here because the empty type can be represented as an empty array + /** * Whether to close the menu. Accepts either a boolean value or a callback function * that receives the newly selected options and returns whether to close the menu. @@ -134,7 +167,7 @@ function List({ onChange, grid, multiple, - disallowEmptySelection, + clearable, isOptionDisabled, shouldFocusWrap = true, shouldFocusOnHover = true, @@ -144,7 +177,7 @@ function List({ closeOnSelect, ...props }: SingleListProps | MultipleListProps) { - const {overlayState, registerListState, saveSelectedOptions, search, overlayIsOpen} = + const {overlayState, saveSelectedOptions, search, overlayIsOpen} = useContext(SelectContext); const hiddenOptions = useMemo( @@ -166,8 +199,7 @@ function List({ selectionMode: 'multiple' as const, disabledKeys, // react-aria turns all keys into strings - selectedKeys: value?.map(getEscapedKey), - disallowEmptySelection, + selectedKeys: value?.map(getEscapedKey) ?? [], allowDuplicateSelectionEvents: true, onSelectionChange: selection => { const selectedOptions = getSelectedOptions(items, selection); @@ -175,12 +207,7 @@ function List({ saveSelectedOptions(compositeIndex, selectedOptions); onChange?.(selectedOptions); - // Close menu if closeOnSelect is true - if ( - typeof closeOnSelect === 'function' - ? closeOnSelect(selectedOptions) - : closeOnSelect - ) { + if (shouldCloseOnSelect({multiple, closeOnSelect, selectedOptions})) { overlayState?.close(); } }, @@ -191,22 +218,25 @@ function List({ selectionMode: 'single' as const, disabledKeys, // react-aria turns all keys into strings - selectedKeys: defined(value) ? [getEscapedKey(value)] : undefined, - disallowEmptySelection: disallowEmptySelection ?? true, + // we're setting selectedKeys to an empty array when value is undefined, because + // undefined makes react-aria treat it as uncontrolled + selectedKeys: defined(value) ? [getEscapedKey(value)] : [], + disallowEmptySelection: !clearable, 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); + saveSelectedOptions(compositeIndex, selectedOption); + onChange?.(selectedOption); // Close menu if closeOnSelect is true or undefined (by default single-selection // menus will close on selection) if ( - !defined(closeOnSelect) || - (typeof closeOnSelect === 'function' - ? closeOnSelect(selectedOption ?? null) - : closeOnSelect) + shouldCloseOnSelect({ + multiple, + closeOnSelect, + selectedOptions: [selectedOption], + }) ) { overlayState?.close(); } @@ -219,7 +249,7 @@ function List({ isOptionDisabled, hiddenOptions, multiple, - disallowEmptySelection, + clearable, compositeIndex, saveSelectedOptions, closeOnSelect, @@ -234,7 +264,6 @@ function List({ // Register the initialized list state once on mount useLayoutEffect(() => { - registerListState(compositeIndex, listState); saveSelectedOptions( compositeIndex, getSelectedOptions(items, listState.selectionManager.selectedKeys) diff --git a/static/app/components/core/compactSelect/utils.tsx b/static/app/components/core/compactSelect/utils.tsx index 74cae18802985f..2b33a8902963a3 100644 --- a/static/app/components/core/compactSelect/utils.tsx +++ b/static/app/components/core/compactSelect/utils.tsx @@ -6,7 +6,9 @@ import type {ListState} from '@react-stately/list'; import type {Node, Selection} from '@react-types/shared'; import {t} from 'sentry/locale'; +import {defined} from 'sentry/utils'; +import type {SelectProps} from './compactSelect'; import {SectionToggleButton} from './styles'; import type { SelectKey, @@ -327,3 +329,22 @@ export function itemIsSectionWithKey( ): item is SelectSectionWithKey { return 'options' in item; } + +export function shouldCloseOnSelect({ + multiple, + closeOnSelect, + selectedOptions, +}: Pick, 'multiple' | 'closeOnSelect'> & { + selectedOptions: Array>; +}) { + if (typeof closeOnSelect === 'function') { + // type assertions are necessary here because we don't have the discriminated union anymore + return closeOnSelect((multiple ? selectedOptions : selectedOptions[0]) as never); + } + if (defined(closeOnSelect)) { + return closeOnSelect; + } + // By default, single-selection lists close on select, while multiple-selection + // lists stay open + return !multiple; +} diff --git a/static/app/components/forms/fields/choiceMapperField.tsx b/static/app/components/forms/fields/choiceMapperField.tsx index 367fc622694e1f..9759a342876fcf 100644 --- a/static/app/components/forms/fields/choiceMapperField.tsx +++ b/static/app/components/forms/fields/choiceMapperField.tsx @@ -1,6 +1,8 @@ import {Component, Fragment} from 'react'; import styled from '@emotion/styled'; +import type {DistributiveOmit} from '@sentry/scraps/types'; + import {Button} from 'sentry/components/core/button'; import { CompactSelect, @@ -48,7 +50,7 @@ export interface ChoiceMapperProps extends DefaultProps { /** * Props forwarded to the add mapping dropdown. */ - addDropdown: Omit, 'options'> & { + addDropdown: DistributiveOmit, 'options' | 'clearable'> & { items: Array>; noResultsMessage?: string; }; diff --git a/static/app/components/timeRangeSelector/index.tsx b/static/app/components/timeRangeSelector/index.tsx index 0f6ca8ba54e033..66e5027a5f6b86 100644 --- a/static/app/components/timeRangeSelector/index.tsx +++ b/static/app/components/timeRangeSelector/index.tsx @@ -65,7 +65,7 @@ export interface TimeRangeSelectorProps | 'options' | 'hideOptions' | 'value' - | 'defaultValue' + | 'clearable' | 'onChange' | 'onInteractOutside' | 'closeOnSelect' @@ -263,8 +263,8 @@ export function TimeRangeSelector({ ); }, [showRelative, onChange, internalValue, hasChanges]); - const handleChange = useCallback['onChange']>>( - option => { + const handleChange = useCallback( + (option: SelectOption) => { // The absolute option was selected -> open absolute selector if (option.value === ABSOLUTE_OPTION_VALUE) { setInternalValue(current => { diff --git a/static/app/views/detectors/components/forms/metric/templateSection.tsx b/static/app/views/detectors/components/forms/metric/templateSection.tsx index 621ef553b9f194..4652d60d76b0a6 100644 --- a/static/app/views/detectors/components/forms/metric/templateSection.tsx +++ b/static/app/views/detectors/components/forms/metric/templateSection.tsx @@ -120,7 +120,6 @@ export function TemplateSection() { { return ( { - if (!option) { - return; - } const key = option.value; const meta = templateMetaByKey[key as MetricAlertType]; diff --git a/static/app/views/insights/mobile/screenload/components/affectSelector.tsx b/static/app/views/insights/mobile/screenload/components/affectSelector.tsx index 0655017ae467a5..69b44b7611ec7e 100644 --- a/static/app/views/insights/mobile/screenload/components/affectSelector.tsx +++ b/static/app/views/insights/mobile/screenload/components/affectSelector.tsx @@ -23,7 +23,6 @@ export function AffectSelector({transaction}: {transaction?: string}) { return ( void; + onChangeFilter: (newFilter: SpanOperationBreakdownFilter | undefined) => void; organization: OrganizationSummary; }; @@ -62,7 +62,6 @@ function Filter(props: Props) { | null) => { - setSelectedCategory(selectedOption?.value ?? undefined); + const onChange = (selectedOption: SelectOption | undefined) => { + setSelectedCategory(selectedOption?.value); navigate({ ...location, @@ -92,9 +92,7 @@ export function SpanCategoryFilter({serviceEntrySpanName}: Props) { return ( setSelectedCategory(undefined)} options={categoryOptions} value={selectedCategory ?? undefined} onChange={onChange} diff --git a/static/app/views/performance/transactionSummary/transactionEvents/content.tsx b/static/app/views/performance/transactionSummary/transactionEvents/content.tsx index 4786b1a4d241b3..afa5ed7bdfee1f 100644 --- a/static/app/views/performance/transactionSummary/transactionEvents/content.tsx +++ b/static/app/views/performance/transactionSummary/transactionEvents/content.tsx @@ -48,7 +48,9 @@ type Props = { eventsDisplayFilterName: EventsDisplayFilterName; location: Location; onChangeEventsDisplayFilter: (eventsDisplayFilterName: EventsDisplayFilterName) => void; - onChangeSpanOperationBreakdownFilter: (newFilter: SpanOperationBreakdownFilter) => void; + onChangeSpanOperationBreakdownFilter: ( + newFilter: SpanOperationBreakdownFilter | undefined + ) => void; organization: Organization; projectId: string; projects: Project[]; diff --git a/static/app/views/performance/transactionSummary/transactionEvents/index.tsx b/static/app/views/performance/transactionSummary/transactionEvents/index.tsx index 2c263f7d5310b9..c71f51969166c6 100644 --- a/static/app/views/performance/transactionSummary/transactionEvents/index.tsx +++ b/static/app/views/performance/transactionSummary/transactionEvents/index.tsx @@ -58,7 +58,7 @@ function TransactionEvents() { }; const onChangeSpanOperationBreakdownFilter = ( - newFilter: SpanOperationBreakdownFilter + newFilter: SpanOperationBreakdownFilter | undefined ) => { trackAnalytics('performance_views.transactionEvents.ops_filter_dropdown.selection', { organization, @@ -77,7 +77,9 @@ function TransactionEvents() { eventsFilterOptionSort?.kind === currentSort?.kind && eventsFilterOptionSort?.field === currentSort?.field ) { - sortQuery = filterEventsDisplayToLocationQuery(eventsDisplayFilterName, newFilter); + sortQuery = newFilter + ? filterEventsDisplayToLocationQuery(eventsDisplayFilterName, newFilter) + : {}; } const nextQuery: Location['query'] = { diff --git a/static/app/views/performance/transactionSummary/transactionOverview/content.tsx b/static/app/views/performance/transactionSummary/transactionOverview/content.tsx index 182c11a143d441..a275816202aa28 100644 --- a/static/app/views/performance/transactionSummary/transactionOverview/content.tsx +++ b/static/app/views/performance/transactionSummary/transactionOverview/content.tsx @@ -83,7 +83,7 @@ type Props = { eventView: EventView; isLoading: boolean; location: Location; - onChangeFilter: (newFilter: SpanOperationBreakdownFilter) => void; + onChangeFilter: (newFilter: SpanOperationBreakdownFilter | undefined) => void; organization: Organization; projectId: string; projects: Project[]; diff --git a/static/app/views/performance/transactionSummary/transactionOverview/index.tsx b/static/app/views/performance/transactionSummary/transactionOverview/index.tsx index d92ced8af6a234..0ea8b17e8e8338 100644 --- a/static/app/views/performance/transactionSummary/transactionOverview/index.tsx +++ b/static/app/views/performance/transactionSummary/transactionOverview/index.tsx @@ -143,7 +143,7 @@ function OTelOverviewContentWrapper() { const spanOperationBreakdownFilter = decodeFilterFromLocation(location); - const onChangeFilter = (newFilter: SpanOperationBreakdownFilter) => { + const onChangeFilter = (newFilter: SpanOperationBreakdownFilter | undefined) => { trackAnalytics('performance_views.filter_dropdown.selection', { organization, action: newFilter as string, @@ -249,7 +249,7 @@ function OverviewContentWrapper() { const spanOperationBreakdownFilter = decodeFilterFromLocation(location); - const onChangeFilter = (newFilter: SpanOperationBreakdownFilter) => { + const onChangeFilter = (newFilter: SpanOperationBreakdownFilter | undefined) => { trackAnalytics('performance_views.filter_dropdown.selection', { organization, action: newFilter as string, diff --git a/static/app/views/settings/components/settingsBreadcrumb/breadcrumbDropdown.tsx b/static/app/views/settings/components/settingsBreadcrumb/breadcrumbDropdown.tsx index ebd6a5b3bb887e..98d6d321259749 100644 --- a/static/app/views/settings/components/settingsBreadcrumb/breadcrumbDropdown.tsx +++ b/static/app/views/settings/components/settingsBreadcrumb/breadcrumbDropdown.tsx @@ -12,7 +12,8 @@ import Crumb from './crumb'; import Divider from './divider'; import type {RouteWithName} from './types'; -interface BreadcrumbDropdownProps extends Omit, 'onChange'> { +interface BreadcrumbDropdownProps + extends Omit, 'onChange' | 'clearable'> { name: React.ReactNode; onCrumbSelect: (value: string) => void; route: RouteWithName; diff --git a/static/gsApp/views/subscriptionPage/usageLog.tsx b/static/gsApp/views/subscriptionPage/usageLog.tsx index 6f2ee0f4c45d32..d4739329d92736 100644 --- a/static/gsApp/views/subscriptionPage/usageLog.tsx +++ b/static/gsApp/views/subscriptionPage/usageLog.tsx @@ -122,17 +122,17 @@ function UsageLog({location, subscription}: Props) { [auditLogs?.eventNames] ); - const handleEventFilter = (value: string | null) => { - if (value === null) { - // Clear filters + const handleEventFilter = (value: string | undefined) => { + if (typeof value === 'string') { navigate({ pathname: location.pathname, - query: {...location.query, event: undefined, cursor: undefined}, + query: {...location.query, event: value, cursor: undefined}, }); } else { + // Clear filters navigate({ pathname: location.pathname, - query: {...location.query, event: value, cursor: undefined}, + query: {...location.query, event: undefined, cursor: undefined}, }); } @@ -166,9 +166,8 @@ function UsageLog({location, subscription}: Props) { menuTitle={t('Subscription Actions')} options={eventNameOptions} value={selectedEventName} - onClear={() => handleEventFilter(null)} onChange={option => { - handleEventFilter(option.value); + handleEventFilter(option?.value); }} triggerProps={{ size: 'sm',