Merge pull request #235 from bento-platform/refact/chart-clickable

refact(overview): use hook for calculating chart clickability

src/js/components/BentoAppRouter.tsx

@@ -5,7 +5,7 @@ import { useAppDispatch } from '@/hooks';
 
 import { makeGetConfigRequest, makeGetServiceInfoRequest } from '@/features/config/config.store';
 import { makeGetAboutRequest } from '@/features/content/content.store';
-import { makeGetDataRequestThunk, populateClickable } from '@/features/data/data.store';
+import { makeGetDataRequestThunk } from '@/features/data/data.store';
 import { makeGetKatsuPublic, makeGetSearchFields } from '@/features/search/query.store';
 import { getBeaconConfig } from '@/features/beacon/beacon.store';
 import { getBeaconNetworkConfig } from '@/features/beacon/network.store';
@@ -18,6 +18,7 @@ import { getGenomes } from '@/features/reference/reference.store';
 import Loader from '@/components/Loader';
 import DefaultLayout from '@/components/Util/DefaultLayout';
 import { BEACON_NETWORK_ENABLED } from '@/config';
+import { WAITING_STATES } from '@/constants/requests';
 import { RequestStatus } from '@/types/requests';
 import { BentoRoute } from '@/types/routes';
 import { scopeEqual, validProjectDataset } from '@/utils/router';
@@ -35,7 +36,7 @@ const ScopedRoute = () => {
   const { selectedScope, projects, projectsStatus } = useMetadata();
 
   useEffect(() => {
-    if ([RequestStatus.Idle, RequestStatus.Pending].includes(projectsStatus)) return; // Wait for projects to load first
+    if (WAITING_STATES.includes(projectsStatus)) return; // Wait for projects to load first
 
     // Update selectedScope based on URL parameters
     const valid = validProjectDataset(projects, { project: projectId, dataset: datasetId });
@@ -92,12 +93,8 @@ const BentoAppRouter = () => {
     }
 
     dispatch(makeGetAboutRequest());
-    // The "Populate clickable" action needs both chart sections and search fields to be available.
-    // TODO: this is not a very good pattern. It would be better to have a memoized way of determining click-ability at
-    //  render time.
-    Promise.all([dispatch(makeGetDataRequestThunk()), dispatch(makeGetSearchFields())]).then(() =>
-      dispatch(populateClickable())
-    );
+    dispatch(makeGetDataRequestThunk());
+    dispatch(makeGetSearchFields());
     dispatch(makeGetKatsuPublic());
     dispatch(fetchKatsuData());
   }, [dispatch, isAuthenticated, selectedScope]);

src/js/components/Overview/ChartCard.tsx

@@ -12,7 +12,7 @@ import SmallChartCardTitle from '@/components/Util/SmallChartCardTitle';
 const CARD_STYLE: CSSProperties = { height: '415px', borderRadius: '11px', ...BOX_SHADOW };
 const ROW_EMPTY_STYLE: CSSProperties = { height: `${CHART_HEIGHT}px` };
 
-const ChartCard = memo(({ section, chart, onRemoveChart }: ChartCardProps) => {
+const ChartCard = memo(({ section, chart, onRemoveChart, searchable }: ChartCardProps) => {
   const t = useTranslationFn();
   const containerRef = useRef<HTMLDivElement>(null);
   const width = useElementWidth(containerRef, chart.width);
@@ -21,7 +21,6 @@ const ChartCard = memo(({ section, chart, onRemoveChart }: ChartCardProps) => {
     data,
     field: { id, description, title, config },
     chartConfig,
-    isSearchable,
   } = chart;
 
   const extraOptionsData = [
@@ -59,7 +58,7 @@ const ChartCard = memo(({ section, chart, onRemoveChart }: ChartCardProps) => {
             units={config?.units || ''}
             id={id}
             key={id}
-            isClickable={isSearchable}
+            isClickable={!!searchable}
           />
         ) : (
           <Row style={ROW_EMPTY_STYLE} justify="center" align="middle">
@@ -77,6 +76,7 @@ export interface ChartCardProps {
   section: string;
   chart: ChartDataField;
   onRemoveChart: (arg: { section: string; id: string }) => void;
+  searchable?: boolean;
 }
 
 export default ChartCard;

src/js/components/Overview/Counts.tsx

@@ -5,6 +5,7 @@ import { BiDna } from 'react-icons/bi';
 
 import CountsTitleWithHelp from '@/components/Util/CountsTitleWithHelp';
 import { BOX_SHADOW, COUNTS_FILL } from '@/constants/overviewConstants';
+import { WAITING_STATES } from '@/constants/requests';
 import { NO_RESULTS_DASHES } from '@/constants/searchConstants';
 import { useAppSelector, useTranslationFn } from '@/hooks';
 import { useCanSeeUncensoredCounts } from '@/hooks/censorship';
@@ -23,7 +24,7 @@ type CountEntry = { entity: BentoEntity; icon: ReactNode; count: number };
 const Counts = () => {
   const t = useTranslationFn();
 
-  const { counts, isFetchingData } = useAppSelector((state) => state.data);
+  const { counts, status } = useAppSelector((state) => state.data);
 
   const uncensoredCounts = useCanSeeUncensoredCounts();
 
@@ -46,18 +47,20 @@ const Counts = () => {
     },
   ];
 
+  const waitingForData = WAITING_STATES.includes(status);
+
   return (
     <>
       <Typography.Title level={3}>{t('Counts')}</Typography.Title>
       <Space wrap>
         {data.map(({ entity, icon, count }, i) => (
-          <Card key={i} style={{ ...styles.countCard, height: isFetchingData ? 138 : 114 }}>
+          <Card key={i} style={{ ...styles.countCard, height: waitingForData ? 138 : 114 }}>
             <Statistic
               title={<CountsTitleWithHelp entity={entity} />}
               value={count || (uncensoredCounts ? count : NO_RESULTS_DASHES)}
               valueStyle={{ color: COUNTS_FILL }}
               prefix={icon}
-              loading={isFetchingData}
+              loading={waitingForData}
             />
           </Card>
         ))}

src/js/components/Overview/Drawer/ManageChartsDrawer.tsx

@@ -13,7 +13,7 @@ const ManageChartsDrawer = ({ onManageDrawerClose, manageDrawerVisible }: Manage
 
   const dispatch = useAppDispatch();
 
-  const sections = useAppSelector((state) => state.data.sections);
+  const { sections } = useAppSelector((state) => state.data);
 
   return (
     <Drawer

src/js/components/Overview/OverviewDisplayData.tsx

@@ -11,7 +11,7 @@ import ChartCard from './ChartCard';
 
 import type { ChartDataField } from '@/types/data';
 
-const OverviewDisplayData = ({ section, allCharts }: OverviewDisplayDataProps) => {
+const OverviewDisplayData = ({ section, allCharts, searchableFields }: OverviewDisplayDataProps) => {
   const dispatch = useAppDispatch();
   const isSmallScreen = useSmallScreen();
 
@@ -31,7 +31,15 @@ const OverviewDisplayData = ({ section, allCharts }: OverviewDisplayDataProps) =
   );
 
   const renderItem = (chart: ChartDataField) => {
-    return <ChartCard key={chart.id} chart={chart} section={section} onRemoveChart={onRemoveChart} />;
+    return (
+      <ChartCard
+        key={chart.id}
+        chart={chart}
+        section={section}
+        onRemoveChart={onRemoveChart}
+        searchable={searchableFields.has(chart.id)}
+      />
+    );
   };
 
   if (isSmallScreen) {
@@ -48,6 +56,7 @@ const OverviewDisplayData = ({ section, allCharts }: OverviewDisplayDataProps) =
 export interface OverviewDisplayDataProps {
   section: string;
   allCharts: ChartDataField[];
+  searchableFields: Set<string>;
 }
 
 export default OverviewDisplayData;

src/js/components/Overview/OverviewSection.tsx

@@ -4,13 +4,21 @@ import OverviewDisplayData from './OverviewDisplayData';
 import { useTranslationFn } from '@/hooks';
 import type { ChartDataField } from '@/types/data';
 
-const OverviewSection = ({ title, chartData }: { title: string; chartData: ChartDataField[] }) => {
+const OverviewSection = ({
+  title,
+  chartData,
+  searchableFields,
+}: {
+  title: string;
+  chartData: ChartDataField[];
+  searchableFields: Set<string>;
+}) => {
   const t = useTranslationFn();
 
   return (
     <Space direction="vertical" size={0} style={{ width: '100%' }}>
       <Typography.Title level={3}>{t(title)}</Typography.Title>
-      <OverviewDisplayData section={title} allCharts={chartData} />
+      <OverviewDisplayData section={title} allCharts={chartData} searchableFields={searchableFields} />
     </Space>
   );
 };

src/js/components/Overview/PublicOverview.tsx

@@ -6,6 +6,7 @@ import { convertSequenceAndDisplayData, saveValue } from '@/utils/localStorage';
 import type { Sections } from '@/types/data';
 
 import { BOX_SHADOW, LOCALSTORAGE_CHARTS_KEY } from '@/constants/overviewConstants';
+import { WAITING_STATES } from '@/constants/requests';
 
 import OverviewSection from './OverviewSection';
 import ManageChartsDrawer from './Drawer/ManageChartsDrawer';
@@ -15,6 +16,7 @@ import Loader from '@/components/Loader';
 import Dataset from '@/components/Provenance/Dataset';
 
 import { useAppSelector } from '@/hooks';
+import { useSearchableFields } from '@/features/data/hooks';
 import { useSelectedProject, useSelectedScope } from '@/features/metadata/hooks';
 import { useTranslation } from 'react-i18next';
 import { RequestStatus } from '@/types/requests';
@@ -28,21 +30,17 @@ const PublicOverview = () => {
   const [drawerVisible, setDrawerVisible] = useState(false);
   const [aboutContent, setAboutContent] = useState('');
 
-  const {
-    isFetchingData: isFetchingOverviewData,
-    isContentPopulated,
-    sections,
-  } = useAppSelector((state) => state.data);
+  const { status: overviewDataStatus, sections } = useAppSelector((state) => state.data);
   const { status: aboutStatus, about } = useAppSelector((state) => state.content);
 
   const selectedProject = useSelectedProject();
   const { scope } = useSelectedScope();
 
   useEffect(() => {
     // Save sections to localStorage when they change
-    if (isFetchingOverviewData) return;
+    if (overviewDataStatus != RequestStatus.Fulfilled) return;
     saveToLocalStorage(sections);
-  }, [isFetchingOverviewData, sections]);
+  }, [overviewDataStatus, sections]);
 
   useEffect(() => {
     const activeLanguage = i18n.language;
@@ -59,7 +57,9 @@ const PublicOverview = () => {
     saveToLocalStorage(sections);
   }, [sections]);
 
-  return !isContentPopulated || isFetchingOverviewData ? (
+  const searchableFields = useSearchableFields();
+
+  return WAITING_STATES.includes(overviewDataStatus) ? (
     <Loader />
   ) : (
     <>
@@ -94,7 +94,7 @@ const PublicOverview = () => {
         <Col flex={1}>
           {displayedSections.map(({ sectionTitle, charts }, i) => (
             <div key={i} className="overview">
-              <OverviewSection title={sectionTitle} chartData={charts} />
+              <OverviewSection title={sectionTitle} chartData={charts} searchableFields={searchableFields} />
             </div>
           ))}
           <LastIngestionInfo />

src/js/constants/requests.ts

@@ -0,0 +1,3 @@
+import { RequestStatus } from '@/types/requests';
+
+export const WAITING_STATES = [RequestStatus.Idle, RequestStatus.Pending];

src/js/features/data/data.store.ts

@@ -1,31 +1,20 @@
 import type { PayloadAction } from '@reduxjs/toolkit';
-import { createAsyncThunk, createSlice } from '@reduxjs/toolkit';
+import { createSlice } from '@reduxjs/toolkit';
 
 import { makeGetDataRequestThunk } from './makeGetDataRequest.thunk';
 import type { Sections } from '@/types/data';
 import type { Counts } from '@/types/overviewResponse';
-import type { QueryState } from '@/features/search/query.store';
-
-export const populateClickable = createAsyncThunk<string[], void, { state: { query: QueryState } }>(
-  'data/populateClickable',
-  async (_, { getState }) => {
-    return getState()
-      .query.querySections.flatMap((section) => section.fields)
-      .map((field) => field.id);
-  }
-);
+import { RequestStatus } from '@/types/requests';
 
 interface DataState {
-  isFetchingData: boolean;
-  isContentPopulated: boolean;
+  status: RequestStatus;
   defaultLayout: Sections;
   sections: Sections;
   counts: Counts;
 }
 
 const initialState: DataState = {
-  isFetchingData: true,
-  isContentPopulated: false,
+  status: RequestStatus.Idle,
   defaultLayout: [],
   sections: [],
   counts: {
@@ -91,24 +80,16 @@ const data = createSlice({
   extraReducers: (builder) => {
     builder
       .addCase(makeGetDataRequestThunk.pending, (state) => {
-        state.isFetchingData = true;
+        state.status = RequestStatus.Pending;
       })
       .addCase(makeGetDataRequestThunk.fulfilled, (state, { payload }) => {
         state.sections = payload.sectionData;
         state.defaultLayout = payload.defaultData;
         state.counts = payload.counts;
-        state.isFetchingData = false;
+        state.status = RequestStatus.Fulfilled;
       })
       .addCase(makeGetDataRequestThunk.rejected, (state) => {
-        state.isFetchingData = false;
-      })
-      .addCase(populateClickable.fulfilled, (state, { payload }) => {
-        state.sections.forEach((section) => {
-          section.charts.forEach((chart) => {
-            chart.isSearchable = payload.includes(chart.id);
-          });
-        });
-        state.isContentPopulated = true;
+        state.status = RequestStatus.Rejected;
       });
   },
 });

src/js/features/data/hooks.ts

@@ -0,0 +1,14 @@
+import { useMemo } from 'react';
+import { useSearchQuery } from '@/features/search/hooks';
+
+export const useSearchableFields = () => {
+  /**
+   * Hook which calculates a set of searchable fields (which share IDs with charts), which can be used, for example, to
+   * choose whether to add a click event to a chart for the field.
+   */
+  const { querySections } = useSearchQuery();
+  return useMemo(
+    () => new Set(querySections.flatMap((section) => section.fields).map((field) => field.id)),
+    [querySections]
+  );
+};

src/js/features/data/makeGetDataRequest.thunk.ts

@@ -40,7 +40,6 @@ export const makeGetDataRequestThunk = createAsyncThunk<
       // Initial display state
       isDisplayed: i < MAX_CHARTS,
       width: chart.width ?? DEFAULT_CHART_WIDTH, // initial configured width; users can change it from here
-      isSearchable: false,
     };
   };
 

src/js/types/data.ts

@@ -23,7 +23,6 @@ export interface ChartDataField {
   // display options:
   isDisplayed: boolean; // whether the chart is currently displayed (state data)
   width: number; // current width (state data); initial data taken from chart config
-  isSearchable: boolean; // whether the field is searchable
 }
 
 export interface ChartData {