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.

Sign up for GitHub

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

Jump to bottom

[dashboard] Add loading indicator to Prebuilds page #6984

Merged
merged 3 commits into from
Nov 30, 2021
Merged

[dashboard] Add loading indicator to Prebuilds page #6984

merged 3 commits into from
Nov 30, 2021

Conversation

jankeromnes
Copy link
Contributor

@jankeromnes jankeromnes commented Nov 30, 2021 •
edited
Loading

Description

Adds a loading indicator to the Prebuilds page.

Also drive-by disables an annoying ESLint rule, and moves a component into the correct directory.

Related Issue(s)

Fixes #5351

How to test

  1. Open the Prebuilds page of any project, briefly notice the new loading indicator

Release Notes

[dashboard] Add loading indicator to Prebuilds page

Documentation

/uncc

@jankeromnes jankeromnes mentioned this pull request Nov 30, 2021
Closed
@jankeromnes jankeromnes marked this pull request as ready for review November 30, 2021 13:53
@gtsiolis
Copy link
Contributor

gtsiolis commented Nov 30, 2021 •
edited by werft-gitpod-dev-com bot
Loading

Looking at this now! 👀

gtsiolis
gtsiolis approved these changes Nov 30, 2021
View reviewed changes
Copy link
Contributor

@gtsiolis gtsiolis left a comment

Choose a reason for hiding this comment

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

UX works like a charm, @jankeromnes! 🔮

Great to see the spinner being reused here. 🍔

Thinking out loud, the natural next step could be to tackle #5022 which affects new projects but definitely out of the scope of this PR. Cc @jldec

Approving to unblock merging, but holding in case this needs a closer look at the code changes from someone else on the team.

/hold

5w2zzs

@roboquat roboquat added the lgtm label Nov 30, 2021
@roboquat
Copy link
Contributor

LGTM label has been added.

Git tree hash: 73cffa678954fe3686051c82b3656acb1f57cc43

@roboquat
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gtsiolis

Associated issue: #5351

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jankeromnes
Copy link
Contributor Author

jankeromnes commented Nov 30, 2021 •
edited by werft-gitpod-dev-com bot
Loading

Best. Review. Ever! 😂 🙌 Many thanks @gtsiolis!

I think these changes are safe enough for a single review. 😇 🚀

/unhold

@roboquat roboquat merged commit b196c71 into main Nov 30, 2021
@roboquat roboquat deleted the jx/prebuilds-loading-indicator branch November 30, 2021 16:48
@roboquat roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Dec 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gtsiolis gtsiolis gtsiolis approved these changes

Assignees

@gtsiolis gtsiolis

Labels
approved component: dashboard deployed: webapp Meta team change is running in production deployed Change is completely running in production release-note size/S team: webapp Issue belongs to the WebApp team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add loading indicator when fetching branches and prebuilds
3 participants
@jankeromnes @gtsiolis @roboquat