-
Notifications
You must be signed in to change notification settings - Fork 54
Changed loading page to have a spinning animation. #219
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
base: main
Are you sure you want to change the base?
Conversation
|
@Kr-Utkarsh-01 is attempting to deploy a commit to the openlake's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe LoadingScreen component in the frontend has been enhanced to display a fullscreen, centered loading interface with a spinner animation and prominent text, replacing the previous basic loading markup for improved user experience during loading states. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@frontend/src/Components/common/LoadingScreen.js`:
- Around line 4-6: Update the LoadingScreen component's root container div to
include accessibility attributes role="status" aria-live="polite" and
aria-busy="true", change the spinner element's class from "animate-spin" to
"motion-safe:animate-spin" to respect reduced-motion preferences, and add
aria-hidden="true" to the spinner div so only the container is announced; modify
the elements referenced as the outer container div and the spinner div in
LoadingScreen.js accordingly.
| <div className="fixed inset-0 bg-[#FDFAE2] flex flex-col items-center justify-center z-50"> | ||
| <div className="animate-spin rounded-full h-16 w-16 border-t-4 border-b-4 border-black mb-4"></div> | ||
| <div className="text-xl font-semibold text-black">Loading...</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add loading ARIA semantics and honor reduced‑motion.
Screen readers won’t announce the loading state, and animate-spin ignores reduced‑motion preferences. Add role="status" aria-live="polite" aria-busy="true" on the container, and switch to motion-safe:animate-spin with aria-hidden on the spinner.
Suggested fix
- <div className="fixed inset-0 bg-[`#FDFAE2`] flex flex-col items-center justify-center z-50">
- <div className="animate-spin rounded-full h-16 w-16 border-t-4 border-b-4 border-black mb-4"></div>
+ <div
+ className="fixed inset-0 bg-[`#FDFAE2`] flex flex-col items-center justify-center z-50"
+ role="status"
+ aria-live="polite"
+ aria-busy="true"
+ >
+ <div
+ className="motion-safe:animate-spin rounded-full h-16 w-16 border-t-4 border-b-4 border-black mb-4"
+ aria-hidden="true"
+ ></div>
<div className="text-xl font-semibold text-black">Loading...</div>
</div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div className="fixed inset-0 bg-[#FDFAE2] flex flex-col items-center justify-center z-50"> | |
| <div className="animate-spin rounded-full h-16 w-16 border-t-4 border-b-4 border-black mb-4"></div> | |
| <div className="text-xl font-semibold text-black">Loading...</div> | |
| <div | |
| className="fixed inset-0 bg-[`#FDFAE2`] flex flex-col items-center justify-center z-50" | |
| role="status" | |
| aria-live="polite" | |
| aria-busy="true" | |
| > | |
| <div | |
| className="motion-safe:animate-spin rounded-full h-16 w-16 border-t-4 border-b-4 border-black mb-4" | |
| aria-hidden="true" | |
| ></div> | |
| <div className="text-xl font-semibold text-black">Loading...</div> | |
| </div> |
🤖 Prompt for AI Agents
In `@frontend/src/Components/common/LoadingScreen.js` around lines 4 - 6, Update
the LoadingScreen component's root container div to include accessibility
attributes role="status" aria-live="polite" and aria-busy="true", change the
spinner element's class from "animate-spin" to "motion-safe:animate-spin" to
respect reduced-motion preferences, and add aria-hidden="true" to the spinner
div so only the container is announced; modify the elements referenced as the
outer container div and the spinner div in LoadingScreen.js accordingly.
name: " Pull Request"
about: Propose and submit changes to the project for review
title: "PR: [Changed loading page to have a spinning animation instead of "Loading..." text.]"
labels: ""
Related Issue
Changes Introduced
Screenshots / Video (if applicable)
| Before |


| After |
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.