Browse Source

feat(ddm): Add widget selection (#61242)

Add ddm context with widget selection state.
Add empty state to tray.
Memoize props and some components to improve performance.

- part of https://github.com/getsentry/sentry/issues/60076
ArthurKnaus 1 year ago
parent
commit
c5e0a5a20a

+ 7 - 1
static/app/utils/formatters.spec.tsx

@@ -401,7 +401,6 @@ describe('parseLargestSuffix', () => {
 
 describe('formatNumberWithDynamicDecimals', () => {
   it('rounds to two decimal points without forcing them', () => {
-    expect(formatNumberWithDynamicDecimalPoints(0)).toEqual('0');
     expect(formatNumberWithDynamicDecimalPoints(1)).toEqual('1');
     expect(formatNumberWithDynamicDecimalPoints(1.0)).toEqual('1');
     expect(formatNumberWithDynamicDecimalPoints(1.5)).toEqual('1.5');
@@ -417,4 +416,11 @@ describe('formatNumberWithDynamicDecimals', () => {
     expect(formatNumberWithDynamicDecimalPoints(0.000125)).toEqual('0.00013');
     expect(formatNumberWithDynamicDecimalPoints(0.0000123)).toEqual('0.000012');
   });
+
+  it('handles zero, NaN and Infinity', () => {
+    expect(formatNumberWithDynamicDecimalPoints(0)).toEqual('0');
+    expect(formatNumberWithDynamicDecimalPoints(NaN)).toEqual('NaN');
+    expect(formatNumberWithDynamicDecimalPoints(Infinity)).toEqual('∞');
+    expect(formatNumberWithDynamicDecimalPoints(-Infinity)).toEqual('-∞');
+  });
 });

+ 2 - 2
static/app/utils/formatters.tsx

@@ -433,8 +433,8 @@ export function formatAbbreviatedNumber(
  * @param value number to format
  */
 export function formatNumberWithDynamicDecimalPoints(value: number): string {
-  if (value === 0) {
-    return '0';
+  if ([0, Infinity, -Infinity, NaN].includes(value)) {
+    return value.toLocaleString();
   }
 
   const exponent = Math.floor(Math.log10(value));

+ 0 - 1
static/app/utils/metrics/index.tsx

@@ -78,7 +78,6 @@ export interface MetricWidgetQueryParams
   extends Pick<MetricsQuery, 'mri' | 'op' | 'query' | 'groupBy'> {
   displayType: MetricDisplayType;
   focusedSeries?: string;
-  position?: number;
   powerUserMode?: boolean;
   showSummaryTable?: boolean;
   sort?: SortState;

+ 3 - 1
static/app/views/ddm/constants.tsx

@@ -1,8 +1,10 @@
+import {SortState} from 'sentry/utils/metrics';
+
 export const DDM_CHART_GROUP = 'ddm_chart_group';
 
 export const MIN_WIDGET_WIDTH = 400;
 
-export const DEFAULT_SORT_STATE = {
+export const DEFAULT_SORT_STATE: SortState = {
   name: undefined,
   order: 'asc',
 };

+ 124 - 0
static/app/views/ddm/context.tsx

@@ -0,0 +1,124 @@
+import {createContext, useCallback, useContext, useMemo, useState} from 'react';
+
+import {MRI} from 'sentry/types';
+import {
+  defaultMetricDisplayType,
+  MetricDisplayType,
+  MetricWidgetQueryParams,
+  updateQuery,
+} from 'sentry/utils/metrics';
+import {decodeList} from 'sentry/utils/queryString';
+import useRouter from 'sentry/utils/useRouter';
+import {DEFAULT_SORT_STATE} from 'sentry/views/ddm/constants';
+
+interface DDMContextValue {
+  addWidget: () => void;
+  selectedWidgetIndex: number;
+  setSelectedWidgetIndex: (index: number) => void;
+  updateWidget: (index: number, data: Partial<MetricWidgetQueryParams>) => void;
+  widgets: MetricWidgetQueryParams[];
+}
+
+export const DDMContext = createContext<DDMContextValue>({
+  selectedWidgetIndex: 0,
+  setSelectedWidgetIndex: () => {},
+  addWidget: () => {},
+  updateWidget: () => {},
+  widgets: [],
+});
+
+export function useDDMContext() {
+  return useContext(DDMContext);
+}
+
+const emptyWidget: MetricWidgetQueryParams = {
+  mri: '' as MRI,
+  op: undefined,
+  query: '',
+  groupBy: [],
+  sort: DEFAULT_SORT_STATE,
+  displayType: MetricDisplayType.LINE,
+};
+
+export function useMetricWidgets() {
+  const router = useRouter();
+
+  const widgets = useMemo<MetricWidgetQueryParams[]>(() => {
+    const currentWidgets = JSON.parse(
+      router.location.query.widgets ?? JSON.stringify([emptyWidget])
+    );
+
+    return currentWidgets.map((widget: MetricWidgetQueryParams) => {
+      return {
+        mri: widget.mri,
+        op: widget.op,
+        query: widget.query,
+        groupBy: decodeList(widget.groupBy),
+        displayType: widget.displayType ?? defaultMetricDisplayType,
+        focusedSeries: widget.focusedSeries,
+        showSummaryTable: widget.showSummaryTable ?? true, // temporary default
+        powerUserMode: widget.powerUserMode,
+        sort: widget.sort ?? DEFAULT_SORT_STATE,
+      };
+    });
+  }, [router.location.query.widgets]);
+
+  const updateWidget = useCallback(
+    (index: number, data: Partial<MetricWidgetQueryParams>) => {
+      const widgetsCopy = [...widgets];
+      widgetsCopy[index] = {...widgets[index], ...data};
+
+      updateQuery(router, {
+        widgets: JSON.stringify(widgetsCopy),
+      });
+    },
+    [widgets, router]
+  );
+
+  const addWidget = useCallback(() => {
+    const widgetsCopy = [...widgets];
+    widgetsCopy.push(emptyWidget);
+
+    updateQuery(router, {
+      widgets: JSON.stringify(widgetsCopy),
+    });
+  }, [widgets, router]);
+
+  return {
+    widgets,
+    updateWidget,
+    addWidget,
+  };
+}
+
+export function DDMContextProvider({children}: {children: React.ReactNode}) {
+  const [selectedWidgetIndex, setSelectedWidgetIndex] = useState(0);
+  const {widgets, updateWidget, addWidget} = useMetricWidgets();
+
+  const handleAddWidget = useCallback(() => {
+    addWidget();
+    setSelectedWidgetIndex(widgets.length);
+  }, [addWidget, widgets.length]);
+
+  const handleUpdateWidget = useCallback(
+    (index: number, data: Partial<MetricWidgetQueryParams>) => {
+      updateWidget(index, data);
+      setSelectedWidgetIndex(index);
+    },
+    [updateWidget]
+  );
+
+  const contextValue = useMemo<DDMContextValue>(
+    () => ({
+      addWidget: handleAddWidget,
+      selectedWidgetIndex:
+        selectedWidgetIndex > widgets.length - 1 ? 0 : selectedWidgetIndex,
+      setSelectedWidgetIndex,
+      updateWidget: handleUpdateWidget,
+      widgets,
+    }),
+    [handleAddWidget, handleUpdateWidget, selectedWidgetIndex, widgets]
+  );
+
+  return <DDMContext.Provider value={contextValue}>{children}</DDMContext.Provider>;
+}

+ 4 - 1
static/app/views/ddm/ddm.tsx

@@ -5,6 +5,7 @@ import SentryDocumentTitle from 'sentry/components/sentryDocumentTitle';
 import {t} from 'sentry/locale';
 import {trackAnalytics} from 'sentry/utils/analytics';
 import useOrganization from 'sentry/utils/useOrganization';
+import {DDMContextProvider} from 'sentry/views/ddm/context';
 import {DDMLayout} from 'sentry/views/ddm/layout';
 
 function DDM() {
@@ -20,7 +21,9 @@ function DDM() {
   return (
     <SentryDocumentTitle title={t('DDM')} orgSlug={organization.slug}>
       <PageFiltersContainer disablePersistence>
-        <DDMLayout />
+        <DDMContextProvider>
+          <DDMLayout />
+        </DDMContextProvider>
       </PageFiltersContainer>
     </SentryDocumentTitle>
   );

+ 6 - 6
static/app/views/ddm/layout.tsx

@@ -1,4 +1,4 @@
-import {Fragment, useRef} from 'react';
+import {Fragment, memo, useRef} from 'react';
 import styled from '@emotion/styled';
 
 import ButtonBar from 'sentry/components/buttonBar';
@@ -66,7 +66,7 @@ function MainContent({showTraceTable}: {showTraceTable?: boolean}) {
   );
 }
 
-export function DDMLayout() {
+export const DDMLayout = memo(() => {
   const organization = useOrganization();
   const hasNewLayout = hasDDMExperimentalFeature(organization);
 
@@ -85,9 +85,9 @@ export function DDMLayout() {
   return (
     <FullViewport ref={measureRef}>
       {
-        // FullViewport as a grid layout with `grid-template-rows: auto 1fr;`
-        // therefore we need the empty div so that SplitPanel can span the whole height */
-        // TODO(arthur): Check on the styles of FullViewport */
+        // FullViewport has a grid layout with `grid-template-rows: auto 1fr;`
+        // therefore we need the empty div so that SplitPanel can span the whole height
+        // TODO(arthur): Check on the styles of FullViewport
       }
       <div />
       {hasSize && (
@@ -109,7 +109,7 @@ export function DDMLayout() {
       )}
     </FullViewport>
   );
-}
+});
 
 const ScrollingPage = styled(Layout.Page)`
   height: 100%;

+ 9 - 8
static/app/views/ddm/metricsExplorer.tsx

@@ -1,21 +1,19 @@
 import {useState} from 'react';
 
 import {MRI} from 'sentry/types';
-import {MetricDisplayType} from 'sentry/utils/metrics';
-import {MetricWidget, MetricWidgetProps} from 'sentry/views/ddm/widget';
+import {MetricDisplayType, MetricWidgetQueryParams} from 'sentry/utils/metrics';
+import {MetricWidget} from 'sentry/views/ddm/widget';
 
 // TODO(ddm): move this to admin
 export default function MetricsExplorer() {
-  const [widget, setWidget] = useState<MetricWidgetProps>({
+  const [widget, setWidget] = useState<MetricWidgetQueryParams>({
     mri: '' as MRI,
     op: undefined,
     query: '',
     groupBy: [],
     displayType: MetricDisplayType.LINE,
-    position: 0,
     powerUserMode: true,
     showSummaryTable: true,
-    onChange: () => {},
     sort: {name: 'name', order: 'asc'},
   });
 
@@ -23,10 +21,13 @@ export default function MetricsExplorer() {
     <MetricWidget
       widget={{
         ...widget,
-        onChange: data => {
-          setWidget(curr => ({...curr, ...data}));
-        },
       }}
+      isSelected={false}
+      onSelect={() => {}}
+      onChange={(_, data) => {
+        setWidget(curr => ({...curr, ...data}));
+      }}
+      index={0}
       datetime={{
         start: null,
         end: null,

+ 6 - 1
static/app/views/ddm/queryBuilder.tsx

@@ -34,6 +34,10 @@ type QueryBuilderProps = {
   powerUserMode?: boolean;
 };
 
+function stopPropagation(e: React.MouseEvent) {
+  e.stopPropagation();
+}
+
 export function QueryBuilder({
   metricsQuery,
   projects,
@@ -174,7 +178,8 @@ export function QueryBuilder({
           />
         </WrapPageFilterBar>
       </QueryBuilderRow>
-      <QueryBuilderRow>
+      {/* Stop propagation so widget does not get selected immediately */}
+      <QueryBuilderRow onClick={stopPropagation}>
         <MetricSearchBar
           // TODO(aknaus): clean up projectId type in ddm
           projectIds={projects.map(id => id.toString())}

+ 23 - 10
static/app/views/ddm/scratchpad.tsx

@@ -1,3 +1,4 @@
+import {useCallback} from 'react';
 import styled from '@emotion/styled';
 import * as echarts from 'echarts/core';
 
@@ -6,17 +7,28 @@ import Panel from 'sentry/components/panels/panel';
 import {IconAdd} from 'sentry/icons';
 import {space} from 'sentry/styles/space';
 import {trackAnalytics} from 'sentry/utils/analytics';
+import {MetricWidgetQueryParams} from 'sentry/utils/metrics';
+import {hasDDMExperimentalFeature} from 'sentry/utils/metrics/features';
 import useOrganization from 'sentry/utils/useOrganization';
 import usePageFilters from 'sentry/utils/usePageFilters';
 import {DDM_CHART_GROUP, MIN_WIDGET_WIDTH} from 'sentry/views/ddm/constants';
+import {useDDMContext} from 'sentry/views/ddm/context';
 
-import {MetricWidget, useMetricWidgets} from './widget';
+import {MetricWidget} from './widget';
 
 export function MetricScratchpad() {
-  const {widgets, onChange, addWidget} = useMetricWidgets();
+  const {setSelectedWidgetIndex, selectedWidgetIndex, widgets, updateWidget, addWidget} =
+    useDDMContext();
   const {selection} = usePageFilters();
   const organization = useOrganization();
 
+  const handleChange = useCallback(
+    (index: number, widget: Partial<MetricWidgetQueryParams>) => {
+      updateWidget(index, widget);
+    },
+    [updateWidget]
+  );
+
   const Wrapper =
     widgets.length === 1 ? StyledSingleWidgetWrapper : StyledMetricDashboard;
 
@@ -24,15 +36,16 @@ export function MetricScratchpad() {
 
   return (
     <Wrapper>
-      {widgets.map(widget => (
+      {widgets.map((widget, index) => (
         <MetricWidget
-          key={widget.position}
-          widget={{
-            ...widget,
-            onChange: data => {
-              onChange(widget.position, data);
-            },
-          }}
+          key={index}
+          index={index}
+          onSelect={setSelectedWidgetIndex}
+          isSelected={
+            hasDDMExperimentalFeature(organization) && selectedWidgetIndex === index
+          }
+          onChange={handleChange}
+          widget={widget}
           datetime={selection.datetime}
           projects={selection.projects}
           environments={selection.environments}

Some files were not shown because too many files changed in this diff