From c362d8e092aa69b6be2f435b1527f7f3a9393521 Mon Sep 17 00:00:00 2001 From: TkDodo Date: Mon, 26 Jan 2026 14:27:08 +0100 Subject: [PATCH 1/3] Reapply "fix(scraps): align leadingItems in compactSelect with check box/icon (#106167)" (#106889) This reverts commit 43df0b7c99b2b257d03b61f9ac92da0ef0889676. --- .../compactSelect/compactSelect.stories.tsx | 2 ++ .../core/compactSelect/gridList/option.tsx | 19 ++++++++------- .../core/compactSelect/listBox/option.tsx | 17 +++++++++----- .../components/core/compactSelect/styles.tsx | 23 +++++++++++-------- .../components/events/breadcrumbs/utils.tsx | 1 + .../views/dashboards/editAccessSelector.tsx | 5 ---- 6 files changed, 39 insertions(+), 28 deletions(-) diff --git a/static/app/components/core/compactSelect/compactSelect.stories.tsx b/static/app/components/core/compactSelect/compactSelect.stories.tsx index 35c4090e7bfd72..da96558a5ccae0 100644 --- a/static/app/components/core/compactSelect/compactSelect.stories.tsx +++ b/static/app/components/core/compactSelect/compactSelect.stories.tsx @@ -561,6 +561,8 @@ const arrayToOptions = (array: string[]) => array.map(item => ({ value: item, label: item, + details: 'Details', + leadingItems: , })); const COUNTRY_NAMES = Object.keys(countryNameToCode).sort(); diff --git a/static/app/components/core/compactSelect/gridList/option.tsx b/static/app/components/core/compactSelect/gridList/option.tsx index 17b6cbeca3c25a..671d40feceeaed 100644 --- a/static/app/components/core/compactSelect/gridList/option.tsx +++ b/static/app/components/core/compactSelect/gridList/option.tsx @@ -8,7 +8,7 @@ import type {ListState} from '@react-stately/list'; import type {Node} from '@react-types/shared'; import {Checkbox} from 'sentry/components/core/checkbox'; -import {CheckWrap} from 'sentry/components/core/compactSelect/styles'; +import {LeadWrap} from 'sentry/components/core/compactSelect/styles'; import {InnerWrap, MenuListItem} from 'sentry/components/core/menuListItem'; import {IconCheckmark} from 'sentry/icons'; import {space} from 'sentry/styles/space'; @@ -84,14 +84,19 @@ export function GridListOption({node, listState, size}: GridListOptionProps) { const leadingItemsMemo = useMemo(() => { const checkboxSize = size === 'xs' ? 'xs' : 'sm'; - if (hideCheck && !leadingItems) { + const leading = + typeof leadingItems === 'function' + ? leadingItems({disabled: isDisabled, isFocused, isSelected}) + : leadingItems; + + if (hideCheck && !leading) { return null; } return ( {!hideCheck && ( - + {multiple ? ( )} - + )} - {typeof leadingItems === 'function' - ? leadingItems({disabled: isDisabled, isFocused, isSelected}) - : leadingItems} + {leading ? {leading} : null} ); // eslint-disable-next-line react-hooks/exhaustive-deps - }, [multiple, isSelected, isDisabled, size, leadingItems, hideCheck]); + }, [multiple, isSelected, isDisabled, isFocused, size, leadingItems, hideCheck]); return ( { const checkboxSize = size === 'xs' ? 'xs' : 'sm'; - if (hideCheck && !leadingItems) { + const leading = + typeof leadingItems === 'function' + ? leadingItems({disabled: isDisabled, isFocused, isSelected}) + : leadingItems; + + if (hideCheck && !leading) { return null; } return ( {!hideCheck && ( - + )} - {leadingItems} + {leading ? : null} ); - }, [multiple, isSelected, isDisabled, size, leadingItems, hideCheck]); + }, [size, leadingItems, isDisabled, isFocused, isSelected, hideCheck, multiple]); return ( ` - display: flex; - justify-content: center; - align-items: center; - min-width: 1em; - height: 1.4em; - padding-bottom: 1px; - pointer-events: none; -`; +export function LeadWrap(props: FlexProps) { + return ( + + ); +} export const EmptyMessage = styled('p')` text-align: center; diff --git a/static/app/components/events/breadcrumbs/utils.tsx b/static/app/components/events/breadcrumbs/utils.tsx index 81b89cd796a16b..5104acfad2d53d 100644 --- a/static/app/components/events/breadcrumbs/utils.tsx +++ b/static/app/components/events/breadcrumbs/utils.tsx @@ -58,6 +58,7 @@ export const BREADCRUMB_TIME_DISPLAY_LOCALSTORAGE_KEY = 'event-breadcrumb-time-d const Color = styled('span')<{ colorConfig: NonNullable; }>` + display: flex; color: ${p => p.colorConfig.icon}; `; diff --git a/static/app/views/dashboards/editAccessSelector.tsx b/static/app/views/dashboards/editAccessSelector.tsx index 1e084c80283972..3a4fc3367b9f85 100644 --- a/static/app/views/dashboards/editAccessSelector.tsx +++ b/static/app/views/dashboards/editAccessSelector.tsx @@ -14,7 +14,6 @@ import {Tag} from 'sentry/components/core/badge/tag'; import {Button} from 'sentry/components/core/button'; import {ButtonBar} from 'sentry/components/core/button/buttonBar'; import {CompactSelect} from 'sentry/components/core/compactSelect'; -import {CheckWrap} from 'sentry/components/core/compactSelect/styles'; import {InnerWrap, LeadingItems} from 'sentry/components/core/menuListItem'; import {Tooltip} from 'sentry/components/core/tooltip'; import UserBadge from 'sentry/components/idBadge/userBadge'; @@ -412,10 +411,6 @@ const StyledCompactSelect = styled(CompactSelect)` ${LeadingItems} { margin-top: 0; } - - ${CheckWrap} { - padding-bottom: 0; - } `; const StyledDisplayName = styled('div')` From d6dadd6dd7d92a0b505d2a9740d459f8e74e316a Mon Sep 17 00:00:00 2001 From: TkDodo Date: Mon, 26 Jan 2026 16:30:41 +0100 Subject: [PATCH 2/3] ref: rewrite test to controlled --- .../organizations/hybridFilter.spec.tsx | 31 ++++++++++++------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/static/app/components/organizations/hybridFilter.spec.tsx b/static/app/components/organizations/hybridFilter.spec.tsx index f7f5253ee7ce04..fa13728dddd9a2 100644 --- a/static/app/components/organizations/hybridFilter.spec.tsx +++ b/static/app/components/organizations/hybridFilter.spec.tsx @@ -1,3 +1,5 @@ +import {useState} from 'react'; + import { fireEvent, render, @@ -57,9 +59,23 @@ describe('ProjectPageFilter', () => { it('handles multiple selection', async () => { const onChange = jest.fn(); - const {rerender} = render( - - ); + function ControlledHybridFilter() { + const [value, setValue] = useState([]); + + return ( + { + onChange(newValue); + setValue(newValue); + }} + /> + ); + } + + render(); // Clicking on the checkboxes in Option One & Option Two _adds_ the options to the // current selection state (multiple selection mode) @@ -73,15 +89,6 @@ describe('ProjectPageFilter', () => { await userEvent.click(screen.getByRole('button', {name: 'Apply'})); expect(onChange).toHaveBeenCalledWith(expect.arrayContaining(['one', 'two'])); - // HybridFilter is controlled-only, so we need to rerender it with new value - rerender( - - ); await userEvent.click(screen.getByRole('button', {expanded: false})); // Ctrl-clicking on Option One & Option Two _removes_ them to the current selection From 02cee3ce3cac0d867f1e4977b9eac6ee8724c766 Mon Sep 17 00:00:00 2001 From: TkDodo Date: Mon, 26 Jan 2026 17:10:32 +0100 Subject: [PATCH 3/3] fix: do not wrap leading items into an additional wrapper if hideCheck is passed with hideCheck, call-sides like HybridFilter will pass their own checked indicator in `leading`, so we shouldn't wrap it again which messes with pointer events --- .../core/compactSelect/gridList/option.tsx | 32 +++++++++---------- .../core/compactSelect/listBox/option.tsx | 30 ++++++++--------- 2 files changed, 29 insertions(+), 33 deletions(-) diff --git a/static/app/components/core/compactSelect/gridList/option.tsx b/static/app/components/core/compactSelect/gridList/option.tsx index 671d40feceeaed..959d51597a6468 100644 --- a/static/app/components/core/compactSelect/gridList/option.tsx +++ b/static/app/components/core/compactSelect/gridList/option.tsx @@ -89,27 +89,25 @@ export function GridListOption({node, listState, size}: GridListOptionProps) { ? leadingItems({disabled: isDisabled, isFocused, isSelected}) : leadingItems; - if (hideCheck && !leading) { - return null; + if (hideCheck) { + return leading; } return ( - {!hideCheck && ( - - {multiple ? ( - - ) : ( - isSelected && - )} - - )} + + {multiple ? ( + + ) : ( + isSelected && + )} + {leading ? {leading} : null} ); diff --git a/static/app/components/core/compactSelect/listBox/option.tsx b/static/app/components/core/compactSelect/listBox/option.tsx index 4faec79c11666a..afa64486a25c2e 100644 --- a/static/app/components/core/compactSelect/listBox/option.tsx +++ b/static/app/components/core/compactSelect/listBox/option.tsx @@ -78,26 +78,24 @@ export function ListBoxOption({ ? leadingItems({disabled: isDisabled, isFocused, isSelected}) : leadingItems; - if (hideCheck && !leading) { - return null; + if (hideCheck) { + return leading; } return ( - {!hideCheck && ( - - )} + {leading ? : null} );