diff --git a/static/app/views/dashboards/filtersBar.spec.tsx b/static/app/views/dashboards/filtersBar.spec.tsx index 653c3ca5d9f7c8..6d32ab62e01abd 100644 --- a/static/app/views/dashboards/filtersBar.spec.tsx +++ b/static/app/views/dashboards/filtersBar.spec.tsx @@ -5,12 +5,7 @@ import {OrganizationFixture} from 'sentry-fixture/organization'; import {ReleaseFixture} from 'sentry-fixture/release'; import {TagsFixture} from 'sentry-fixture/tags'; -import { - render, - screen, - waitFor, - waitForElementToBeRemoved, -} from 'sentry-test/reactTestingLibrary'; +import {render, screen, waitFor} from 'sentry-test/reactTestingLibrary'; import type {Organization} from 'sentry/types/organization'; import {FieldKind} from 'sentry/utils/fields'; @@ -63,9 +58,8 @@ describe('FiltersBar', () => { }, }); renderFilterBar({location: newLocation}); - await waitForElementToBeRemoved(() => screen.queryByTestId('loading-indicator')); expect( - screen.getByRole('button', {name: /browser\.name.*Chrome/i}) + await screen.findByRole('button', {name: /browser\.name.*Chrome/i}) ).toBeInTheDocument(); }); @@ -80,8 +74,7 @@ describe('FiltersBar', () => { }, }); renderFilterBar({location: newLocation, hasUnsavedChanges: true}); - await waitForElementToBeRemoved(() => screen.queryByTestId('loading-indicator')); - expect(screen.getByRole('button', {name: 'Save'})).toBeInTheDocument(); + expect(await screen.findByRole('button', {name: 'Save'})).toBeInTheDocument(); expect(screen.getByRole('button', {name: 'Cancel'})).toBeInTheDocument(); }); @@ -98,9 +91,8 @@ describe('FiltersBar', () => { }); renderFilterBar({location: newLocation}); - await waitForElementToBeRemoved(() => screen.queryByTestId('loading-indicator')); expect( - screen.getByRole('button', {name: /browser\.name.*Chrome/i}) + await screen.findByRole('button', {name: /browser\.name.*Chrome/i}) ).toBeInTheDocument(); expect(screen.queryByRole('button', {name: 'Save'})).not.toBeInTheDocument(); expect(screen.queryByRole('button', {name: 'Cancel'})).not.toBeInTheDocument(); @@ -215,9 +207,8 @@ describe('FiltersBar', () => { hasUnsavedChanges: true, prebuiltDashboardId: PrebuiltDashboardId.FRONTEND_SESSION_HEALTH, }); - await waitForElementToBeRemoved(() => screen.queryByTestId('loading-indicator')); expect( - screen.getByRole('button', {name: /browser\.name.*Chrome/i}) + await screen.findByRole('button', {name: /browser\.name.*Chrome/i}) ).toBeInTheDocument(); expect(screen.getByRole('button', {name: 'Save for Everyone'})).toBeInTheDocument(); expect(screen.getByRole('button', {name: 'Cancel'})).toBeInTheDocument(); diff --git a/static/app/views/dashboards/globalFilter/filterSelector.spec.tsx b/static/app/views/dashboards/globalFilter/filterSelector.spec.tsx index 448259dfd665c5..2f4c5be2dcf9a7 100644 --- a/static/app/views/dashboards/globalFilter/filterSelector.spec.tsx +++ b/static/app/views/dashboards/globalFilter/filterSelector.spec.tsx @@ -38,7 +38,7 @@ describe('FilterSelector', () => { ); const button = screen.getByRole('button', { - name: `${mockGlobalFilter.tag.key} contains`, + name: `${mockGlobalFilter.tag.key} contains All`, }); await userEvent.click(button); @@ -58,7 +58,7 @@ describe('FilterSelector', () => { ); const button = screen.getByRole('button', { - name: `${mockGlobalFilter.tag.key} contains`, + name: `${mockGlobalFilter.tag.key} contains All`, }); await userEvent.click(button); @@ -90,7 +90,7 @@ describe('FilterSelector', () => { /> ); - const button = screen.getByRole('button', {name: mockGlobalFilter.tag.key + ' :'}); + const button = screen.getByRole('button', {name: /^browser :/}); await userEvent.click(button); expect(screen.getByRole('checkbox', {name: 'Select firefox'})).toBeChecked(); @@ -108,7 +108,7 @@ describe('FilterSelector', () => { ); const button = screen.getByRole('button', { - name: `${mockGlobalFilter.tag.key} contains`, + name: `${mockGlobalFilter.tag.key} contains All`, }); await userEvent.click(button); await userEvent.click(screen.getByRole('button', {name: 'Remove Filter'})); @@ -209,7 +209,7 @@ describe('FilterSelector', () => { ); const button = screen.getByRole('button', { - name: `${SpanFields.USER_GEO_SUBREGION} contains`, + name: `${SpanFields.USER_GEO_SUBREGION} contains All`, }); await userEvent.click(button); @@ -240,7 +240,7 @@ describe('FilterSelector', () => { ); const button = screen.getByRole('button', { - name: `${mockGlobalFilter.tag.key} contains`, + name: `${mockGlobalFilter.tag.key} contains All`, }); await userEvent.click(button); diff --git a/static/app/views/dashboards/globalFilter/filterSelector.tsx b/static/app/views/dashboards/globalFilter/filterSelector.tsx index ccf727c10e195d..0bf9a24589d4cc 100644 --- a/static/app/views/dashboards/globalFilter/filterSelector.tsx +++ b/static/app/views/dashboards/globalFilter/filterSelector.tsx @@ -16,6 +16,7 @@ import {Flex} from '@sentry/scraps/layout'; import {OverlayTrigger} from '@sentry/scraps/overlayTrigger'; import {DropdownMenu} from 'sentry/components/dropdownMenu'; +import {LoadingIndicator} from 'sentry/components/loadingIndicator'; import {usePageFilters} from 'sentry/components/pageFilters/usePageFilters'; import {useStagedCompactSelect} from 'sentry/components/pageFilters/useStagedCompactSelect'; import { @@ -342,10 +343,15 @@ export function FilterSelector({ activeFilterValues={filterValues} operator={stagedOperator} options={translatedOptions} - queryResult={queryResult} /> ); + const loadingFooter = isFetching ? ( + + + + ) : null; + if (!canSelectMultipleValues) { return ( { setStagedFilterValues([]); }} + menuFooter={loadingFooter} menuTitle={ {t('%s Filter', getDatasetLabel(globalFilter.dataset))} @@ -421,17 +428,22 @@ export function FilterSelector({ isFetching ? t('Loading filter values...') : t('No filter values found') } menuFooter={ - hasStagedChanges ? ( - - dispatch({type: 'remove staged'})} - /> - { - dispatch({type: 'remove staged'}); - handleChange(stagedSelect.value); - }} - /> + hasStagedChanges || isFetching ? ( + + {loadingFooter} + {hasStagedChanges && ( + + dispatch({type: 'remove staged'})} + /> + { + dispatch({type: 'remove staged'}); + handleChange(stagedSelect.value); + }} + /> + + )} ) : null } @@ -517,6 +529,12 @@ export const MenuTitleWrapper = styled('span')` padding-bottom: ${p => p.theme.space.xs}; `; +const FooterLoadingIndicator = styled(LoadingIndicator)` + && { + margin: 0; + } +`; + const OperatorFlex = styled(Flex)` margin-left: -${p => p.theme.space.sm}; `; diff --git a/static/app/views/dashboards/globalFilter/filterSelectorTrigger.tsx b/static/app/views/dashboards/globalFilter/filterSelectorTrigger.tsx index e53dab57a1b736..32960f5711cfe5 100644 --- a/static/app/views/dashboards/globalFilter/filterSelectorTrigger.tsx +++ b/static/app/views/dashboards/globalFilter/filterSelectorTrigger.tsx @@ -1,12 +1,8 @@ -import styled from '@emotion/styled'; -import type {UseQueryResult} from '@tanstack/react-query'; - import {Badge} from '@sentry/scraps/badge'; import type {SelectOption} from '@sentry/scraps/compactSelect'; import {Container, Flex} from '@sentry/scraps/layout'; import {Text} from '@sentry/scraps/text'; -import {LoadingIndicator} from 'sentry/components/loadingIndicator'; import {OP_LABELS} from 'sentry/components/searchQueryBuilder/tokens/filter/utils'; import {TermOperator} from 'sentry/components/searchSyntax/parser'; import {t} from 'sentry/locale'; @@ -19,7 +15,6 @@ type FilterSelectorTriggerProps = { globalFilter: GlobalFilter; operator: TermOperator; options: Array>; - queryResult: UseQueryResult; }; export function FilterSelectorTrigger({ @@ -27,12 +22,10 @@ export function FilterSelectorTrigger({ activeFilterValues, operator, options, - queryResult, }: FilterSelectorTriggerProps) { - const {isFetching} = queryResult; const {tag} = globalFilter; - const shouldShowBadge = !isFetching && activeFilterValues.length > 1; + const shouldShowBadge = activeFilterValues.length > 1; // "All" means no filter is applied (empty selection). We intentionally avoid // comparing against options.length because when tag values fail to load, @@ -64,20 +57,17 @@ export function FilterSelectorTrigger({ {opLabel} - {!isFetching && - (isAllSelected ? ( - - {t('All')} + {isAllSelected ? ( + + {t('All')} + + ) : ( + + + {label} - ) : ( - - - {label} - - - ))} - - {isFetching && } + + )} {shouldShowBadge && ( @@ -87,10 +77,3 @@ export function FilterSelectorTrigger({ ); } - -const InlineLoadingIndicator = styled(LoadingIndicator)` - && { - margin: 0; - margin-left: ${p => p.theme.space.xs}; - } -`;