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

add a loading spinner during gerenating quiz #119 #155

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rahulkumarbhagat-coder
Copy link

This PR introduces a loading spinner to enhance user experience by providing visual feedback during asynchronous operations, such as data fetching or form submission. This deal with issue #119

@ts-31
Copy link

ts-31 commented Jan 20, 2025

Hi @rahulkumarbhagat-coder, nice work on the PR! I noticed you’ve used the react-loader-spinner package to implement the spinner. The project currently already has a spinner implemented during quiz generation.

Could you clarify if using react-loader-spinner introduces any improvements over the existing implementation? For example:

Does it provide better design or animation options?
Is it more maintainable or flexible for future updates?
Also, adding a new package increases the dependency footprint, so it might be worth discussing whether the existing spinner could be enhanced instead of introducing a new one. Let me know your thoughts! 😊

@rahulkumarbhagat-coder
Copy link
Author

Hi @ts-31 , Thank you for your feedback and for taking the time to review the PR!

Use of the react-loader-spinner package was aimed at leveraging its customizable design and animation options. Compared to the existing implementation, the new spinner might bring visual consistency and flexibility.

Since react-loader-spinner is a widely used and actively maintained package, it simplifies updates or enhancements.

That said, you’ve raised an important point about dependency management. Adding a new package does increase the project’s footprint, so it’s worth evaluating whether the existing spinner could be enhanced to meet our needs without introducing a new dependency.

I’d be happy to enhance the existing spinner if the team prefers to avoid additional dependencies. Alternatively, if the new spinner aligns better with our long-term goals (e.g., design consistency or extensibility), we could discuss whether it’s worth adopting react-loader-spinner.

@Aditya062003
Copy link
Contributor

Hey, this feature is already being worked on here: #135

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants