search

open-sauced / hot

🍕The site that recommends the hottest projects on GitHub.
https://hot.opensauced.pizza
MIT License
426 stars 148 forks source link

feat: add Loading skeleton in Hero #308

Closed OgDev-01 closed 2 years ago

OgDev-01 commented 2 years ago

What type of PR is this? (check all applicable)

Description

This PR adds loading skeleton to the hero section

Related Tickets & Documents

Feature #297

Mobile & Desktop Screenshots/Recordings

Desktop

Screenshot (25)

Mobile

Screenshot (30)

Added tests?

Added to documentation?

[optional] Are there any post-deployment tasks we need to perform?

[optional] What gif best describes this PR or how it makes you feel?

netlify[bot] commented 2 years ago

Deploy Preview for hot-sauced-ui ready!

Name Link
Latest commit a7536eefa5c2455baaed65d1c4d22a107bbc91f3
Latest deploy log https://app.netlify.com/sites/hot-sauced-ui/deploys/62f623e616f62d00087c69e8
Deploy Preview https://deploy-preview-308--hot-sauced-ui.netlify.app/
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

bdougie commented 2 years ago

Seeing this live, I am not convinced this is need or the correct solution.

Showing the skeleton on each load is more of a nuance. The 200 mili seconds is due the de ounce and intended

This issue needs more discussion @open-sauced/triage team.

OgDev-01 commented 2 years ago

Seeing this live, I am not convinced this is need or the correct solution.

Showing the skeleton on each load is more of a nuance. The 200 mili seconds is due the de ounce and intended

This issue needs more discussion @open-sauced/triage team.

Had same thoughts in mind about this solution... but still havent figured out what will best suite this case. I would suggest closing this issue till a better alternative is available.. just a suggestion though :)

0-vortex commented 2 years ago

Seeing this live, I am not convinced this is need or the correct solution.

Showing the skeleton on each load is more of a nuance. The 200 mili seconds is due the de ounce and intended

This issue needs more discussion @open-sauced/triage team.

Had same thoughts in mind about this solution... but still havent figured out what will best suite this case. I would suggest closing this issue till a better alternative is available.. just a suggestion though :)

I don't think this is bad actually. We are getting current results from supabase postgrest, switchning to the global backed api could increase the loading timer above 200ms, then there is the debounce, which 200ms is not enough, I would argue half a second is decent in terms of supporting mobile users who type at a rate of 300+. Increasing those values results in a smoother experience and then displaying 1 loader (more like a spinner tbh) could make things feel like home.

I do agree having 3 skeletons for potentially less than 3 results invalidates the requirement for content skeletons and the web 1.0 default for that is circular loading animations 😅

0-vortex commented 2 years ago

whatever ends up happening, kudos for the amazing effort! 🍕

bdougie commented 2 years ago

I do agree having 3 skeletons for potentially less than 3 results invalidates the requirement for content skeletons and the web 1.0 default for that is circular loading animations 😅

Circular or loading animations seems like a better approach. Can link to what other solutions are out there?

OgDev-01 commented 2 years ago

I do agree having 3 skeletons for potentially less than 3 results invalidates the requirement for content skeletons and the web 1.0 default for that is circular loading animations sweat_smile

Circular or loading animations seems like a better approach. Can link to what other solutions are out there?

Cool.. can I replace the skeleton with a spinner instead?

OgDev-01 commented 2 years ago

Or rather, a beat loader like this 👇 20220813_075209

NsdHSO commented 2 years ago

Indeed for our implementation it is a good approach to have circular. I suggest this lib, open to discussion react-loading, to avoid reinventing the wheel.

bdougie commented 2 years ago

Let's move the conversation back to the issue.

0-vortex commented 2 years ago

Indeed for our implementation it is a good approach to have circular. I suggest this lib, open to discussion react-loading, to avoid reinventing the wheel.

That library is not maintained and breaking in React 18, just a heads up.