From b3f9eceafc4ad1dd3b753bd56226a457464c0f2b Mon Sep 17 00:00:00 2001 From: David Lougheed Date: Wed, 27 Nov 2024 17:01:08 -0500 Subject: [PATCH 1/3] refact(overview): use hook for calculating chart clickability --- src/js/components/Overview/ChartCard.tsx | 6 ++--- .../Overview/OverviewDisplayData.tsx | 13 +++++++++-- .../components/Overview/OverviewSection.tsx | 12 ++++++++-- src/js/components/Overview/PublicOverview.tsx | 13 +++++------ src/js/features/data/data.store.ts | 22 +------------------ src/js/features/data/hooks.ts | 14 ++++++++++++ .../features/data/makeGetDataRequest.thunk.ts | 1 - src/js/types/data.ts | 1 - 8 files changed, 45 insertions(+), 37 deletions(-) create mode 100644 src/js/features/data/hooks.ts diff --git a/src/js/components/Overview/ChartCard.tsx b/src/js/components/Overview/ChartCard.tsx index d2c3a4c3..939bd521 100644 --- a/src/js/components/Overview/ChartCard.tsx +++ b/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(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} /> ) : ( @@ -77,6 +76,7 @@ export interface ChartCardProps { section: string; chart: ChartDataField; onRemoveChart: (arg: { section: string; id: string }) => void; + searchable?: boolean; } export default ChartCard; diff --git a/src/js/components/Overview/OverviewDisplayData.tsx b/src/js/components/Overview/OverviewDisplayData.tsx index c0f735ac..f9c72370 100644 --- a/src/js/components/Overview/OverviewDisplayData.tsx +++ b/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 ; + return ( + + ); }; if (isSmallScreen) { @@ -48,6 +56,7 @@ const OverviewDisplayData = ({ section, allCharts }: OverviewDisplayDataProps) = export interface OverviewDisplayDataProps { section: string; allCharts: ChartDataField[]; + searchableFields: Set; } export default OverviewDisplayData; diff --git a/src/js/components/Overview/OverviewSection.tsx b/src/js/components/Overview/OverviewSection.tsx index b930c3d4..ee3d72dc 100644 --- a/src/js/components/Overview/OverviewSection.tsx +++ b/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; +}) => { const t = useTranslationFn(); return ( {t(title)} - + ); }; diff --git a/src/js/components/Overview/PublicOverview.tsx b/src/js/components/Overview/PublicOverview.tsx index 1c6f71e2..30611cf5 100644 --- a/src/js/components/Overview/PublicOverview.tsx +++ b/src/js/components/Overview/PublicOverview.tsx @@ -15,6 +15,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,11 +29,7 @@ const PublicOverview = () => { const [drawerVisible, setDrawerVisible] = useState(false); const [aboutContent, setAboutContent] = useState(''); - const { - isFetchingData: isFetchingOverviewData, - isContentPopulated, - sections, - } = useAppSelector((state) => state.data); + const { isFetchingData: isFetchingOverviewData, sections } = useAppSelector((state) => state.data); const { status: aboutStatus, about } = useAppSelector((state) => state.content); const selectedProject = useSelectedProject(); @@ -59,7 +56,9 @@ const PublicOverview = () => { saveToLocalStorage(sections); }, [sections]); - return !isContentPopulated || isFetchingOverviewData ? ( + const searchableFields = useSearchableFields(); + + return isFetchingOverviewData ? ( ) : ( <> @@ -94,7 +93,7 @@ const PublicOverview = () => { {displayedSections.map(({ sectionTitle, charts }, i) => (
- +
))} diff --git a/src/js/features/data/data.store.ts b/src/js/features/data/data.store.ts index 6c3dcc82..a7c1fec1 100644 --- a/src/js/features/data/data.store.ts +++ b/src/js/features/data/data.store.ts @@ -1,23 +1,12 @@ 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( - 'data/populateClickable', - async (_, { getState }) => { - return getState() - .query.querySections.flatMap((section) => section.fields) - .map((field) => field.id); - } -); interface DataState { isFetchingData: boolean; - isContentPopulated: boolean; defaultLayout: Sections; sections: Sections; counts: Counts; @@ -25,7 +14,6 @@ interface DataState { const initialState: DataState = { isFetchingData: true, - isContentPopulated: false, defaultLayout: [], sections: [], counts: { @@ -101,14 +89,6 @@ const data = createSlice({ }) .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; }); }, }); diff --git a/src/js/features/data/hooks.ts b/src/js/features/data/hooks.ts new file mode 100644 index 00000000..e35346dc --- /dev/null +++ b/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] + ); +}; diff --git a/src/js/features/data/makeGetDataRequest.thunk.ts b/src/js/features/data/makeGetDataRequest.thunk.ts index 583e78a8..6c5d7374 100644 --- a/src/js/features/data/makeGetDataRequest.thunk.ts +++ b/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, }; }; diff --git a/src/js/types/data.ts b/src/js/types/data.ts index 4099a2ef..b41f53eb 100644 --- a/src/js/types/data.ts +++ b/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 { From 5d0d598e76ad8e89aa012d146c0f0407e23f253f Mon Sep 17 00:00:00 2001 From: David Lougheed Date: Thu, 28 Nov 2024 10:39:07 -0500 Subject: [PATCH 2/3] rm old references to populateClickable action --- src/js/components/BentoAppRouter.tsx | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/js/components/BentoAppRouter.tsx b/src/js/components/BentoAppRouter.tsx index c2545bb5..6665a077 100644 --- a/src/js/components/BentoAppRouter.tsx +++ b/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'; @@ -92,12 +92,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]); From 3b0c087a2c67160f5fbd8ebce389f952e79d4fc1 Mon Sep 17 00:00:00 2001 From: David Lougheed Date: Thu, 28 Nov 2024 10:48:13 -0500 Subject: [PATCH 3/3] refact: move to RequestStatus for overview data --- src/js/components/BentoAppRouter.tsx | 3 ++- src/js/components/Overview/Counts.tsx | 9 ++++++--- .../components/Overview/Drawer/ManageChartsDrawer.tsx | 2 +- src/js/components/Overview/PublicOverview.tsx | 9 +++++---- src/js/constants/requests.ts | 3 +++ src/js/features/data/data.store.ts | 11 ++++++----- 6 files changed, 23 insertions(+), 14 deletions(-) create mode 100644 src/js/constants/requests.ts diff --git a/src/js/components/BentoAppRouter.tsx b/src/js/components/BentoAppRouter.tsx index 6665a077..8a2e313d 100644 --- a/src/js/components/BentoAppRouter.tsx +++ b/src/js/components/BentoAppRouter.tsx @@ -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 }); diff --git a/src/js/components/Overview/Counts.tsx b/src/js/components/Overview/Counts.tsx index 3ac2789b..425da8bf 100644 --- a/src/js/components/Overview/Counts.tsx +++ b/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 ( <> {t('Counts')} {data.map(({ entity, icon, count }, i) => ( - + } value={count || (uncensoredCounts ? count : NO_RESULTS_DASHES)} valueStyle={{ color: COUNTS_FILL }} prefix={icon} - loading={isFetchingData} + loading={waitingForData} /> ))} diff --git a/src/js/components/Overview/Drawer/ManageChartsDrawer.tsx b/src/js/components/Overview/Drawer/ManageChartsDrawer.tsx index 3711aef4..f50f2854 100644 --- a/src/js/components/Overview/Drawer/ManageChartsDrawer.tsx +++ b/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 ( { const [drawerVisible, setDrawerVisible] = useState(false); const [aboutContent, setAboutContent] = useState(''); - const { isFetchingData: isFetchingOverviewData, sections } = useAppSelector((state) => state.data); + const { status: overviewDataStatus, sections } = useAppSelector((state) => state.data); const { status: aboutStatus, about } = useAppSelector((state) => state.content); const selectedProject = useSelectedProject(); @@ -37,9 +38,9 @@ const PublicOverview = () => { 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; @@ -58,7 +59,7 @@ const PublicOverview = () => { const searchableFields = useSearchableFields(); - return isFetchingOverviewData ? ( + return WAITING_STATES.includes(overviewDataStatus) ? ( ) : ( <> diff --git a/src/js/constants/requests.ts b/src/js/constants/requests.ts new file mode 100644 index 00000000..aedca9b5 --- /dev/null +++ b/src/js/constants/requests.ts @@ -0,0 +1,3 @@ +import { RequestStatus } from '@/types/requests'; + +export const WAITING_STATES = [RequestStatus.Idle, RequestStatus.Pending]; diff --git a/src/js/features/data/data.store.ts b/src/js/features/data/data.store.ts index a7c1fec1..4b285949 100644 --- a/src/js/features/data/data.store.ts +++ b/src/js/features/data/data.store.ts @@ -4,16 +4,17 @@ import { createSlice } from '@reduxjs/toolkit'; import { makeGetDataRequestThunk } from './makeGetDataRequest.thunk'; import type { Sections } from '@/types/data'; import type { Counts } from '@/types/overviewResponse'; +import { RequestStatus } from '@/types/requests'; interface DataState { - isFetchingData: boolean; + status: RequestStatus; defaultLayout: Sections; sections: Sections; counts: Counts; } const initialState: DataState = { - isFetchingData: true, + status: RequestStatus.Idle, defaultLayout: [], sections: [], counts: { @@ -79,16 +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; + state.status = RequestStatus.Rejected; }); }, });