Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

web: behaviour: add loading view to dashboard #5458

Merged
merged 3 commits into from Apr 19, 2020
Merged

Conversation

aoldershaw
Copy link
Contributor

@aoldershaw aoldershaw commented Apr 17, 2020

Existing Issue

Fixes #2935 (this time on 6.x).

Why do we need this PR?

As with #5427, add a spinner to the dashboard before
any API responses come back. Note that this isn't as useful for 6.x
since typically the user will have a cached API response, which is very
quick to load. However, it's easy to do, so might as well!

This commit uses a slightly different design than
#5427 - rather than having any text next to the
spinner, just display a spinner (in the center of the screen). I don't
have strong opinions either way, but the thinking was that we probably
don't need to tell the user what a spinner means! Happy to change it
to match the other design, though.

Changes proposed in this pull request

  • Add a loading spinner on the dashboard page before any pipeline data (from the API or from cache) comes back
  • Note that it's based solely on pipeline data because, even without job data, we display empty pipeline boxes (which is more useful than just a spinner)

Screen Shot 2020-04-17 at 3 37 39 PM

Contributor Checklist

Reviewer Checklist

  • Code reviewed
  • Tests reviewed
  • Release notes reviewed
  • PR acceptance performed

@aoldershaw aoldershaw requested a review from a team April 17, 2020 19:57
Aidan Oldershaw added 2 commits April 17, 2020 15:58
As with #5427, add a spinner to the dashboard before
any API responses come back. Note that this isn't as useful for 6.x
since typically the user will have a cached API response, which is very
quick to load. However, it's easy to do, so might as well!

This commit uses a slightly different design than
#5427 - rather than having any text next to the
spinner, just display a spinner (in the center of the screen). I don't
have strong opinions either way, but the thinking was that we probably
don't need to tell the user what a spinner means! Happy to change it
to match the other design, though.

Signed-off-by: Aidan Oldershaw <aoldershaw@pivotal.io>
Signed-off-by: Aidan Oldershaw <aoldershaw@pivotal.io>
@zoetian zoetian self-assigned this Apr 17, 2020
@zoetian
Copy link
Member

zoetian commented Apr 18, 2020

Ugh, I have a super lame question here... I cannot reproduce it on local... (help? @aoldershaw ).....
So I tried to set the throttling upload 30kb/s, download 40kb/s, latency 600ms with both no pipeline set and 2 pipelines unpaused statuses, then loginned. I still did not see the centered loading spinner... How to reproduce this? Thanks!

@aoldershaw
Copy link
Contributor Author

@zoetian ah, should probably have made it more clear in the PR description. The loading screen only appears before any pipelines come back, which could be over the network, but most of the time it'll come from cache first. So 90% of the time, you won't even see the loading screen since the pipelines are almost always cached, but e.g. on the first load in a while, this may be useful.

So, basically, to reproduce it, you'll need to throttle your network as you did, and clear the pipelines field in localStorage (can do that in the Application tab in the dev console)

Copy link
Member

@zoetian zoetian left a comment

Choose a reason for hiding this comment

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

👍 LGTM!
(It's actually quite hard to reproduce this in browser for me. Even after clear the localStorage for pipelines and jobs...)

@aoldershaw aoldershaw merged commit 77ab1d1 into master Apr 19, 2020
@aoldershaw aoldershaw deleted the issue/2935-master branch April 19, 2020 22:12
@jwntrs jwntrs added the release/documented Documentation and release notes have been updated. label Apr 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release/documented Documentation and release notes have been updated.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

loading indicator for dashboard page
3 participants