fix: respond to PR comments

Author: PeterYurkovich

web/src/components/MetricsPage.tsx

@@ -1139,7 +1139,7 @@ const QueryBrowserWrapper: FC<{
     const newParams = new URLSearchParams(queryParams);
     queryStrings.forEach((query, i) => newParams.set(`query${i}`, query || ''));
     if (customDataSourceName) {
-      newParams.set('datasource', customDataSourceName);
+      newParams.set(QueryParams.Datasource, customDataSourceName);
     }
     setQueryParams(newParams);
   }, [queryStrings, customDataSourceName, isFirstRender, queryParams]);

web/src/components/dashboards/legacy/legacy-dashboard.tsx

@@ -72,7 +72,7 @@ const QueryBrowserLink = ({
   }
 
   if (customDataSourceName) {
-    params.set('datasource', customDataSourceName);
+    params.set(QueryParams.Datasource, customDataSourceName);
   }
 
   return (

web/src/components/dashboards/perses/hooks/useDashboardsData.ts

@@ -1,13 +1,14 @@
-import { useMemo, useCallback, useState } from 'react';
+/* eslint-disable react-hooks/refs */
+import { useMemo, useCallback, useRef } from 'react';
 
 import { DashboardResource } from '@perses-dev/core';
-import { useNavigate, useSearchParams } from 'react-router';
 import { StringParam, useQueryParam } from 'use-query-params';
 import { useBoolean } from '../../../hooks/useBoolean';
 import { getDashboardUrl, usePerspective } from '../../../hooks/usePerspective';
 import { QueryParams } from '../../../query-params';
 import { useActiveProject } from '../project/useActiveProject';
 import { usePerses } from './usePerses';
+import { useNavigate, useSearchParams } from 'react-router';
 import { ALL_NAMESPACES_KEY } from '../../../utils';
 
 // This hook syncs with mutliple external API's, redux, and URL state. Its a lot, but needs to all
@@ -40,8 +41,8 @@ export const useDashboardsData = () => {
     return true;
   }, [persesProjectsLoading, persesDashboardsLoading, initialPageLoad, setInitialPageLoadFalse]);
 
-  const [prevDashboards, setPreviousDashboards] = useState<DashboardResource[]>([]);
-  const [prevMetadata, setPreviousMetadata] = useState<CombinedDashboardMetadata[]>([]);
+  const prevDashboardsRef = useRef<DashboardResource[]>([]);
+  const prevMetadataRef = useRef<CombinedDashboardMetadata[]>([]);
 
   // Homogenize data needed for dashboards dropdown between legacy and perses dashboards
   // to enable both to use the same component
@@ -52,18 +53,18 @@ export const useDashboardsData = () => {
 
     // Check if dashboards data has actually changed to avoid recreation
     const dashboardsChanged =
-      persesDashboards.length !== prevDashboards.length ||
+      persesDashboards.length !== prevDashboardsRef.current.length ||
       persesDashboards.some((dashboard, i) => {
-        const prevDashboard = prevDashboards[i];
+        const prevDashboard = prevDashboardsRef.current[i];
         return (
           dashboard?.metadata?.name !== prevDashboard?.metadata?.name ||
           dashboard?.spec?.display?.name !== prevDashboard?.spec?.display?.name ||
           dashboard?.metadata?.project !== prevDashboard?.metadata?.project
         );
       });
 
-    if (!dashboardsChanged && prevMetadata.length > 0) {
-      return prevMetadata;
+    if (!dashboardsChanged && prevMetadataRef.current.length > 0) {
+      return prevMetadataRef.current;
     }
 
     const newMetadata = persesDashboards.map((persesDashboard) => {
@@ -79,17 +80,10 @@ export const useDashboardsData = () => {
       };
     });
 
-    setPreviousDashboards(persesDashboards);
-    setPreviousMetadata(newMetadata);
+    prevDashboardsRef.current = persesDashboards;
+    prevMetadataRef.current = newMetadata;
     return newMetadata;
-  }, [
-    persesDashboards,
-    combinedInitialLoad,
-    prevDashboards,
-    prevMetadata,
-    setPreviousDashboards,
-    setPreviousMetadata,
-  ]);
+  }, [persesDashboards, combinedInitialLoad]);
 
   // Retrieve dashboard metadata for the currently selected project
   const activeProjectDashboardsMetadata = useMemo<CombinedDashboardMetadata[]>(() => {
@@ -107,10 +101,9 @@ export const useDashboardsData = () => {
         // If the board is being cleared then don't do anything
         return;
       }
-
       const params = new URLSearchParams(queryParams);
 
-      params.delete('edit');
+      params.delete(QueryParams.Edit);
 
       const dashboard = combinedDashboardsMetadata.find((item) => item.name === newBoard);
 

web/src/components/dashboards/perses/project/ProjectBar.tsx

@@ -3,6 +3,7 @@ import { KEYBOARD_SHORTCUTS } from './utils';
 import ProjectDropdown from './ProjectDropdown';
 import { getDashboardsListUrl, usePerspective } from '../../../hooks/usePerspective';
 import { useNavigate } from 'react-router';
+import { QueryParams } from '../../../query-params';
 
 export type ProjectBarProps = {
   activeProject: string | null;
@@ -18,7 +19,7 @@ export const ProjectBar: FC<ProjectBarProps> = ({ activeProject }) => {
         <ProjectDropdown
           onSelect={(event, newProject) => {
             const params = new URLSearchParams();
-            params.set('project', newProject);
+            params.set(QueryParams.Project, newProject);
             const url = `${getDashboardsListUrl(perspective)}?${params.toString()}`;
             navigate(url);
           }}

web/src/components/query-browser.tsx

@@ -1,3 +1,4 @@
+/* eslint-disable react-hooks/refs */
 import {
   PrometheusEndpoint,
   PrometheusLabels,
@@ -45,7 +46,7 @@ import { ChartLineIcon } from '@patternfly/react-icons';
 import classNames from 'classnames';
 import * as _ from 'lodash-es';
 import type { FC, Ref, ReactNode, KeyboardEvent, MouseEvent, ComponentType } from 'react';
-import { memo, useState, useEffect, useCallback, useLayoutEffect } from 'react';
+import { memo, useState, useEffect, useCallback, useLayoutEffect, useRef } from 'react';
 import { useTranslation } from 'react-i18next';
 import { useDispatch, useSelector } from 'react-redux';
 
@@ -641,7 +642,7 @@ const QueryBrowser_: FC<QueryBrowserProps> = ({
   const [samples, setSamples] = useState(maxSamplesForSpan);
   const [updating, setUpdating] = useState(true);
   // Track if we ever received valid data to prevent flickering "No datapoints" during refresh
-  const [hasReceivedData, , setReceivedData] = useBoolean(false);
+  const hasReceivedData = useRef(false);
 
   useEffect(() => {
     onLoadingChange?.(updating);
@@ -818,7 +819,7 @@ const QueryBrowser_: FC<QueryBrowserProps> = ({
           onDataChange?.(newGraphData);
           // Mark that we've received valid data to prevent flickering during refresh
           if (newGraphData && newGraphData.some((d) => d.length > 0)) {
-            setReceivedData();
+            hasReceivedData.current = true;
           }
 
           setIsDisconnectedEnabled(dataIsDisconnected);
@@ -1026,7 +1027,7 @@ const QueryBrowser_: FC<QueryBrowserProps> = ({
             data-test={DataTestIDs.MetricGraph}
           >
             {error && <Error error={error} />}
-            {isGraphDataEmpty && !(hideControls && updating && hasReceivedData) ? (
+            {isGraphDataEmpty && !(hideControls && updating && hasReceivedData.current) ? (
               <GraphEmpty loading={updating} />
             ) : (
               <>

web/src/components/query-params.ts

@@ -12,4 +12,5 @@ export enum QueryParams {
   OpenshiftProject = 'project-dropdown-value',
   Refresh = 'refresh',
   Start = 'start',
+  Edit = 'edit',
 }