Browse Source

feat(discover): Use tabs for dataset selection (#76732)

Removes the dataset selector implementation with `CompactSelect` and
replaces it with tabs and adds the tabs to the results header so it sits
above the body content.
Nar Saynorath 6 months ago
parent
commit
6b991128c7

+ 1 - 1
static/app/views/dashboards/widgetBuilder/buildSteps/dataSetStep.tsx

@@ -13,7 +13,7 @@ import {DatasetSource} from 'sentry/utils/discover/types';
 import useOrganization from 'sentry/utils/useOrganization';
 import {DisplayType, type WidgetType} from 'sentry/views/dashboards/types';
 import {hasDatasetSelector} from 'sentry/views/dashboards/utils';
-import {DATASET_LABEL_MAP} from 'sentry/views/discover/savedQuery/datasetSelector';
+import {DATASET_LABEL_MAP} from 'sentry/views/discover/savedQuery/datasetSelectorTabs';
 
 import {DataSet} from '../utils';
 

+ 5 - 3
static/app/views/discover/homepage.spec.tsx

@@ -580,7 +580,10 @@ describe('Discover > Homepage', () => {
 
     expect(screen.getAllByTestId('grid-head-cell').length).toEqual(1);
     screen.getByText('event.type:error');
-    expect(screen.getByRole('button', {name: 'Dataset Errors'})).toBeInTheDocument();
+    expect(screen.getByRole('tab', {name: 'Errors'})).toHaveAttribute(
+      'aria-selected',
+      'true'
+    );
     expect(
       screen.getByText(
         "We're splitting our datasets up to make it a bit easier to digest. We defaulted this query to Errors. Edit as you see fit."
@@ -654,8 +657,7 @@ describe('Discover > Homepage', () => {
     expect(await screen.findByText('Remove Default')).toBeInTheDocument();
     expect(screen.queryByText('Set as Default')).not.toBeInTheDocument();
 
-    await userEvent.click(screen.getByText('Dataset'));
-    await userEvent.click(screen.getByRole('option', {name: 'Transactions'}));
+    await userEvent.click(screen.getByRole('tab', {name: 'Transactions'}));
 
     expect(initialData.router.push).toHaveBeenCalledWith(
       expect.objectContaining({

+ 8 - 4
static/app/views/discover/results.spec.tsx

@@ -1545,9 +1545,10 @@ describe('Results', function () {
       });
       expect(mockRequests.eventsResultsMock).toHaveBeenCalledTimes(1);
       await waitFor(() => {
-        expect(
-          screen.getByRole('button', {name: 'Dataset Transactions'})
-        ).toBeInTheDocument();
+        expect(screen.getByRole('tab', {name: 'Transactions'})).toHaveAttribute(
+          'aria-selected',
+          'true'
+        );
       });
 
       expect(
@@ -1622,7 +1623,10 @@ describe('Results', function () {
       });
       expect(mockRequests.eventsResultsMock).toHaveBeenCalledTimes(1);
 
-      expect(screen.getByRole('button', {name: 'Dataset Errors'})).toBeInTheDocument();
+      expect(screen.getByRole('tab', {name: 'Errors'})).toHaveAttribute(
+        'aria-selected',
+        'true'
+      );
 
       expect(mockRequests.eventsStatsMock).toHaveBeenCalledWith(
         '/organizations/org-slug/events-stats/',

+ 2 - 20
static/app/views/discover/results.tsx

@@ -11,7 +11,6 @@ import {fetchTotalCount} from 'sentry/actionCreators/events';
 import {fetchProjectsCount} from 'sentry/actionCreators/projects';
 import {loadOrganizationTags} from 'sentry/actionCreators/tags';
 import {Client} from 'sentry/api';
-import Feature from 'sentry/components/acl/feature';
 import {Alert} from 'sentry/components/alert';
 import {Button} from 'sentry/components/button';
 import Confirm from 'sentry/components/confirm';
@@ -62,10 +61,7 @@ import withApi from 'sentry/utils/withApi';
 import withOrganization from 'sentry/utils/withOrganization';
 import withPageFilters from 'sentry/utils/withPageFilters';
 import {hasDatasetSelector} from 'sentry/views/dashboards/utils';
-import {
-  DATASET_LABEL_MAP,
-  DatasetSelector,
-} from 'sentry/views/discover/savedQuery/datasetSelector';
+import {DATASET_LABEL_MAP} from 'sentry/views/discover/savedQuery/datasetSelectorTabs';
 import {
   getDatasetFromLocationOrSavedQueryDataset,
   getSavedQueryDataset,
@@ -762,6 +758,7 @@ export class Results extends Component<Props, State> {
             yAxis={yAxisArray}
             router={router}
             isHomepage={isHomepage}
+            splitDecision={splitDecision}
           />
           <Layout.Body>
             <CustomMeasurementsProvider organization={organization} selection={selection}>
@@ -773,21 +770,6 @@ export class Results extends Component<Props, State> {
                 {!hasDatasetSelectorFeature && <SampleDataAlert query={query} />}
 
                 <Wrapper>
-                  <Feature
-                    organization={organization}
-                    features="performance-discover-dataset-selector"
-                  >
-                    {({hasFeature}) =>
-                      hasFeature && (
-                        <DatasetSelector
-                          isHomepage={isHomepage}
-                          savedQuery={savedQuery}
-                          splitDecision={splitDecision}
-                          eventView={eventView}
-                        />
-                      )
-                    }
-                  </Feature>
                   <PageFilterBar condensed>
                     <ProjectPageFilter />
                     <EnvironmentPageFilter />

+ 19 - 0
static/app/views/discover/resultsHeader.tsx

@@ -16,7 +16,9 @@ import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
 import type {Organization, SavedQuery} from 'sentry/types/organization';
 import EventView from 'sentry/utils/discover/eventView';
+import type {SavedQueryDatasets} from 'sentry/utils/discover/types';
 import withApi from 'sentry/utils/withApi';
+import {DatasetSelectorTabs} from 'sentry/views/discover/savedQuery/datasetSelectorTabs';
 import {getSavedQueryWithDataset} from 'sentry/views/discover/savedQuery/utils';
 
 import Banner from './banner';
@@ -35,6 +37,7 @@ type Props = {
   setSavedQuery: (savedQuery?: SavedQuery) => void;
   yAxis: string[];
   isHomepage?: boolean;
+  splitDecision?: SavedQueryDatasets;
 };
 
 type State = {
@@ -153,6 +156,7 @@ class ResultsHeader extends Component<Props, State> {
       router,
       setSavedQuery,
       isHomepage,
+      splitDecision,
     } = this.props;
     const {savedQuery, loading, homepageQuery} = this.state;
 
@@ -220,6 +224,21 @@ class ResultsHeader extends Component<Props, State> {
           />
         </Layout.HeaderActions>
         {isHomepage && this.renderBanner()}
+        <Feature
+          organization={organization}
+          features="performance-discover-dataset-selector"
+        >
+          {({hasFeature}) =>
+            hasFeature && (
+              <DatasetSelectorTabs
+                eventView={eventView}
+                isHomepage={isHomepage}
+                savedQuery={savedQuery}
+                splitDecision={splitDecision}
+              />
+            )
+          }
+        </Feature>
       </Layout.Header>
     );
   }

+ 22 - 17
static/app/views/discover/savedQuery/datasetSelector.spec.tsx → static/app/views/discover/savedQuery/datasetSelectorTabs.spec.tsx

@@ -3,7 +3,7 @@ import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
 
 import EventView, {type EventViewOptions} from 'sentry/utils/discover/eventView';
 import {DiscoverDatasets} from 'sentry/utils/discover/types';
-import {DatasetSelector} from 'sentry/views/discover/savedQuery/datasetSelector';
+import {DatasetSelectorTabs} from 'sentry/views/discover/savedQuery/datasetSelectorTabs';
 
 const EVENT_VIEW_CONSTRUCTOR_PROPS: EventViewOptions = {
   createdBy: undefined,
@@ -27,33 +27,35 @@ describe('Discover DatasetSelector', function () {
     organization: {features: ['performance-view']},
   });
 
-  it('renders selector and options', async function () {
+  it('renders tabs', function () {
     const eventView = new EventView(EVENT_VIEW_CONSTRUCTOR_PROPS);
     render(
-      <DatasetSelector isHomepage={false} savedQuery={undefined} eventView={eventView} />,
+      <DatasetSelectorTabs
+        isHomepage={false}
+        savedQuery={undefined}
+        eventView={eventView}
+      />,
       {
         router,
       }
     );
-    await userEvent.click(screen.getByText('Dataset'));
-    const menuOptions = await screen.findAllByRole('option');
-    expect(menuOptions.map(e => e.textContent)).toEqual([
-      'Errors',
-      'Transactions',
-      'Unknown',
-    ]);
+    expect(screen.getByRole('tab', {name: 'Errors'})).toBeInTheDocument();
+    expect(screen.getByRole('tab', {name: 'Transactions'})).toBeInTheDocument();
   });
 
-  it('pushes new dafault event view if not a saved query', async function () {
+  it('pushes new default event view if not a saved query', async function () {
     const eventView = new EventView(EVENT_VIEW_CONSTRUCTOR_PROPS);
     render(
-      <DatasetSelector isHomepage={false} savedQuery={undefined} eventView={eventView} />,
+      <DatasetSelectorTabs
+        isHomepage={false}
+        savedQuery={undefined}
+        eventView={eventView}
+      />,
       {
         router,
       }
     );
-    await userEvent.click(screen.getByText('Dataset'));
-    await userEvent.click(screen.getByRole('option', {name: 'Transactions'}));
+    await userEvent.click(screen.getByRole('tab', {name: 'Transactions'}));
     expect(router.push).toHaveBeenCalledWith(
       expect.objectContaining({
         query: expect.objectContaining({
@@ -73,13 +75,16 @@ describe('Discover DatasetSelector', function () {
       id: '1',
     });
     render(
-      <DatasetSelector isHomepage={false} savedQuery={undefined} eventView={eventView} />,
+      <DatasetSelectorTabs
+        isHomepage={false}
+        savedQuery={undefined}
+        eventView={eventView}
+      />,
       {
         router,
       }
     );
-    await userEvent.click(screen.getByText('Dataset'));
-    await userEvent.click(screen.getByRole('option', {name: 'Transactions'}));
+    await userEvent.click(screen.getByRole('tab', {name: 'Transactions'}));
     expect(router.push).toHaveBeenCalledWith(
       expect.objectContaining({
         query: expect.objectContaining({

+ 12 - 11
static/app/views/discover/savedQuery/datasetSelector.tsx → static/app/views/discover/savedQuery/datasetSelectorTabs.tsx

@@ -1,4 +1,4 @@
-import {CompactSelect} from 'sentry/components/compactSelect';
+import {TabList, Tabs} from 'sentry/components/tabs';
 import {t} from 'sentry/locale';
 import type {SavedQuery} from 'sentry/types/organization';
 import type EventView from 'sentry/utils/discover/eventView';
@@ -26,7 +26,7 @@ type Props = {
   splitDecision?: SavedQueryDatasets;
 };
 
-export function DatasetSelector(props: Props) {
+export function DatasetSelectorTabs(props: Props) {
   const {savedQuery, isHomepage, splitDecision, eventView} = props;
   const location = useLocation();
   const organization = useOrganization();
@@ -53,16 +53,11 @@ export function DatasetSelector(props: Props) {
   }
 
   return (
-    <CompactSelect
-      triggerProps={{prefix: t('Dataset')}}
+    <Tabs
       value={value}
-      options={options}
       onChange={newValue => {
-        if (newValue.value === value) {
-          return;
-        }
         const nextEventView = eventView.withDataset(
-          getDatasetFromLocationOrSavedQueryDataset(undefined, newValue.value)
+          getDatasetFromLocationOrSavedQueryDataset(undefined, newValue)
         );
         const nextLocation = nextEventView.getResultsViewUrlTarget(
           organization.slug,
@@ -72,10 +67,16 @@ export function DatasetSelector(props: Props) {
           ...location,
           query: {
             ...nextLocation.query,
-            [DATASET_PARAM]: newValue.value,
+            [DATASET_PARAM]: newValue,
           },
         });
       }}
-    />
+    >
+      <TabList variant="filled" hideBorder>
+        {options.map(option => (
+          <TabList.Item key={option.value}>{option.label}</TabList.Item>
+        ))}
+      </TabList>
+    </Tabs>
   );
 }

+ 1 - 1
static/app/views/discover/savedQuery/utils.tsx

@@ -24,7 +24,7 @@ import {
 import {decodeScalar} from 'sentry/utils/queryString';
 import {DisplayType} from 'sentry/views/dashboards/types';
 import {hasDatasetSelector} from 'sentry/views/dashboards/utils';
-import {DATASET_PARAM} from 'sentry/views/discover/savedQuery/datasetSelector';
+import {DATASET_PARAM} from 'sentry/views/discover/savedQuery/datasetSelectorTabs';
 
 export function handleCreateQuery(
   api: Client,