Visual improvements to automatic realtime updates #2532

RobertJoonas · 2022-12-19T21:36:51Z

Changes

Use the react-flip-move library to make updates to ListReports look nicer.
Unify the color of 'No data yet' in darkmode.

Screencasts to showcase 1 and 2: before -> after (the "Before" screencast already includes the auto-updating work done in #2445)

Lose the flicker in the graph when updated in the realtime mode. A console log demonstrates the moment when new data is fetched. Screencasts: before -> after (before = master)
Fix a bug where graph data was requested twice upon changing the interval. Screencasts: before -> after (before = master)
Some refactoring commits to visitor-graph.js

Tests

Writing unit tests for the React dashboard is currently out of scope

Changelog

Realtime dashboard improvements #2445 updates changelog

Documentation

This change does not need a documentation update

Dark mode

The UI has been tested both in dark and light mode

Fetching new data is handled by the `fetchGraphData` callback in `updateInterval`

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.

assets/js/dashboard/stats/graph/visitor-graph.js

assets/js/dashboard/stats/graph/graph-util.js

assets/js/dashboard/stats/graph/visitor-graph.js

vinibrsl · 2022-12-20T09:12:08Z

assets/js/dashboard/stats/graph/visitor-graph.js

@@ -330,16 +329,9 @@ export default class VisitorGraph extends React.Component {
    return validIntervals.includes(interval)
  }

-  maybeRollbackInterval() {


This function checked for invalid states at the very first paint of the component. 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. Could you please also add a comment to the function to make clear this edge case?

Additionally we could delete the localStorage entry whenever an invalid interval value is found. Currently we only reset the state, but not the stored value.

Thanks for pointing that out, @vinibrsl! I totally missed thinking about this. It was indeed hard to understand that this was the reason maybeRollbackInterval was called instead of fetchGraphData. Applied your suggested changes in b9c9da3 and also left a comment.

I'm still not a fan of the way we have to sync the interval from localStorage to the component state along with worrying about whether it's invalid or not.

This got me thinking that maybe the interval doesn't need to be stored in the component state at all. So I made a little PoC that you can see here: https://github.com/plausible/analytics/compare/flipmove...interval-without-state?expand=1

The main difference is that the interval is gone from the component state. It's always fetched from localStorage when sending a request to the backend. And updating the interval value just means updating localStorage. As far as I can tell this works fine, and doesn't allow invalid intervals to cause an error state. What do you think @RobertJoonas @vinibrsl ?

This makes things a little bit easier to follow for me - no need to reset or sync between localStorage and state. Reading and writing the interval key is more direct and does not care about the component's lifecycle (componentDidMount, componentDidUpdate)

Good call, @ukutaht! I agree that reading and writing to a single source is better than managing two different sources (localStorage and component state).

I like that idea, and I agree that we wouldn't need to keep the interval state in the component itself. As long as we have default interval values, there shouldn't be any concerns with this. @ukutaht, @vinibrsl, is it OK to open that PoC PR that Uku linked, test it and merge into this branch?

Looks like a good plan. I approved this PR in case you want to do that in a separate PR.

ukutaht · 2022-12-20T14:36:12Z

I pulled down the code and I saw that reports using ListReport don't show a spinner when changing period from the date picker. Is that a regression?

RobertJoonas · 2022-12-21T11:45:50Z

@ukutaht

I pulled down the code and I saw that reports using ListReport don't show a spinner when changing period from the date picker. Is that a regression?

You're right. The spinner doesn't show anymore. Should I fix it with a new commit in this PR?

* add a new realtime-update-timer module * hook to the new 'tick' event in ListReport for auto-updates This commit fixes the bug where all reports using the `ListReport` component did not auto-update in realtime mode. Those reports are: - Pages (Top / Entry / Exit) - Locations (Countries / Regions / Cities) - Devices (Screen Sizes / Browsers + versions / OS-s + versions) * fetch data for ListReports only when scrolled into view * refactor fetching data in ListReport * refer to one source of truth for utm tags * make the 'All' tab in Sources auto-update * make all UTM tabs in Sources auto-update * fetch UTM data only when scrolled into view * auto-update Referrers with the new timer * auto-update google search terms * auto-update Conversions * make countries map auto-update * auto-update visitor-graph and top stats with new timer * use new tick event for current visitors (in Historical) * remove the old timer class * update changelog * Visual improvements to automatic realtime updates (#2532) * minor consistency fix for text color in dark mode * use FlipMove in goal conversions report * use FlipMove in ListReports * set main graph and top stats loading state correctly * refactor isIntervalValid function * enforce intervals are valid when set and stored * remove duplicate data fetching on interval change Fetching new data is handled by the `fetchGraphData` callback in `updateInterval` * refactor updateMetric function * make it clearer why 'metric' can be a faulty value * extract 'query' and 'site' variables from 'this.props' * 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. * update names of two variables * remove unnecessary negation * make collapsed graph state more explicit * consider stored invalid intervals when graph mounts * fix not showing loading spinner regression * remove interval state from VisitorGraph (#2540) * Realtime prop breakdown (#2535) * disable load more in realtime mode * extract doFetch function * separate fetchPropBreakdown and fetchNextPage functions * subscribe for auto-updates in realtime * improve readability with function name changes

RobertJoonas added 11 commits December 15, 2022 15:20

minor consistency fix for text color in dark mode

6441fb9

use FlipMove in goal conversions report

3944c5b

use FlipMove in ListReports

20a0298

set main graph and top stats loading state correctly

64dac96

refactor isIntervalValid function

3a5288e

enforce intervals are valid when set and stored

8f35fe1

remove duplicate data fetching on interval change

8edd513

Fetching new data is handled by the `fetchGraphData` callback in `updateInterval`

refactor updateMetric function

8e202e5

make it clearer why 'metric' can be a faulty value

0870a19

extract 'query' and 'site' variables from 'this.props'

e559fdc

RobertJoonas changed the base branch from master to realtime-dashboard-improvements December 19, 2022 21:38

vinibrsl reviewed Dec 20, 2022

View reviewed changes

RobertJoonas added 4 commits December 20, 2022 12:05

update names of two variables

216d50b

remove unnecessary negation

39f2186

make collapsed graph state more explicit

ef7a5d9

consider stored invalid intervals when graph mounts

b9c9da3

fix not showing loading spinner regression

9d93cfb

vinibrsl approved these changes Dec 21, 2022

View reviewed changes

RobertJoonas merged commit 497d35a into realtime-dashboard-improvements Dec 21, 2022

RobertJoonas deleted the flipmove branch December 21, 2022 13:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Visual improvements to automatic realtime updates #2532

Visual improvements to automatic realtime updates #2532

RobertJoonas commented Dec 19, 2022

vinibrsl Dec 20, 2022 •

edited

Loading

RobertJoonas Dec 20, 2022

ukutaht Dec 20, 2022 •

edited

Loading

vinibrsl Dec 20, 2022 •

edited

Loading

RobertJoonas Dec 21, 2022

vinibrsl Dec 21, 2022

ukutaht commented Dec 20, 2022 •

edited

Loading

RobertJoonas commented Dec 21, 2022

Visual improvements to automatic realtime updates #2532

Visual improvements to automatic realtime updates #2532

Conversation

RobertJoonas commented Dec 19, 2022

Changes

Tests

Changelog

Documentation

Dark mode

vinibrsl Dec 20, 2022 • edited Loading

Choose a reason for hiding this comment

RobertJoonas Dec 20, 2022

Choose a reason for hiding this comment

ukutaht Dec 20, 2022 • edited Loading

Choose a reason for hiding this comment

vinibrsl Dec 20, 2022 • edited Loading

Choose a reason for hiding this comment

RobertJoonas Dec 21, 2022

Choose a reason for hiding this comment

vinibrsl Dec 21, 2022

Choose a reason for hiding this comment

ukutaht commented Dec 20, 2022 • edited Loading

RobertJoonas commented Dec 21, 2022

vinibrsl Dec 20, 2022 •

edited

Loading

ukutaht Dec 20, 2022 •

edited

Loading

vinibrsl Dec 20, 2022 •

edited

Loading

ukutaht commented Dec 20, 2022 •

edited

Loading