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

feat: add loader #12

Merged
merged 2 commits into from Oct 17, 2020
Merged

feat: add loader #12

merged 2 commits into from Oct 17, 2020

Conversation

ansh-saini
Copy link
Contributor

@ansh-saini ansh-saini commented Oct 16, 2020

Feature #5

@vercel
Copy link

vercel bot commented Oct 16, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/anup-a/svgwave/gbpncm6xi
✅ Preview: https://svgwave-git-main.anup-a.vercel.app

@anup-a
Copy link
Owner

anup-a commented Oct 17, 2020

I'm not sure if this is the right way to add loader.

I use to do it using onload attribute on body tag.

@ansh-les
Copy link
Contributor

@anup-a I've changed the implementation. Please take a look.
Although the loader doesn't really show up because the app loads almost instantly. Should I add a setTimeout of 1s to make it somewhat visible?

@anup-a
Copy link
Owner

anup-a commented Oct 17, 2020

@ansh-les it is good enough... ( though I couldn't even see the loader once 😅😄)
Lets leave as it is for now.

@anup-a
Copy link
Owner

anup-a commented Oct 17, 2020

Great work, I think it is ready to merge.
Give us a ⭐ on the repo.

@ansh-saini
Copy link
Contributor Author

Yay! 👍

@anup-a anup-a merged commit fc0e34a into anup-a:main Oct 17, 2020
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.

None yet

3 participants