Skip to content
This repository has been archived by the owner on Aug 26, 2022. It is now read-only.

Don't show the loading bar for fast client-side navigation. #5528

Merged
merged 1 commit into from Jun 29, 2019

Conversation

davidflanagan
Copy link

Currently in our single-page app, we make a point of showing the
loading bar animation on every page transition, and even prolong
the length of the animation so that the user has postitive feedback
that something has happened.

We've gotten feedback that the loading bar makes client-side navigation
feel slower than it actually is, however. So this PR changes the
progress bar animation in two ways.

  • first, it no longer artificially prolongs the animation
  • second, it changes the animation so that nothing happens
    for the first 100ms.

The upshot is that when the page transition takes less than 100ms
the user will not see the progress bar at all. For navigations that
are under 200ms the user will just see a flash of blue, and for
navigations that take longer than that, they'll probably see something
that they recognize as a progress bar.

This is a trivial patch. We should try it out and see if we like
the site better with or without it.

Currently in our single-page app, we make a point of showing the
loading bar animation on every page transition, and even prolong
the length of the animation so that the user has postitive feedback
that something has happened.

We've gotten feedback that the loading bar makes client-side navigation
feel slower than it actually is, however. So this PR changes the
progress bar animation in two ways.

- first, it no longer artificially prolongs the animation
- second, it changes the animation so that nothing happens
  for the first 100ms.

The upshot is that when the page transition takes less than 100ms
the user will not see the progress bar at all. For navigations that
are under 200ms the user will just see a flash of blue, and for
navigations that take longer than that, they'll probably see something
that they recognize as a progress bar.

This is a trivial patch. We should try it out and see if we like
the site better with or without it.
@davidflanagan
Copy link
Author

@nickbrandt we had a meeting this morning where a knowledgeable external reviewer took a look at our beta site and suggested that we were over-doing it with the loading animation and that the site would feel faster if we left it out for quick transitions.

This PR does that, so we can try it and see what we think.

@codecov-io
Copy link

Codecov Report

Merging #5528 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #5528   +/-   ##
=======================================
  Coverage   95.75%   95.75%           
=======================================
  Files         301      301           
  Lines       25447    25447           
  Branches     1878     1878           
=======================================
  Hits        24366    24366           
  Misses        856      856           
  Partials      225      225

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7b37f87...8db5162. Read the comment docs.

@peterbe
Copy link
Contributor

peterbe commented Jun 28, 2019

Note-to-self; Hopefully we can get to the true research on what the ideal delay should be https://twitter.com/peterbe/status/1144679241688190976

Copy link
Contributor

@peterbe peterbe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still not sure that 100ms is enough but I'll take it for now. I think it's an upgrade.

Copy link

@schalkneethling schalkneethling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r+

@schalkneethling schalkneethling merged commit 7d12ca3 into mdn:master Jun 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants