Browse Source

fix(discover): Fix discover yaxis dropdown options disabled in top display mode (#34213)

YAxis dropdown options in Discover were being disabled. This fixes it by changing the selector type from multi to single when in a Top display mode.
edwardgou-sentry 2 years ago
parent
commit
57d9b6e729

+ 16 - 7
static/app/views/eventsV2/chartFooter.tsx

@@ -68,13 +68,22 @@ export default function ChartFooter({
             onChange={onTopEventsChange}
           />
         )}
-        <OptionSelector
-          multiple
-          title={t('Y-Axis')}
-          selected={yAxisValue}
-          options={yAxisOptions}
-          onChange={onAxisChange}
-        />
+        {TOP_EVENT_MODES.includes(displayMode) ? (
+          <OptionSelector
+            title={t('Y-Axis')}
+            selected={yAxisValue[0]}
+            options={yAxisOptions}
+            onChange={yAxis => onAxisChange([yAxis])}
+          />
+        ) : (
+          <OptionSelector
+            multiple
+            title={t('Y-Axis')}
+            selected={yAxisValue}
+            options={yAxisOptions}
+            onChange={onAxisChange}
+          />
+        )}
       </InlineContainer>
     </ChartControls>
   );

+ 3 - 24
static/app/views/eventsV2/resultsChart.tsx

@@ -20,6 +20,7 @@ import {isEquation, stripEquationPrefix} from 'sentry/utils/discover/fields';
 import {
   DisplayModes,
   MULTI_Y_AXIS_SUPPORTED_DISPLAY_MODES,
+  TOP_EVENT_MODES,
   TOP_N,
 } from 'sentry/utils/discover/types';
 import getDynamicText from 'sentry/utils/getDynamicText';
@@ -193,21 +194,14 @@ class ResultsChartContainer extends Component<ContainerProps> {
         // top5 modes are only available with larger packages in saas.
         // We remove instead of disable here as showing tooltips in dropdown
         // menus is clunky.
-        if (
-          [DisplayModes.TOP5, DisplayModes.DAILYTOP5].includes(
-            opt.value as DisplayModes
-          ) &&
-          !hasQueryFeature
-        ) {
+        if (TOP_EVENT_MODES.includes(opt.value) && !hasQueryFeature) {
           return false;
         }
         return true;
       })
       .map(opt => {
         // Can only use default display or total daily with multi y axis
-        if (
-          [DisplayModes.TOP5, DisplayModes.DAILYTOP5].includes(opt.value as DisplayModes)
-        ) {
+        if (TOP_EVENT_MODES.includes(opt.value)) {
           opt.label = DisplayModes.TOP5 === opt.value ? 'Top Period' : 'Top Daily';
         }
         if (
@@ -226,21 +220,6 @@ class ResultsChartContainer extends Component<ContainerProps> {
       });
 
     let yAxisOptions = eventView.getYAxisOptions();
-    // Hide multi y axis checkbox when in an unsupported Display Mode
-    if (
-      !MULTI_Y_AXIS_SUPPORTED_DISPLAY_MODES.includes(
-        eventView.getDisplayMode() as DisplayModes
-      )
-    ) {
-      yAxisOptions = yAxisOptions.map(option => {
-        return {
-          ...option,
-          disabled: true,
-          tooltip: t('Multiple Y-Axis cannot be plotted on this Display mode.'),
-          checkboxHidden: true,
-        };
-      });
-    }
     // Equations on World Map isn't supported on the events-geo endpoint
     // Disabling equations as an option to prevent erroring out
     if (eventView.getDisplayMode() === DisplayModes.WORLDMAP) {

+ 53 - 0
tests/js/spec/views/eventsV2/chartFooter.spec.tsx

@@ -1,5 +1,6 @@
 import {mountWithTheme} from 'sentry-test/enzyme';
 import {initializeOrg} from 'sentry-test/initializeOrg';
+import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
 
 import {t} from 'sentry/locale';
 import {DisplayModes} from 'sentry/utils/discover/types';
@@ -90,4 +91,56 @@ describe('EventsV2 > ChartFooter', function () {
     const optionSelector = wrapper.find('OptionSelector[title="Limit"]');
     expect(optionSelector.props().selected).toEqual('5');
   });
+
+  it('renders single value y-axis dropdown selector on a Top display', async function () {
+    const organization = TestStubs.Organization({
+      features,
+    });
+    let yAxis = ['count()'];
+
+    render(
+      <ChartFooter
+        organization={organization}
+        total={100}
+        yAxisValue={yAxis}
+        yAxisOptions={yAxisOptions}
+        onAxisChange={newYAxis => (yAxis = newYAxis)}
+        displayMode={DisplayModes.TOP5}
+        displayOptions={[{label: DisplayModes.TOP5, value: DisplayModes.TOP5}]}
+        onDisplayChange={() => undefined}
+        onTopEventsChange={() => undefined}
+        topEvents="5"
+      />
+    );
+
+    userEvent.click(screen.getByText('count()'));
+    userEvent.click(screen.getByText('failure_count()'));
+    expect(yAxis).toEqual(['failure_count()']);
+  });
+
+  it('renders multi value y-axis dropdown selector on a non-Top display', async function () {
+    const organization = TestStubs.Organization({
+      features,
+    });
+    let yAxis = ['count()'];
+
+    render(
+      <ChartFooter
+        organization={organization}
+        total={100}
+        yAxisValue={yAxis}
+        yAxisOptions={yAxisOptions}
+        onAxisChange={newYAxis => (yAxis = newYAxis)}
+        displayMode={DisplayModes.DEFAULT}
+        displayOptions={[{label: DisplayModes.DEFAULT, value: DisplayModes.DEFAULT}]}
+        onDisplayChange={() => undefined}
+        onTopEventsChange={() => undefined}
+        topEvents="5"
+      />
+    );
+
+    userEvent.click(screen.getByText('count()'));
+    userEvent.click(screen.getByText('failure_count()'));
+    expect(yAxis).toEqual(['count()', 'failure_count()']);
+  });
 });

+ 0 - 25
tests/js/spec/views/eventsV2/resultsChart.spec.tsx

@@ -83,31 +83,6 @@ describe('EventsV2 > ResultsChart', function () {
     );
   });
 
-  it('disables other y-axis options when not in default, daily, previous period, or bar display mode', function () {
-    eventView.display = DisplayModes.WORLDMAP;
-    const wrapper = mountWithTheme(
-      <ResultsChart
-        router={TestStubs.router()}
-        organization={organization}
-        eventView={eventView}
-        location={location}
-        onAxisChange={() => undefined}
-        onDisplayChange={() => undefined}
-        total={1}
-        confirmedQuery
-        yAxis={['count()']}
-        onTopEventsChange={() => {}}
-      />,
-      initialData.routerContext
-    );
-    const yAxisOptions = wrapper.find('ChartFooter').props().yAxisOptions;
-    yAxisOptions.forEach(({value, disabled}) => {
-      if (value !== 'count()') {
-        expect(disabled).toBe(true);
-      }
-    });
-  });
-
   it('disables equation y-axis options when in World Map display mode', function () {
     eventView.display = DisplayModes.WORLDMAP;
     eventView.fields = [