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

[Live-Site] Feature/loading #641

Merged
merged 7 commits into from
Sep 28, 2023
Merged

Conversation

Pratiyushkumar
Copy link
Contributor

@Pratiyushkumar Pratiyushkumar commented Sep 13, 2023

Issue

What is the change?

There should be a loader shown while kicking out the peers from a live stream, so when we kick peers, there is a delay between the request sent to the backend and the response from the backend, so to cover that delay we are putting a loader.

Is Development Tested?

  • Yes
  • No

Change video recording

bandicam.2023-09-20.20-22-02-793.mp4

@Pratiyushkumar Pratiyushkumar changed the base branch from develop to develop-ember September 13, 2023 15:15
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Sep 13, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 7289f48
Status: ✅  Deploy successful!
Preview URL: https://b03f5dd2.www-rds.pages.dev
Branch Preview URL: https://feature-loading.www-rds.pages.dev

View logs

@Pratiyushkumar Pratiyushkumar marked this pull request as ready for review September 13, 2023 15:19
@Pratiyushkumar Pratiyushkumar self-assigned this Sep 14, 2023
Copy link

@prerana1821 prerana1821 left a comment

Choose a reason for hiding this comment

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

Hey @Pratiyushkumar,
image
I don't think we need both loading text and the animation.
You can remove the "Loading" text. Also, can the space between the dots decrease a little?

app/components/live-participants.hbs Outdated Show resolved Hide resolved
app/styles/live-sidebar.module.css Outdated Show resolved Hide resolved
@Pratiyushkumar
Copy link
Contributor Author

Hey @Pratiyushkumar, image I don't think we need both loading text and the animation. You can remove the "Loading" text. Also, can the space between the dots decrease a little?

removed loading text only animation is there

@satyam73
Copy link
Member

satyam73 commented Sep 19, 2023

The loader is overall good can we decrease the size of bubbles, a little and make it pink color(for all the three bubbles).

satyam73
satyam73 previously approved these changes Sep 27, 2023
Copy link
Member

@satyam73 satyam73 left a comment

Choose a reason for hiding this comment

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

LGTM

@satyam73 satyam73 changed the title Feature/loading [Live-Site] Feature/loading Sep 27, 2023
vinit717
vinit717 previously approved these changes Sep 27, 2023
@satyam73 satyam73 dismissed stale reviews from vinit717 and themself via 7289f48 September 28, 2023 07:48
@satyam73 satyam73 removed the request for review from prerana1821 September 28, 2023 10:51
Comment on lines +28 to +32
<div class='loading'>
<span></span>
<span></span>
<span></span>
</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT: this can be broken into a separate component

@prakashchoudhary07 prakashchoudhary07 merged commit 4d55640 into develop-ember Sep 28, 2023
@prakashchoudhary07 prakashchoudhary07 deleted the feature/loading branch September 28, 2023 10:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants