fix(dashboards): Move global filter loading spinner to dropdown footer

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();

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);
 

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 ? (
+    <Flex justify="center" padding="xs">
+      <FooterLoadingIndicator size={14} />
+    </Flex>
+  ) : null;
+
   if (!canSelectMultipleValues) {
     return (
       <CompactSelect
@@ -360,6 +366,7 @@ export function FilterSelector({
         onClose={() => {
           setStagedFilterValues([]);
         }}
+        menuFooter={loadingFooter}
         menuTitle={
           <MenuTitleWrapper>
             {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 ? (
-          <Flex gap="md" align="center" justify="end">
-            <MenuComponents.CancelButton
-              onClick={() => dispatch({type: 'remove staged'})}
-            />
-            <MenuComponents.ApplyButton
-              onClick={() => {
-                dispatch({type: 'remove staged'});
-                handleChange(stagedSelect.value);
-              }}
-            />
+        hasStagedChanges || isFetching ? (
+          <Flex direction="column" gap="md">
+            {loadingFooter}
+            {hasStagedChanges && (
+              <Flex gap="md" align="center" justify="end">
+                <MenuComponents.CancelButton
+                  onClick={() => dispatch({type: 'remove staged'})}
+                />
+                <MenuComponents.ApplyButton
+                  onClick={() => {
+                    dispatch({type: 'remove staged'});
+                    handleChange(stagedSelect.value);
+                  }}
+                />
+              </Flex>
+            )}
           </Flex>
         ) : 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};
 `;

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,20 +15,17 @@ type FilterSelectorTriggerProps = {
   globalFilter: GlobalFilter;
   operator: TermOperator;
   options: Array<SelectOption<string>>;
-  queryResult: UseQueryResult<string[]>;
 };
 
 export function FilterSelectorTrigger({
   globalFilter,
   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}
       </Text>
 
-      {!isFetching &&
-        (isAllSelected ? (
-          <Text variant="primary" bold={false}>
-            {t('All')}
+      {isAllSelected ? (
+        <Text variant="primary" bold={false}>
+          {t('All')}
+        </Text>
+      ) : (
+        <Container minWidth={0} flexShrink={1} flexGrow={0} overflow="hidden">
+          <Text variant="primary" bold={false} ellipsis>
+            {label}
           </Text>
-        ) : (
-          <Container minWidth={0} flexShrink={1} flexGrow={0} overflow="hidden">
-            <Text variant="primary" bold={false} ellipsis>
-              {label}
-            </Text>
-          </Container>
-        ))}
-
-      {isFetching && <InlineLoadingIndicator size={14} />}
+        </Container>
+      )}
 
       {shouldShowBadge && (
         <Container>
@@ -87,10 +77,3 @@ export function FilterSelectorTrigger({
     </Flex>
   );
 }
-
-const InlineLoadingIndicator = styled(LoadingIndicator)`
-  && {
-    margin: 0;
-    margin-left: ${p => p.theme.space.xs};
-  }
-`;