Dashboard loading state UX improvements#6480
Conversation
Also separates SVG rendering from loading state handling and adds a fade-in animation to the map appearing (like all other IndexBreakdowns)
|
|
|
||
| export type MetricLabel = { singular: string; plural: string } | ||
|
|
||
| export function WorldMapSvg({ |
There was a problem hiding this comment.
question: Why must the world map flash in and out whenever the user is doing something on the dashboard? Is it to indicate we're actually working on their new selection? Can we find a cheaper, less disruptive way to indicate that? By cheaper, I mean less work for the browser. By less disruptive, I mean something that's changing less things on the dashboard at once. The map itself flashing in and out is, in my view, a step backward.
As I see it, the world map with country outlines is constant (at least in dashboard timescales). The previous implementation took pains to respect that and to just fade paint according to the current data.
With dummy.site where the autogenerated visitors are spread between very few locations, this fade wasn't very noticeable, but on real sites, it seems the variety of visitor locations guarantees a noticeable change in the map when filters are changed.
There was a problem hiding this comment.
Why must the world map flash in and out whenever the user is doing something on the dashboard? Is it to indicate we're actually working on their new selection?
Yeah, and also just trying to make the UX consistent. It feels strange when you apply a filter and the whole dashboard reacts to that change with loading spinners all around (including the graph container), but the map stays intact. Sometimes, there's not even any notable change in the colors and the report just looks frozen.
Can we find a cheaper, less disruptive way to indicate that? By cheaper, I mean less work for the browser. By less disruptive, I mean something that's changing less things on the dashboard at once. The map itself flashing in and out is, in my view, a step backward.
Yeah, I see how it feels a bit like stepping backwards. But perhaps for the sake of consistency, it's acceptable for the time being?
Letting the SVGs (both map and the main graph) stay mounted throughout the loading state is definitely an idea worth pursuing -- I can see the potential to make the loading state look really nice, but only if we get rid of the loading spinners altogether. For the rest of the dashboard (i.e. IndexBreakdowns) we should probably run some animations from old (placeholder) data to new, fresh data. There's also the question of the initial load where we'd most likely have to introduce some silhouettes. I believe @sanne-san had something sketched out already.
Doing all that sounds quite tricky and I don't think this will be worked on any time soon. Therefore proposing this small step backwards in pursuit of consistency.
Re: browser performance, I believe it used to work like this in the past for a long time without any issues. Do you see it as a blocker @apata?
There was a problem hiding this comment.
Disruptive is subjective and I couldn't prove the performance difference, so I shouldn't say it's a blocker. It's something that fully caught my attention though.
(I tried to benchmark this env vs staging with browser performance APIs but failed: the envs didn't have as much as parity as I expected. There were differences in first paint performance, which this PR shouldn't much affect).
Even though I don't have concrete data to show, I just want us to keep in mind that any time we do relayout or fade or anything fancier (like scaling up / down animation), it's the user's device doing work. We want the work to be justified and proportional to the benefits we hope to achieve.
Also, the more we do, the more we are exposed to glitching from bugs in browsers. Maybe a minor matter, but it's an argument in favor of simplicity.
Re: browser performance, I believe it used to work like this in the past for a long time without any issues.
It seems "worked in the past without any issues" argument can be used to justify not changing the map loading logic as well. To me, it's a matter of choosing what we (or what we believe our users will) value more, slightly more consistent from report box to box, or slightly less visually disruptive.
On the subject of consistency, there's the time axis to consider as well. The map has worked without flashing in and out for years. Changing it (in any way) will cause an inconsistency with on the time axis. I suspect quite a few users don't actually like things changing.
| maxValue: number | ||
| dataByAlpha3Code: Map<string, CountryData> | ||
| metricLabel: MetricLabel | ||
| mode: UIMode | ||
| onCountryClick: (country: CountryData) => void |
There was a problem hiding this comment.
nitpick, non-blocking: These 5 props pass through and the others are from API state to calculate whether to show loading screen. Can it be avoided by removing this intermediate component?
Also, mode can be accessed from context directly in WorldMapSVG, it's not used elsewhere.
| <div | ||
| className="relative flex justify-center items-center mt-4 w-full" | ||
| style={{ | ||
| height: MAP_CONTAINER_HEIGHT, | ||
| maxWidth: MAP_CONTAINER_WIDTH | ||
| }} | ||
| > |
There was a problem hiding this comment.
suggestion, non-blocking: Because this wrapper is integral to rendering the map and tooltip correctly, I'd wrap it in WorldMapSvg
2 participants
Changes
IndexBreakdowntabs (e.g. Sources -> Channels) -- done by passing akey={selectedTabKey}to the IndexBreakdown componentsWorldMapcomponent improvementsTests
Changelog
Documentation
Dark mode