Fix various experiment header issues (G-Research#52)

* Fix rerender issue with no selected params * Fix metrics error bug * Fix select form popper styling * Update createAppModel.ts
vinayan3 · Aug 28, 2024 · 2f907fd · 2f907fd
1 parent d00c169
commit 2f907fd
Show file tree

Hide file tree

Showing 5 changed files with 52 additions and 42 deletions.
diff --git a/src/src/pages/Metrics/components/MetricsBar/MetricsBar.tsx b/src/src/pages/Metrics/components/MetricsBar/MetricsBar.tsx
@@ -35,6 +35,9 @@ function MetricsBar({
   onToggleAllExperiments,
 }: IMetricsBarProps): React.FunctionComponentElement<React.ReactNode> {
   const [popover, setPopover] = React.useState<string>('');
+  const [selectedExperimentNames, setSelectedExperimentNames] = React.useState<
+    string[]
+  >(getSelectedExperimentNames());
 
   const route = useRouteMatch<any>();
 
@@ -48,8 +51,6 @@ function MetricsBar({
   const { data: experimentsData, loading: isExperimentsLoading } =
     experimentsState;
 
-  const selectedExperimentNames = getSelectedExperimentNames();
-
   function handleBookmarkClick(value: string): void {
     setPopover(value);
   }
@@ -65,6 +66,7 @@ function MetricsBar({
 
   function handleExperimentNamesChange(experimentName: string): void {
     onSelectExperimentNamesChange(experimentName);
+    setSelectedExperimentNames(getSelectedExperimentNames());
   }
 
   return (

diff --git a/src/src/pages/Metrics/components/SelectForm/SelectForm.scss b/src/src/pages/Metrics/components/SelectForm/SelectForm.scss
@@ -131,14 +131,15 @@
   margin: 10px;
 }
 
-.Popper__SelectForm__select{
+.Popper__SelectForm__select {
   @extend .Metrics__SelectForm__metric__select;
 }
 
-.Popper__SelectForm__option{
+.Popper__SelectForm__option {
   @extend .Metrics__SelectForm__option !optional;
 }
 
-.Popper__SelectForm__option__label{
+.Popper__SelectForm__option__label {
   @extend .Metrics__SelectForm__option__label !optional;
+  margin-left: $space-xs;
 }
diff --git a/src/src/pages/Params/components/SelectForm/SelectForm.scss b/src/src/pages/Params/components/SelectForm/SelectForm.scss
@@ -117,14 +117,15 @@
   margin: 10px;
 }
 
-.Popper__SelectForm__select{
+.Popper__SelectForm__select {
   @extend .SelectForm__param__select;
 }
 
-.Popper__SelectForm__option{
+.Popper__SelectForm__option {
   @extend .SelectForm__option !optional;
 }
 
-.Popper__SelectForm__option__label{
+.Popper__SelectForm__option__label {
   @extend .SelectForm__option__label !optional;
+  margin-left: $space-xs;
 }
diff --git a/src/src/services/models/explorer/metricsModelMethods.ts b/src/src/services/models/explorer/metricsModelMethods.ts
@@ -96,7 +96,6 @@ import onSelectExperimentNamesChange from 'utils/app/onSelectExperimentNamesChan
 import onMetricVisibilityChange from 'utils/app/onMetricsVisibilityChange';
 import onRowHeightChange from 'utils/app/onRowHeightChange';
 import onRowVisibilityChange from 'utils/app/onRowVisibilityChange';
-import onSelectAdvancedQueryChange from 'utils/app/onSelectAdvancedQueryChange';
 import onSelectRunQueryChange from 'utils/app/onSelectRunQueryChange';
 import onSmoothingChange from 'utils/app/onSmoothingChange';
 import { onTableDiffShow } from 'utils/app/onTableDiffShow';
@@ -304,41 +303,45 @@ function getMetricsAppModelMethods(
     setRequestProgress(model);
     return {
       call: async () => {
-        model.setState({
-          requestStatus: RequestStatusEnum.Pending,
-          queryIsEmpty: false,
-          selectedRows: shouldResetSelectedRows
-            ? {}
-            : model.getState()?.selectedRows,
-        });
-        liveUpdateInstance?.stop().then();
-        try {
-          const stream = await metricsRequestRef.call((detail) => {
-            exceptionHandler({ detail, model });
-            resetModelState(configData, shouldResetSelectedRows!);
+        if (_.isEmpty(configData?.select?.options)) {
+          resetModelState(configData, shouldResetSelectedRows!);
+        } else {
+          model.setState({
+            requestStatus: RequestStatusEnum.Pending,
+            queryIsEmpty: false,
+            selectedRows: shouldResetSelectedRows
+              ? {}
+              : model.getState()?.selectedRows,
           });
-          const runData = await getRunData(stream, (progress) =>
-            setRequestProgress(model, progress),
-          );
-          if (shouldUrlUpdate) {
-            updateURL({ configData, appName });
-          }
-          saveRecentSearches(appName, query);
-          updateData(runData);
-        } catch (ex: Error | any) {
-          if (ex.name === 'AbortError') {
-            // Abort Error
-          } else {
-            // eslint-disable-next-line no-console
-            console.log('Unhandled error: ', ex);
+          liveUpdateInstance?.stop().then();
+          try {
+            const stream = await metricsRequestRef.call((detail) => {
+              exceptionHandler({ detail, model });
+              resetModelState(configData, shouldResetSelectedRows!);
+            });
+            const runData = await getRunData(stream, (progress) =>
+              setRequestProgress(model, progress),
+            );
+            if (shouldUrlUpdate) {
+              updateURL({ configData, appName });
+            }
+            saveRecentSearches(appName, query);
+            updateData(runData);
+          } catch (ex: Error | any) {
+            if (ex.name === 'AbortError') {
+              // Abort Error
+            } else {
+              // eslint-disable-next-line no-console
+              console.log('Unhandled error: ', ex);
+            }
           }
-        }
 
-        liveUpdateInstance?.start({
-          q: query,
-          p: configData?.chart?.densityType,
-          ...(metric && { x_axis: metric }),
-        });
+          liveUpdateInstance?.start({
+            q: query,
+            p: configData?.chart?.densityType,
+            ...(metric && { x_axis: metric }),
+          });
+        }
       },
       abort: metricsRequestRef.abort,
     };

diff --git a/src/src/utils/app/onSelectExperimentNamesChange.ts b/src/src/utils/app/onSelectExperimentNamesChange.ts
@@ -33,7 +33,10 @@ export default function onSelectExperimentNamesChange<M extends State>({
   if (configData?.select) {
     const newConfig = {
       ...configData,
-      select: { ...configData.select, selectedExperimentNames },
+      select: {
+        ...configData.select,
+        selectedExperimentNames,
+      },
     };
     model.setState({ config: newConfig });
   }