From 644cd12ae597fa69a31927502159c2def974f11b Mon Sep 17 00:00:00 2001 From: Robert Joonas Date: Mon, 29 Dec 2025 14:57:05 +0000 Subject: [PATCH 1/3] get rid of onListUpdate --- assets/js/dashboard/stats/locations/index.js | 88 +++++++------------- assets/js/dashboard/stats/more-link.js | 8 +- assets/js/dashboard/stats/reports/list.tsx | 27 +----- 3 files changed, 35 insertions(+), 88 deletions(-) diff --git a/assets/js/dashboard/stats/locations/index.js b/assets/js/dashboard/stats/locations/index.js index 09161fda9fc0..ac4c8d5a9c1c 100644 --- a/assets/js/dashboard/stats/locations/index.js +++ b/assets/js/dashboard/stats/locations/index.js @@ -19,8 +19,9 @@ import { ReportLayout } from '../reports/report-layout' import { ReportHeader } from '../reports/report-header' import { TabButton, TabWrapper } from '../../components/tabs' import MoreLink from '../more-link' +import { MoreLinkState } from '../more-link-state' -function Countries({ query, site, onClick, afterFetchData, onListUpdate }) { +function Countries({ query, site, onClick, afterFetchData }) { function fetchData() { return api.get(apiPath(site, '/countries'), query, { limit: 9 }) } @@ -60,12 +61,11 @@ function Countries({ query, site, onClick, afterFetchData, onListUpdate }) { }} renderIcon={renderIcon} color="bg-orange-50 group-hover/row:bg-orange-100" - onListUpdate={onListUpdate} /> ) } -function Regions({ query, site, onClick, afterFetchData, onListUpdate }) { +function Regions({ query, site, onClick, afterFetchData }) { function fetchData() { return api.get(apiPath(site, '/regions'), query, { limit: 9 }) } @@ -102,12 +102,11 @@ function Regions({ query, site, onClick, afterFetchData, onListUpdate }) { detailsLinkProps={{ path: regionsRoute.path, search: (search) => search }} renderIcon={renderIcon} color="bg-orange-50 group-hover/row:bg-orange-100" - onListUpdate={onListUpdate} /> ) } -function Cities({ query, site, afterFetchData, onListUpdate }) { +function Cities({ query, site, afterFetchData }) { function fetchData() { return api.get(apiPath(site, '/cities'), query, { limit: 9 }) } @@ -143,7 +142,6 @@ function Cities({ query, site, afterFetchData, onListUpdate }) { detailsLinkProps={{ path: citiesRoute.path, search: (search) => search }} renderIcon={renderIcon} color="bg-orange-50 group-hover/row:bg-orange-100" - onListUpdate={onListUpdate} /> ) } @@ -154,19 +152,13 @@ class Locations extends React.Component { this.onCountryFilter = this.onCountryFilter.bind(this) this.onRegionFilter = this.onRegionFilter.bind(this) this.afterFetchData = this.afterFetchData.bind(this) - this.onListUpdate = this.onListUpdate.bind(this) - this.onMapDataUpdate = this.onMapDataUpdate.bind(this) this.tabKey = `geoTab__${props.site.domain}` const storedTab = storage.getItem(this.tabKey) this.state = { mode: storedTab || 'map', loading: true, skipImportedReason: null, - listData: null, - linkProps: null, - listLoading: true, - mapData: null, - mapLoading: true + moreLinkState: MoreLinkState.LOADING } } @@ -190,32 +182,17 @@ class Locations extends React.Component { this.props.query !== prevProps.query || this.state.mode !== prevState.mode ) { - this.setState({ loading: true }) + this.setState({ loading: true, moreLinkState: MoreLinkState.LOADING }) } } setMode(mode) { return () => { storage.setItem(this.tabKey, mode) - this.setState({ - mode, - listData: null, - linkProps: null, - listLoading: true, - mapData: null, - mapLoading: true - }) + this.setState({ mode }) } } - onListUpdate(list, linkProps, loading) { - this.setState({ listData: list, linkProps, listLoading: loading }) - } - - onMapDataUpdate(data, loading) { - this.setState({ mapData: data, mapLoading: loading }) - } - onCountryFilter(mode) { return () => { this.countriesRestoreMode = mode @@ -228,8 +205,16 @@ class Locations extends React.Component { } afterFetchData(apiResponse) { + let newMoreLinkState + + if (apiResponse.results && apiResponse.results.length > 0) { + newMoreLinkState = MoreLinkState.READY + } else { + newMoreLinkState = MoreLinkState.HIDDEN + } this.setState({ loading: false, + moreLinkState: newMoreLinkState, skipImportedReason: apiResponse.skip_imported_reason }) } @@ -242,7 +227,6 @@ class Locations extends React.Component { site={this.props.site} query={this.props.query} afterFetchData={this.afterFetchData} - onListUpdate={this.onListUpdate} /> ) case 'regions': @@ -252,7 +236,6 @@ class Locations extends React.Component { site={this.props.site} query={this.props.query} afterFetchData={this.afterFetchData} - onListUpdate={this.onListUpdate} /> ) case 'countries': @@ -262,7 +245,6 @@ class Locations extends React.Component { site={this.props.site} query={this.props.query} afterFetchData={this.afterFetchData} - onListUpdate={this.onListUpdate} /> ) case 'map': @@ -271,38 +253,23 @@ class Locations extends React.Component { ) } } - getMoreLink() { - if (this.state.mode === 'map') { - const data = this.state.mapData?.results ?? [] - return ( - search - }} - loading={this.state.mapLoading} - className="" - onClick={undefined} - /> - ) + getMoreLinkProps() { + let path + + if (this.state.mode === 'regions') { + path = regionsRoute.path + } else if (this.state.mode === 'cities') { + path = citiesRoute.path } else { - return ( - - ) + path = countriesRoute.path } + + return { path: path, search: (search) => search } } render() { @@ -333,7 +300,10 @@ class Locations extends React.Component { skipImportedReason={this.state.skipImportedReason} /> - {this.getMoreLink()} + {this.renderContent()} diff --git a/assets/js/dashboard/stats/more-link.js b/assets/js/dashboard/stats/more-link.js index 51108b575a11..99da748ef8ed 100644 --- a/assets/js/dashboard/stats/more-link.js +++ b/assets/js/dashboard/stats/more-link.js @@ -20,7 +20,7 @@ function detailsIcon() { ) } -export default function MoreLink({ linkProps, state, onClick = undefined }) { +export default function MoreLink({ linkProps, state }) { const portalRef = useRef(null) useEffect(() => { @@ -47,11 +47,7 @@ export default function MoreLink({ linkProps, state, onClick = undefined }) { return ( - + {icon} diff --git a/assets/js/dashboard/stats/reports/list.tsx b/assets/js/dashboard/stats/reports/list.tsx index 2515cab32f7d..fdd35e33c7c6 100644 --- a/assets/js/dashboard/stats/reports/list.tsx +++ b/assets/js/dashboard/stats/reports/list.tsx @@ -87,9 +87,7 @@ export interface SharedReportProps< afterFetchNextPage?: (response: TResponse) => void } -type ListReportProps< - TListItem extends Record & { name: string } -> = { +type ListReportProps = { /** What each entry in the list represents (for UI only). */ keyLabel: string metrics: Metric[] @@ -100,12 +98,6 @@ type ListReportProps< onClick?: () => void /** Color of the comparison bars in light-mode. */ color?: string - /** Callback that receives the list data, linkProps, and loading state whenever it updates. Used to render MoreLink in parent components. */ - onListUpdate?: ( - list: TListItem[] | null, - linkProps: AppNavigationLinkProps | undefined, - loading: boolean - ) => void } /** @@ -128,10 +120,8 @@ export default function ListReport< getFilterInfo, renderIcon, getExternalLinkUrl, - fetchData, - onListUpdate -}: Omit, 'afterFetchNextPage'> & - ListReportProps) { + fetchData +}: Omit, 'afterFetchNextPage'> & ListReportProps) { const { query } = useQueryContext() const [state, setState] = useState<{ loading: boolean @@ -147,9 +137,6 @@ export default function ListReport< const getData = useCallback(() => { if (!isRealtime) { setState({ loading: true, list: null, meta: null }) - if (onListUpdate) { - onListUpdate(null, detailsLinkProps, true) - } } fetchData().then((response) => { if (afterFetchData) { @@ -157,12 +144,9 @@ export default function ListReport< } setState({ loading: false, list: response.results, meta: response.meta }) - if (onListUpdate) { - onListUpdate(response.results, detailsLinkProps, false) - } }) // eslint-disable-next-line react-hooks/exhaustive-deps - }, [keyLabel, query, detailsLinkProps, onListUpdate]) + }, [keyLabel, query, detailsLinkProps]) const onVisible = () => { setVisible(true) @@ -174,9 +158,6 @@ export default function ListReport< // loading state, even in realtime mode, because the metrics list will change. We can // only read the new metrics once the new list is loaded. setState({ loading: true, list: null, meta: null }) - if (onListUpdate) { - onListUpdate(null, detailsLinkProps, true) - } } // eslint-disable-next-line react-hooks/exhaustive-deps }, [goalFilterApplied]) From 2c49d142a364691103f55b24c9aa334f5f0a4721 Mon Sep 17 00:00:00 2001 From: Robert Joonas Date: Mon, 29 Dec 2025 15:10:05 +0000 Subject: [PATCH 2/3] get rid of detailsLinkProps --- assets/js/dashboard/stats/behaviours/conversions.js | 5 ----- .../dashboard/stats/behaviours/goal-conversions.js | 6 ------ assets/js/dashboard/stats/behaviours/props.js | 6 ------ assets/js/dashboard/stats/locations/index.js | 6 ------ assets/js/dashboard/stats/pages/index.js | 12 ------------ assets/js/dashboard/stats/reports/list.tsx | 6 +----- 6 files changed, 1 insertion(+), 40 deletions(-) diff --git a/assets/js/dashboard/stats/behaviours/conversions.js b/assets/js/dashboard/stats/behaviours/conversions.js index 72111eeab9d1..128b72b10514 100644 --- a/assets/js/dashboard/stats/behaviours/conversions.js +++ b/assets/js/dashboard/stats/behaviours/conversions.js @@ -6,7 +6,6 @@ import * as metrics from '../reports/metrics' import ListReport from '../reports/list' import { useSiteContext } from '../../site-context' import { useQueryContext } from '../../query-context' -import { conversionsRoute } from '../../router' export default function Conversions({ afterFetchData, onGoalFilterClick }) { const site = useSiteContext() @@ -50,10 +49,6 @@ export default function Conversions({ afterFetchData, onGoalFilterClick }) { keyLabel="Goal" onClick={onGoalFilterClick} metrics={chooseMetrics()} - detailsLinkProps={{ - path: conversionsRoute.path, - search: (search) => search - }} color="bg-red-50 group-hover/row:bg-red-100" colMinWidth={90} /> diff --git a/assets/js/dashboard/stats/behaviours/goal-conversions.js b/assets/js/dashboard/stats/behaviours/goal-conversions.js index e83c7fb19a8b..64925b6f6b3c 100644 --- a/assets/js/dashboard/stats/behaviours/goal-conversions.js +++ b/assets/js/dashboard/stats/behaviours/goal-conversions.js @@ -11,7 +11,6 @@ import { } from '../../util/filters' import { useSiteContext } from '../../site-context' import { useQueryContext } from '../../query-context' -import { customPropsRoute } from '../../router' export const SPECIAL_GOALS = { 404: { title: '404 Pages', prop: 'path' }, @@ -87,11 +86,6 @@ function SpecialPropBreakdown({ prop, afterFetchData }) { getFilterInfo={getFilterInfo} keyLabel={prop} metrics={chooseMetrics()} - detailsLinkProps={{ - path: customPropsRoute.path, - params: { propKey: url.maybeEncodeRouteParam(prop) }, - search: (search) => search - }} getExternalLinkUrl={getExternalLinkUrlFactory()} color="bg-red-50" colMinWidth={90} diff --git a/assets/js/dashboard/stats/behaviours/props.js b/assets/js/dashboard/stats/behaviours/props.js index 1243f08484de..2a453c654e56 100644 --- a/assets/js/dashboard/stats/behaviours/props.js +++ b/assets/js/dashboard/stats/behaviours/props.js @@ -6,7 +6,6 @@ import * as url from '../../util/url' import { EVENT_PROPS_PREFIX, hasConversionGoalFilter } from '../../util/filters' import { useQueryContext } from '../../query-context' import { useSiteContext } from '../../site-context' -import { customPropsRoute } from '../../router' export default function Properties({ propKey, afterFetchData }) { const { query } = useQueryContext() @@ -47,11 +46,6 @@ export default function Properties({ propKey, afterFetchData }) { getFilterInfo={getFilterInfo} keyLabel={propKey} metrics={chooseMetrics()} - detailsLinkProps={{ - path: customPropsRoute.path, - params: { propKey }, - search: (search) => search - }} color="bg-red-50 group-hover/row:bg-red-100" colMinWidth={90} /> diff --git a/assets/js/dashboard/stats/locations/index.js b/assets/js/dashboard/stats/locations/index.js index ac4c8d5a9c1c..073b1518097f 100644 --- a/assets/js/dashboard/stats/locations/index.js +++ b/assets/js/dashboard/stats/locations/index.js @@ -55,10 +55,6 @@ function Countries({ query, site, onClick, afterFetchData }) { onClick={onClick} keyLabel="Country" metrics={chooseMetrics()} - detailsLinkProps={{ - path: countriesRoute.path, - search: (search) => search - }} renderIcon={renderIcon} color="bg-orange-50 group-hover/row:bg-orange-100" /> @@ -99,7 +95,6 @@ function Regions({ query, site, onClick, afterFetchData }) { onClick={onClick} keyLabel="Region" metrics={chooseMetrics()} - detailsLinkProps={{ path: regionsRoute.path, search: (search) => search }} renderIcon={renderIcon} color="bg-orange-50 group-hover/row:bg-orange-100" /> @@ -139,7 +134,6 @@ function Cities({ query, site, afterFetchData }) { getFilterInfo={getFilterInfo} keyLabel="City" metrics={chooseMetrics()} - detailsLinkProps={{ path: citiesRoute.path, search: (search) => search }} renderIcon={renderIcon} color="bg-orange-50 group-hover/row:bg-orange-100" /> diff --git a/assets/js/dashboard/stats/pages/index.js b/assets/js/dashboard/stats/pages/index.js index 3f9fd2e7bac6..30650a2d511e 100644 --- a/assets/js/dashboard/stats/pages/index.js +++ b/assets/js/dashboard/stats/pages/index.js @@ -54,10 +54,6 @@ function EntryPages({ afterFetchData }) { getFilterInfo={getFilterInfo} keyLabel="Entry page" metrics={chooseMetrics()} - detailsLinkProps={{ - path: entryPagesRoute.path, - search: (search) => search - }} getExternalLinkUrl={getExternalLinkUrl} color="bg-orange-50 group-hover/row:bg-orange-100" /> @@ -102,10 +98,6 @@ function ExitPages({ afterFetchData }) { getFilterInfo={getFilterInfo} keyLabel="Exit page" metrics={chooseMetrics()} - detailsLinkProps={{ - path: exitPagesRoute.path, - search: (search) => search - }} getExternalLinkUrl={getExternalLinkUrl} color="bg-orange-50 group-hover/row:bg-orange-100" /> @@ -146,10 +138,6 @@ function TopPages({ afterFetchData }) { getFilterInfo={getFilterInfo} keyLabel="Page" metrics={chooseMetrics()} - detailsLinkProps={{ - path: topPagesRoute.path, - search: (search) => search - }} getExternalLinkUrl={getExternalLinkUrl} color="bg-orange-50 group-hover/row:bg-orange-100" /> diff --git a/assets/js/dashboard/stats/reports/list.tsx b/assets/js/dashboard/stats/reports/list.tsx index fdd35e33c7c6..79cf70cddba1 100644 --- a/assets/js/dashboard/stats/reports/list.tsx +++ b/assets/js/dashboard/stats/reports/list.tsx @@ -1,5 +1,4 @@ import React, { useState, useEffect, useCallback, ReactNode } from 'react' -import { AppNavigationLinkProps } from '../../navigation/use-app-navigate' import FlipMove from 'react-flip-move' import FadeIn from '../../fade-in' @@ -92,8 +91,6 @@ type ListReportProps = { keyLabel: string metrics: Metric[] colMinWidth?: number - /** Navigation props to be passed to "More" link, if any. */ - detailsLinkProps?: AppNavigationLinkProps /** Function with additional action to be taken when a list entry is clicked. */ onClick?: () => void /** Color of the comparison bars in light-mode. */ @@ -114,7 +111,6 @@ export default function ListReport< metrics, colMinWidth = COL_MIN_WIDTH, afterFetchData, - detailsLinkProps, onClick, color, getFilterInfo, @@ -146,7 +142,7 @@ export default function ListReport< setState({ loading: false, list: response.results, meta: response.meta }) }) // eslint-disable-next-line react-hooks/exhaustive-deps - }, [keyLabel, query, detailsLinkProps]) + }, [keyLabel, query]) const onVisible = () => { setVisible(true) From 5eb3b6a70ece4f71d1c5400a1658a79c9dfb7b16 Mon Sep 17 00:00:00 2001 From: Robert Joonas Date: Mon, 29 Dec 2025 15:15:02 +0000 Subject: [PATCH 3/3] remove onDataUpdate from map.tsx as well --- assets/js/dashboard/stats/locations/map.tsx | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/assets/js/dashboard/stats/locations/map.tsx b/assets/js/dashboard/stats/locations/map.tsx index d514f024c5d1..8a0a53668955 100644 --- a/assets/js/dashboard/stats/locations/map.tsx +++ b/assets/js/dashboard/stats/locations/map.tsx @@ -34,15 +34,10 @@ type WorldJsonCountryData = { properties: { name: string; a3: string } } const WorldMap = ({ onCountrySelect, - afterFetchData, - onDataUpdate + afterFetchData }: { onCountrySelect: () => void afterFetchData: (response: unknown) => void - onDataUpdate?: ( - data: { results: CountryData[] } | null, - loading: boolean - ) => void }) => { const navigate = useAppNavigate() const { mode } = useTheme() @@ -91,10 +86,7 @@ const WorldMap = ({ if (data) { afterFetchData(data) } - if (onDataUpdate) { - onDataUpdate(data ?? null, isFetching) - } - }, [afterFetchData, data, isFetching, onDataUpdate]) + }, [afterFetchData, data, isFetching]) const { maxValue, dataByCountryCode } = useMemo(() => { const dataByCountryCode: Map = new Map()