Skip to content

Show loading spinner when loading team members #16417

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
Merged
merged 1 commit into from
Mar 9, 2023
Merged

Show loading spinner when loading team members #16417

merged 1 commit into from
Mar 9, 2023

Conversation

RayAsh37
Copy link
Contributor

@RayAsh37 RayAsh37 commented Feb 15, 2023 •
edited by gtsiolis
Loading

replaced "no members found" with a spinner

Description

Replaced the "no members found" with a spinner

Related Issue(s)

Fixes #16241

Release Notes

Show loading spinner when fetching organization members

@RayAsh37 RayAsh37 requested a review from a team February 15, 2023 15:08
@RayAsh37 RayAsh37 requested a review from gtsiolis as a code owner February 15, 2023 15:08
@werft-gitpod-dev-com
Copy link

annotations in the pull request changed, but user is not allowed to start a job

@gtsiolis
Copy link
Contributor

gtsiolis commented Feb 15, 2023 •
edited by werft-gitpod-dev-com bot
Loading

/werft run with-preview=true

👍 started the job as gitpod-build-spinner-replaces-no-members-fork.0
(with .werft/ from main)

@RayAsh37 RayAsh37 mentioned this pull request Feb 15, 2023
Closed
@gtsiolis
Copy link
Contributor

gtsiolis commented Feb 15, 2023 •
edited
Loading

@RayAsh37:

  1. It looks like there's a lint error with the build[1].
  2. You'll also need to sign a CLA[1] once before merging your first contribution. Cc @meysholdt

@RayAsh37
Copy link
Contributor Author

@gtsiolis
Oops, the classic mistake of writing class instead of className. I have rectified it. Please review

@gtsiolis
Copy link
Contributor

gtsiolis commented Feb 16, 2023 •
edited
Loading

Thanks, @RayAsh37! 🏀

Friendly reminder, you'll need to sign the CLA linked above in #16417 (comment) before merging this.

/werft run with-preview=true

👍 started the job as gitpod-build-spinner-replaces-no-members-fork.1
(with .werft/ from main)

gtsiolis
gtsiolis suggested changes Feb 16, 2023
View reviewed changes
Copy link
Contributor

@gtsiolis gtsiolis left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@RayAsh37 Build is now passing and preview environment is now up and running. 💯

https://spinner-re9fbc403ef0.preview.gitpod-dev.com/workspaces

  1. Left one blocking comment below. 🏓
  2. Re-posting from above: you'll need to sign the CLA linked above in Show loading spinner when loading team members #16417 (comment) before merging this.

components/dashboard/src/teams/Members.tsx Outdated
<p className="pt-16 text-center">No members found</p>
<div className="flex h-screen items-center justify-center">
<div className="h-16 w-16 animate-spin rounded-full border-t-2 border-b-2 border-gray-200">
{/* Spinner */}
Copy link
Contributor

Choose a reason for hiding this comment

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

issue: Did you intent to use the Loader component here? 😁

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gtsiolis my bad 😅
Didn't realize that there was a standard component for that

I have added the SpinnerLoader instead of SpinnerOverlayLoader as the latter blocks the entire page which seems redundant

@gtsiolis
Copy link
Contributor

gtsiolis commented Feb 16, 2023 •
edited by werft-gitpod-dev-com bot
Loading

/werft run with-preview=true

👍 started the job as gitpod-build-spinner-replaces-no-members-fork.2
(with .werft/ from main)

@RayAsh37
Copy link
Contributor Author

  1. Re-posting from above: you'll need to sign the CLA linked above in #16417 (comment) before merging this.

@gtsiolis I have signed the CLA. How do I submit it?

gtsiolis
gtsiolis approved these changes Feb 16, 2023
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 LGTM! 🌮 🌮

I have signed the CLA. How do I submit it?

@RayAsh37 If you filled in and submitted the form linked above in #16417 (comment), @meysholdt will reach out here to confirm and unblock this from merging.

Approving to unblocking merging but holding until CLA signing has been confirmed.

/hold

@gtsiolis
Copy link
Contributor

@RayAsh37 In the meantime, could you rebase and squash the commits?

@RayAsh37
Copy link
Contributor Author

RayAsh37 commented Feb 16, 2023 •
edited by werft-gitpod-dev-com bot
Loading

@RayAsh37 In the meantime, could you rebase and squash the commits?

@gtsiolis Done! 😊

@gtsiolis
Copy link
Contributor

gtsiolis commented Feb 16, 2023 •
edited by werft-gitpod-dev-com bot
Loading

/werft run with-preview=true

👍 started the job as gitpod-build-spinner-replaces-no-members-fork.3
(with .werft/ from main)

@gtsiolis
Copy link
Contributor

Thanks, @RayAsh37! Made some minor changes to the commit message.

I'll come back here once @meysholdt confirms the CLA signing on our side.

@gtsiolis gtsiolis assigned meysholdt and unassigned RayAsh37 Feb 16, 2023
@meysholdt
Copy link
Member

Thank you, @RayAsh37, for signing our CLA!

@meysholdt meysholdt assigned gtsiolis and unassigned meysholdt Mar 6, 2023
@gtsiolis gtsiolis self-requested a review March 6, 2023 20:42
@gtsiolis gtsiolis removed the do-not-merge/cla-pending CLA has not been signed label Mar 9, 2023
@gtsiolis gtsiolis changed the title added spinner Show loading spinner when loading team members Mar 9, 2023
gtsiolis
gtsiolis reviewed Mar 9, 2023
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.

Thanks for your patience, @RayAsh37!

Confirmed the signed CLA, removing hold.

/unhold

@gtsiolis
Copy link
Contributor

gtsiolis commented Mar 9, 2023 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-spinner-replaces-no-members-fork.4
(with .werft/ from main)

@roboquat roboquat merged commit 57e4149 into gitpod-io:main Mar 9, 2023
@RayAsh37 RayAsh37 deleted the spinner-replaces-no-members branch March 9, 2023 16:31
@gtsiolis gtsiolis mentioned this pull request Mar 10, 2023
Merged
16 tasks
@gtsiolis gtsiolis mentioned this pull request Aug 10, 2023
Merged
14 tasks
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
cla: accepted ✔️ community-contribution release-note size/XS
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Show a spinner when waiting for TeamMembers to be retrieved
4 participants
@RayAsh37 @gtsiolis @meysholdt @roboquat