From 6441fb9248a57a172cb9351c091cd686c0471629 Mon Sep 17 00:00:00 2001 From: Robert Date: Tue, 13 Dec 2022 18:29:38 +0200 Subject: [PATCH 01/16] minor consistency fix for text color in dark mode --- assets/js/dashboard/stats/sources/source-list.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assets/js/dashboard/stats/sources/source-list.js b/assets/js/dashboard/stats/sources/source-list.js index 45406a8031b7..948e32ae1f00 100644 --- a/assets/js/dashboard/stats/sources/source-list.js +++ b/assets/js/dashboard/stats/sources/source-list.js @@ -114,7 +114,7 @@ class AllSources extends React.Component { ) } else { - return
No data yet
+ return
No data yet
} } From 3944c5b02de54836999a0a4da8a1311bce408e38 Mon Sep 17 00:00:00 2001 From: Robert Date: Wed, 14 Dec 2022 10:03:46 +0200 Subject: [PATCH 02/16] use FlipMove in goal conversions report --- assets/js/dashboard/stats/conversions/index.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/assets/js/dashboard/stats/conversions/index.js b/assets/js/dashboard/stats/conversions/index.js index 728b3fc80ab7..3a18fcd1db0a 100644 --- a/assets/js/dashboard/stats/conversions/index.js +++ b/assets/js/dashboard/stats/conversions/index.js @@ -1,5 +1,7 @@ import React from 'react'; import { Link } from 'react-router-dom' +import FlipMove from 'react-flip-move' + import Bar from '../bar' import PropBreakdown from './prop-breakdown' @@ -106,8 +108,9 @@ export default class Conversions extends React.Component { CR - - { this.state.goals.map(this.renderGoal.bind(this)) } + + { this.state.goals.map(this.renderGoal.bind(this)) } + ) } From 20a02986f826d6532ffc502a23b901a22bd2c1f7 Mon Sep 17 00:00:00 2001 From: Robert Date: Wed, 14 Dec 2022 10:06:26 +0200 Subject: [PATCH 03/16] use FlipMove in ListReports --- assets/js/dashboard/stats/reports/list.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/assets/js/dashboard/stats/reports/list.js b/assets/js/dashboard/stats/reports/list.js index 72339df716e9..c2ffd5bc986d 100644 --- a/assets/js/dashboard/stats/reports/list.js +++ b/assets/js/dashboard/stats/reports/list.js @@ -1,5 +1,7 @@ import React, { useState, useEffect, useCallback } from 'react'; import { Link } from 'react-router-dom' +import FlipMove from 'react-flip-move'; + import FadeIn from '../../fade-in' import MoreLink from '../more-link' @@ -33,7 +35,6 @@ export default function ListReport(props) { const showConversionRate = !!props.query.filters.goal const fetchData = useCallback(() => { - setState({loading: true, list: null}) props.fetchData() .then((res) => setState({loading: false, list: res})) }, [props.query]) @@ -118,7 +119,9 @@ export default function ListReport(props) { {showConversionRate && CR} - { state.list && state.list.map(renderListItem) } + + { state.list.map(renderListItem) } + ) } From 64dac962e722aba030caa2689f1f301d216790f4 Mon Sep 17 00:00:00 2001 From: Robert Date: Wed, 14 Dec 2022 15:43:33 +0200 Subject: [PATCH 04/16] set main graph and top stats loading state correctly --- assets/js/dashboard/stats/graph/visitor-graph.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/assets/js/dashboard/stats/graph/visitor-graph.js b/assets/js/dashboard/stats/graph/visitor-graph.js index aa8105426000..3b62cc95d9d7 100644 --- a/assets/js/dashboard/stats/graph/visitor-graph.js +++ b/assets/js/dashboard/stats/graph/visitor-graph.js @@ -415,7 +415,7 @@ export default class VisitorGraph extends React.Component { fetchGraphData() { if (!this.state.metric) { - this.setState({ mainGraphLoadingState: LOADING_STATE.ready, graphData: null }) + this.setState({ mainGraphLoadingState: LOADING_STATE.loaded, graphData: null }) return } @@ -425,19 +425,19 @@ export default class VisitorGraph extends React.Component { api.get(url, this.props.query, params) .then((res) => { - this.setState({ mainGraphLoadingState: LOADING_STATE.ready, graphData: res }) + this.setState({ mainGraphLoadingState: LOADING_STATE.loaded, graphData: res }) return res }) .catch((err) => { console.log(err) - this.setState({ mainGraphLoadingState: LOADING_STATE.ready, graphData: false }) + this.setState({ mainGraphLoadingState: LOADING_STATE.loaded, graphData: false }) }) } fetchTopStatData() { api.get(`/api/stats/${encodeURIComponent(this.props.site.domain)}/top-stats`, this.props.query) .then((res) => { - this.setState({ topStatsLoadingState: LOADING_STATE.ready, topStatData: res }) + this.setState({ topStatsLoadingState: LOADING_STATE.loaded, topStatData: res }) return res }) } @@ -448,8 +448,8 @@ export default class VisitorGraph extends React.Component { const theme = document.querySelector('html').classList.contains('dark') || false - const topStatsReadyOrRefreshing = (topStatsLoadingState === LOADING_STATE.ready || topStatsLoadingState === LOADING_STATE.refreshing) - const mainGraphReadyOrRefreshing = (mainGraphLoadingState === LOADING_STATE.ready || mainGraphLoadingState === LOADING_STATE.refreshing) + const topStatsReadyOrRefreshing = (topStatsLoadingState === LOADING_STATE.loaded || topStatsLoadingState === LOADING_STATE.refreshing) + const mainGraphReadyOrRefreshing = (mainGraphLoadingState === LOADING_STATE.loaded || mainGraphLoadingState === LOADING_STATE.refreshing) const noMetricOrRefreshing = (!metric || mainGraphLoadingState === LOADING_STATE.refreshing) const topStatAndGraphLoaded = !!(topStatData && graphData) From 3a5288ef1f28cc60962280529cfee750bde44cbe Mon Sep 17 00:00:00 2001 From: Robert Date: Wed, 14 Dec 2022 20:36:17 +0200 Subject: [PATCH 05/16] refactor isIntervalValid function --- .../js/dashboard/stats/graph/visitor-graph.js | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/assets/js/dashboard/stats/graph/visitor-graph.js b/assets/js/dashboard/stats/graph/visitor-graph.js index 3b62cc95d9d7..142adaa81c0f 100644 --- a/assets/js/dashboard/stats/graph/visitor-graph.js +++ b/assets/js/dashboard/stats/graph/visitor-graph.js @@ -325,21 +325,15 @@ export default class VisitorGraph extends React.Component { this.updateInterval = this.updateInterval.bind(this) } - isIntervalValid({ query, site }) { - const period = query?.period - const validIntervalsForPeriod = site.validIntervalsByPeriod[period] || [] - const storedInterval = getStoredInterval(period, site.domain) - - return validIntervalsForPeriod.includes(storedInterval) + isIntervalValid(interval) { + const validIntervals = this.props.site.validIntervalsByPeriod[this.props.query.period] || [] + return validIntervals.includes(interval) } maybeRollbackInterval() { - if (this.isIntervalValid(this.props)) { - const interval = getStoredInterval(this.props.query.period, this.props.site.domain) - - this.setState({interval}, () => { - this.fetchGraphData() - }) + const interval = getStoredInterval(this.props.query.period, this.props.site.domain) + if (this.isIntervalValid(interval)) { + this.setState({interval}, this.fetchGraphData) } else { this.setState({interval: undefined}, () => { this.setState({graphData: null}) From 8f35fe1467049409c49325182c84c3a1784a0ff3 Mon Sep 17 00:00:00 2001 From: Robert Date: Thu, 15 Dec 2022 20:25:59 +0200 Subject: [PATCH 06/16] enforce intervals are valid when set and stored --- assets/js/dashboard/stats/graph/graph-util.js | 2 -- assets/js/dashboard/stats/graph/visitor-graph.js | 12 ++++++------ 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/assets/js/dashboard/stats/graph/graph-util.js b/assets/js/dashboard/stats/graph/graph-util.js index 98000e69abb4..cc306cb6a550 100644 --- a/assets/js/dashboard/stats/graph/graph-util.js +++ b/assets/js/dashboard/stats/graph/graph-util.js @@ -1,8 +1,6 @@ import numberFormatter, {durationFormatter} from '../../util/number-formatter' import dateFormatter from './date-formatter.js' -export const INTERVALS = ["month", "week", "date", "hour", "minute"] - export const METRIC_MAPPING = { 'Unique visitors (last 30 min)': 'visitors', 'Pageviews (last 30 min)': 'pageviews', diff --git a/assets/js/dashboard/stats/graph/visitor-graph.js b/assets/js/dashboard/stats/graph/visitor-graph.js index 142adaa81c0f..7c524a011467 100644 --- a/assets/js/dashboard/stats/graph/visitor-graph.js +++ b/assets/js/dashboard/stats/graph/visitor-graph.js @@ -5,7 +5,7 @@ import { navigateToQuery } from '../../query' import * as api from '../../api' import * as storage from '../../util/storage' import LazyLoader from '../../components/lazy-loader' -import {GraphTooltip, buildDataSet, INTERVALS, METRIC_MAPPING, METRIC_LABELS, METRIC_FORMATTER} from './graph-util'; +import {GraphTooltip, buildDataSet, METRIC_MAPPING, METRIC_LABELS, METRIC_FORMATTER} from './graph-util'; import dateFormatter from './date-formatter'; import TopStats from './top-stats'; import { IntervalPicker, getStoredInterval, storeInterval } from './interval-picker'; @@ -331,9 +331,9 @@ export default class VisitorGraph extends React.Component { } maybeRollbackInterval() { - const interval = getStoredInterval(this.props.query.period, this.props.site.domain) - if (this.isIntervalValid(interval)) { - this.setState({interval}, this.fetchGraphData) + const storedInterval = getStoredInterval(this.props.query.period, this.props.site.domain) + if (storedInterval) { + this.setState({interval: storedInterval}, this.fetchGraphData) } else { this.setState({interval: undefined}, () => { this.setState({graphData: null}) @@ -343,8 +343,8 @@ export default class VisitorGraph extends React.Component { } updateInterval(interval) { - if (INTERVALS.includes(interval)) { - this.setState({interval, mainGraphLoadingState: LOADING_STATE.refreshing}, this.maybeRollbackInterval) + if (this.isIntervalValid(interval)) { + this.setState({interval, mainGraphLoadingState: LOADING_STATE.refreshing}, this.fetchGraphData) storeInterval(this.props.query.period, this.props.site.domain, interval) } } From 8edd5134d8dd370d3930416ead6e96e3eb6f2b00 Mon Sep 17 00:00:00 2001 From: Robert Date: Mon, 19 Dec 2022 13:05:56 +0200 Subject: [PATCH 07/16] remove duplicate data fetching on interval change Fetching new data is handled by the `fetchGraphData` callback in `updateInterval` --- assets/js/dashboard/stats/graph/visitor-graph.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/assets/js/dashboard/stats/graph/visitor-graph.js b/assets/js/dashboard/stats/graph/visitor-graph.js index 7c524a011467..7e367c476bf3 100644 --- a/assets/js/dashboard/stats/graph/visitor-graph.js +++ b/assets/js/dashboard/stats/graph/visitor-graph.js @@ -359,7 +359,7 @@ export default class VisitorGraph extends React.Component { } componentDidUpdate(prevProps, prevState) { - const { metric, topStatData, interval } = this.state; + const { metric, topStatData } = this.state; if (this.props.query !== prevProps.query) { if (metric) { @@ -374,10 +374,6 @@ export default class VisitorGraph extends React.Component { this.setState({mainGraphLoadingState: LOADING_STATE.refreshing}, this.maybeRollbackInterval) } - if (interval !== prevState.interval && interval) { - this.setState({mainGraphLoadingState: LOADING_STATE.refreshing}, this.maybeRollbackInterval) - } - const savedMetric = storage.getItem(`metric__${this.props.site.domain}`) const topStatLabels = topStatData && topStatData.top_stats.map(({ name }) => METRIC_MAPPING[name]).filter(name => name) const prevTopStatLabels = prevState.topStatData && prevState.topStatData.top_stats.map(({ name }) => METRIC_MAPPING[name]).filter(name => name) From 8e202e5c661174929018ee2378dfac782dc04fa1 Mon Sep 17 00:00:00 2001 From: Robert Date: Mon, 19 Dec 2022 13:23:34 +0200 Subject: [PATCH 08/16] refactor updateMetric function --- assets/js/dashboard/stats/graph/visitor-graph.js | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/assets/js/dashboard/stats/graph/visitor-graph.js b/assets/js/dashboard/stats/graph/visitor-graph.js index 7e367c476bf3..58e0a761a4e5 100644 --- a/assets/js/dashboard/stats/graph/visitor-graph.js +++ b/assets/js/dashboard/stats/graph/visitor-graph.js @@ -393,14 +393,11 @@ export default class VisitorGraph extends React.Component { document.removeEventListener('tick', this.fetchTopStatData) } - updateMetric(newMetric) { - if (newMetric === this.state.metric) { - storage.setItem(`metric__${this.props.site.domain}`, "") - this.setState({ metric: "" }) - } else { - storage.setItem(`metric__${this.props.site.domain}`, newMetric) - this.setState({ metric: newMetric }) - } + updateMetric(clickedMetric) { + const newMetric = clickedMetric !== this.state.metric ? clickedMetric : "" + + storage.setItem(`metric__${this.props.site.domain}`, newMetric) + this.setState({ metric: newMetric }) } fetchGraphData() { From 0870a19fe75e3746aa57eea91e066ecc810064f5 Mon Sep 17 00:00:00 2001 From: Robert Date: Mon, 19 Dec 2022 13:31:06 +0200 Subject: [PATCH 09/16] make it clearer why 'metric' can be a faulty value --- assets/js/dashboard/stats/graph/visitor-graph.js | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/assets/js/dashboard/stats/graph/visitor-graph.js b/assets/js/dashboard/stats/graph/visitor-graph.js index 58e0a761a4e5..9210ac765b0e 100644 --- a/assets/js/dashboard/stats/graph/visitor-graph.js +++ b/assets/js/dashboard/stats/graph/visitor-graph.js @@ -362,10 +362,10 @@ export default class VisitorGraph extends React.Component { const { metric, topStatData } = this.state; if (this.props.query !== prevProps.query) { - if (metric) { - this.setState({ mainGraphLoadingState: LOADING_STATE.loading, topStatsLoadingState: LOADING_STATE.loading, graphData: null, topStatData: null }, this.maybeRollbackInterval) - } else { + if (this.isGraphCollapsed()) { this.setState({ topStatsLoadingState: LOADING_STATE.loading, topStatData: null }) + } else { + this.setState({ mainGraphLoadingState: LOADING_STATE.loading, topStatsLoadingState: LOADING_STATE.loading, graphData: null, topStatData: null }, this.maybeRollbackInterval) } this.fetchTopStatData() } @@ -388,6 +388,10 @@ export default class VisitorGraph extends React.Component { } } + isGraphCollapsed() { + return !this.state.metric + } + componentWillUnmount() { document.removeEventListener('tick', this.maybeRollbackInterval) document.removeEventListener('tick', this.fetchTopStatData) @@ -401,13 +405,13 @@ export default class VisitorGraph extends React.Component { } fetchGraphData() { - if (!this.state.metric) { + if (this.isGraphCollapsed()) { this.setState({ mainGraphLoadingState: LOADING_STATE.loaded, graphData: null }) return } const url = `/api/stats/${encodeURIComponent(this.props.site.domain)}/main-graph` - let params = {metric: this.state.metric || 'none'} + let params = {metric: this.state.metric} if (this.state.interval) { params.interval = this.state.interval } api.get(url, this.props.query, params) From e559fdcdd7eb2992f99efd92191cbb68d73d6031 Mon Sep 17 00:00:00 2001 From: Robert Date: Mon, 19 Dec 2022 20:45:02 +0200 Subject: [PATCH 10/16] extract 'query' and 'site' variables from 'this.props' --- assets/js/dashboard/stats/graph/visitor-graph.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/assets/js/dashboard/stats/graph/visitor-graph.js b/assets/js/dashboard/stats/graph/visitor-graph.js index 9210ac765b0e..fb55194268da 100644 --- a/assets/js/dashboard/stats/graph/visitor-graph.js +++ b/assets/js/dashboard/stats/graph/visitor-graph.js @@ -360,8 +360,9 @@ export default class VisitorGraph extends React.Component { componentDidUpdate(prevProps, prevState) { const { metric, topStatData } = this.state; + const { query, site } = this.props - if (this.props.query !== prevProps.query) { + if (query !== prevProps.query) { if (this.isGraphCollapsed()) { this.setState({ topStatsLoadingState: LOADING_STATE.loading, topStatData: null }) } else { @@ -374,11 +375,11 @@ export default class VisitorGraph extends React.Component { this.setState({mainGraphLoadingState: LOADING_STATE.refreshing}, this.maybeRollbackInterval) } - const savedMetric = storage.getItem(`metric__${this.props.site.domain}`) + const savedMetric = storage.getItem(`metric__${site.domain}`) const topStatLabels = topStatData && topStatData.top_stats.map(({ name }) => METRIC_MAPPING[name]).filter(name => name) const prevTopStatLabels = prevState.topStatData && prevState.topStatData.top_stats.map(({ name }) => METRIC_MAPPING[name]).filter(name => name) if (topStatLabels && `${topStatLabels}` !== `${prevTopStatLabels}`) { - if (this.props.query.filters.goal && metric !== 'conversions') { + if (query.filters.goal && metric !== 'conversions') { this.setState({ metric: 'conversions' }) } else if (topStatLabels.includes(savedMetric) && savedMetric !== "") { this.setState({ metric: savedMetric }) From 0a531723a2b4cd898e3548d165ee92541095f138 Mon Sep 17 00:00:00 2001 From: Robert Date: Mon, 19 Dec 2022 21:07:00 +0200 Subject: [PATCH 11/16] reset interval state only when period is changed The 'maybeRollbackInterval' function was also used to fetch data. This commit replaces all those function calls with 'fetchGraphData' which better describes the actual behavior. We should only worry about rolling back the interval if 'query.period' has changed. This commit also stops the graph from flickering when it is updated in realtime. --- .../js/dashboard/stats/graph/visitor-graph.js | 26 ++++++++----------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/assets/js/dashboard/stats/graph/visitor-graph.js b/assets/js/dashboard/stats/graph/visitor-graph.js index fb55194268da..a68df05e909e 100644 --- a/assets/js/dashboard/stats/graph/visitor-graph.js +++ b/assets/js/dashboard/stats/graph/visitor-graph.js @@ -321,7 +321,6 @@ export default class VisitorGraph extends React.Component { this.updateMetric = this.updateMetric.bind(this) this.fetchTopStatData = this.fetchTopStatData.bind(this) this.fetchGraphData = this.fetchGraphData.bind(this) - this.maybeRollbackInterval = this.maybeRollbackInterval.bind(this) this.updateInterval = this.updateInterval.bind(this) } @@ -330,16 +329,9 @@ export default class VisitorGraph extends React.Component { return validIntervals.includes(interval) } - maybeRollbackInterval() { + resetInterval() { const storedInterval = getStoredInterval(this.props.query.period, this.props.site.domain) - if (storedInterval) { - this.setState({interval: storedInterval}, this.fetchGraphData) - } else { - this.setState({interval: undefined}, () => { - this.setState({graphData: null}) - this.fetchGraphData() - }) - } + this.setState({interval: storedInterval || undefined}) } updateInterval(interval) { @@ -350,10 +342,10 @@ export default class VisitorGraph extends React.Component { } onVisible() { - this.setState({mainGraphLoadingState: LOADING_STATE.loading}, this.maybeRollbackInterval) + this.setState({mainGraphLoadingState: LOADING_STATE.loading}, this.fetchGraphData) this.fetchTopStatData() if (this.props.query.period === 'realtime') { - document.addEventListener('tick', this.maybeRollbackInterval) + document.addEventListener('tick', this.fetchGraphData) document.addEventListener('tick', this.fetchTopStatData) } } @@ -362,17 +354,21 @@ export default class VisitorGraph extends React.Component { const { metric, topStatData } = this.state; const { query, site } = this.props + if (query.period !== prevProps.query.period) { + this.resetInterval() + } + if (query !== prevProps.query) { if (this.isGraphCollapsed()) { this.setState({ topStatsLoadingState: LOADING_STATE.loading, topStatData: null }) } else { - this.setState({ mainGraphLoadingState: LOADING_STATE.loading, topStatsLoadingState: LOADING_STATE.loading, graphData: null, topStatData: null }, this.maybeRollbackInterval) + this.setState({ mainGraphLoadingState: LOADING_STATE.loading, topStatsLoadingState: LOADING_STATE.loading, graphData: null, topStatData: null }, this.fetchGraphData) } this.fetchTopStatData() } if (metric !== prevState.metric) { - this.setState({mainGraphLoadingState: LOADING_STATE.refreshing}, this.maybeRollbackInterval) + this.setState({mainGraphLoadingState: LOADING_STATE.refreshing}, this.fetchGraphData) } const savedMetric = storage.getItem(`metric__${site.domain}`) @@ -394,7 +390,7 @@ export default class VisitorGraph extends React.Component { } componentWillUnmount() { - document.removeEventListener('tick', this.maybeRollbackInterval) + document.removeEventListener('tick', this.fetchGraphData) document.removeEventListener('tick', this.fetchTopStatData) } From 216d50bb9b0a79346a7254d61d97db552ccc1bd4 Mon Sep 17 00:00:00 2001 From: Robert Date: Tue, 20 Dec 2022 12:05:55 +0200 Subject: [PATCH 12/16] update names of two variables --- assets/js/dashboard/stats/graph/visitor-graph.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/assets/js/dashboard/stats/graph/visitor-graph.js b/assets/js/dashboard/stats/graph/visitor-graph.js index a68df05e909e..5b2efb1c304a 100644 --- a/assets/js/dashboard/stats/graph/visitor-graph.js +++ b/assets/js/dashboard/stats/graph/visitor-graph.js @@ -436,14 +436,14 @@ export default class VisitorGraph extends React.Component { const theme = document.querySelector('html').classList.contains('dark') || false - const topStatsReadyOrRefreshing = (topStatsLoadingState === LOADING_STATE.loaded || topStatsLoadingState === LOADING_STATE.refreshing) - const mainGraphReadyOrRefreshing = (mainGraphLoadingState === LOADING_STATE.loaded || mainGraphLoadingState === LOADING_STATE.refreshing) + const topStatsLoadedOrRefreshing = (topStatsLoadingState === LOADING_STATE.loaded || topStatsLoadingState === LOADING_STATE.refreshing) + const mainGraphLoadedOrRefreshing = (mainGraphLoadingState === LOADING_STATE.loaded || mainGraphLoadingState === LOADING_STATE.refreshing) const noMetricOrRefreshing = (!metric || mainGraphLoadingState === LOADING_STATE.refreshing) const topStatAndGraphLoaded = !!(topStatData && graphData) const showGraph = - topStatsReadyOrRefreshing && - mainGraphReadyOrRefreshing && + topStatsLoadedOrRefreshing && + mainGraphLoadedOrRefreshing && (topStatData && noMetricOrRefreshing || topStatAndGraphLoaded) return ( From 39f2186f5dc4dd2d6fe394139af21f106b260650 Mon Sep 17 00:00:00 2001 From: Robert Date: Tue, 20 Dec 2022 12:12:02 +0200 Subject: [PATCH 13/16] remove unnecessary negation --- assets/js/dashboard/stats/graph/visitor-graph.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assets/js/dashboard/stats/graph/visitor-graph.js b/assets/js/dashboard/stats/graph/visitor-graph.js index 5b2efb1c304a..c13af5e59420 100644 --- a/assets/js/dashboard/stats/graph/visitor-graph.js +++ b/assets/js/dashboard/stats/graph/visitor-graph.js @@ -395,7 +395,7 @@ export default class VisitorGraph extends React.Component { } updateMetric(clickedMetric) { - const newMetric = clickedMetric !== this.state.metric ? clickedMetric : "" + const newMetric = clickedMetric === this.state.metric ? "" : clickedMetric storage.setItem(`metric__${this.props.site.domain}`, newMetric) this.setState({ metric: newMetric }) From ef7a5d9274a130cd372ddfe8e0f90c00be96e194 Mon Sep 17 00:00:00 2001 From: Robert Date: Tue, 20 Dec 2022 12:13:08 +0200 Subject: [PATCH 14/16] make collapsed graph state more explicit --- assets/js/dashboard/stats/graph/visitor-graph.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assets/js/dashboard/stats/graph/visitor-graph.js b/assets/js/dashboard/stats/graph/visitor-graph.js index c13af5e59420..511f49ba14fe 100644 --- a/assets/js/dashboard/stats/graph/visitor-graph.js +++ b/assets/js/dashboard/stats/graph/visitor-graph.js @@ -386,7 +386,7 @@ export default class VisitorGraph extends React.Component { } isGraphCollapsed() { - return !this.state.metric + return this.state.metric === "" } componentWillUnmount() { From b9c9da3591ac2545e0d0d347a31674eef4f556bc Mon Sep 17 00:00:00 2001 From: Robert Date: Tue, 20 Dec 2022 12:47:26 +0200 Subject: [PATCH 15/16] consider stored invalid intervals when graph mounts --- .../dashboard/stats/graph/interval-picker.js | 4 ++++ .../js/dashboard/stats/graph/visitor-graph.js | 23 +++++++++++++++---- assets/js/dashboard/util/storage.js | 8 +++++++ 3 files changed, 31 insertions(+), 4 deletions(-) diff --git a/assets/js/dashboard/stats/graph/interval-picker.js b/assets/js/dashboard/stats/graph/interval-picker.js index d438572b2fa2..bd7baa34008b 100644 --- a/assets/js/dashboard/stats/graph/interval-picker.js +++ b/assets/js/dashboard/stats/graph/interval-picker.js @@ -21,6 +21,10 @@ export const storeInterval = function(period, domain, interval) { storage.setItem(`interval__${period}__${domain}`, interval) } +export const removeStoredInterval = function(period, domain) { + storage.removeItem(`interval__${period}__${domain}`) +} + function subscribeKeybinding(element) { const handleKeyPress = useCallback((event) => { if (isKeyPressed(event, "i")) element.current?.click() diff --git a/assets/js/dashboard/stats/graph/visitor-graph.js b/assets/js/dashboard/stats/graph/visitor-graph.js index 511f49ba14fe..4e3e06962472 100644 --- a/assets/js/dashboard/stats/graph/visitor-graph.js +++ b/assets/js/dashboard/stats/graph/visitor-graph.js @@ -8,7 +8,7 @@ import LazyLoader from '../../components/lazy-loader' import {GraphTooltip, buildDataSet, METRIC_MAPPING, METRIC_LABELS, METRIC_FORMATTER} from './graph-util'; import dateFormatter from './date-formatter'; import TopStats from './top-stats'; -import { IntervalPicker, getStoredInterval, storeInterval } from './interval-picker'; +import { IntervalPicker, getStoredInterval, storeInterval, removeStoredInterval } from './interval-picker'; import FadeIn from '../../fade-in'; import * as url from '../../util/url' import classNames from "classnames"; @@ -315,7 +315,7 @@ export default class VisitorGraph extends React.Component { topStatsLoadingState: LOADING_STATE.loading, mainGraphLoadingState: LOADING_STATE.loading, metric: storage.getItem(`metric__${this.props.site.domain}`) || 'visitors', - interval: getStoredInterval(this.props.query.period, this.props.site.domain) + interval: undefined } this.onVisible = this.onVisible.bind(this) this.updateMetric = this.updateMetric.bind(this) @@ -324,14 +324,29 @@ export default class VisitorGraph extends React.Component { this.updateInterval = this.updateInterval.bind(this) } + componentDidMount() { + // Before fetching any data, we want to make sure that the stored interval is valid. + // For example if Plausible ever changes an interval name or removes an interval for + // a period, users' localStorage would still keep the old invalid value. We need to + // check that and reset the interval in this case + this.resetInterval() + } + isIntervalValid(interval) { const validIntervals = this.props.site.validIntervalsByPeriod[this.props.query.period] || [] return validIntervals.includes(interval) } resetInterval() { - const storedInterval = getStoredInterval(this.props.query.period, this.props.site.domain) - this.setState({interval: storedInterval || undefined}) + const { query, site } = this.props + const storedInterval = getStoredInterval(query.period, site.domain) + + if (this.isIntervalValid(storedInterval)) { + this.setState({interval: storedInterval}) + } else { + this.setState({interval: undefined}) + removeStoredInterval(query.period, site.domain) + } } updateInterval(interval) { diff --git a/assets/js/dashboard/util/storage.js b/assets/js/dashboard/util/storage.js index 3333199e4ed5..b4de6cb5ae0b 100644 --- a/assets/js/dashboard/util/storage.js +++ b/assets/js/dashboard/util/storage.js @@ -33,3 +33,11 @@ export function getItem(key) { return memStore[key] } } + +export function removeItem(key) { + if (isLocalStorageAvailable) { + window.localStorage.removeItem(key) + } else { + delete memStore[key] + } +} From 9d93cfb9805ccabed0b0fa36e2ca5bc4055f602e Mon Sep 17 00:00:00 2001 From: Robert Date: Wed, 21 Dec 2022 14:36:33 +0200 Subject: [PATCH 16/16] fix not showing loading spinner regression --- assets/js/dashboard/stats/reports/list.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/assets/js/dashboard/stats/reports/list.js b/assets/js/dashboard/stats/reports/list.js index c2ffd5bc986d..b838e163f8e4 100644 --- a/assets/js/dashboard/stats/reports/list.js +++ b/assets/js/dashboard/stats/reports/list.js @@ -35,6 +35,9 @@ export default function ListReport(props) { const showConversionRate = !!props.query.filters.goal const fetchData = useCallback(() => { + if (props.query.period !== 'realtime') { + setState({loading: true, list: null}) + } props.fetchData() .then((res) => setState({loading: false, list: res})) }, [props.query])