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 skeleton loader for modal connecting state #34

Closed
rolznz opened this issue Aug 25, 2023 · 5 comments · Fixed by #70
Closed

Add skeleton loader for modal connecting state #34

rolznz opened this issue Aug 25, 2023 · 5 comments · Fixed by #70
Assignees
Labels

Comments

@rolznz
Copy link
Collaborator

rolznz commented Aug 25, 2023

Currently shows just a spinner which doesn't look good and causes the modal height to jump. Instead it should show the connected content, with skeleton loaders for any content that is loading.

@rolznz rolznz added the good first issue Good for newcomers label Sep 1, 2023
@hirenchauhan2
Copy link
Contributor

I can help on this, could you please guide where exactly I need to make changes? On React wrapper or to the Lit components?

@rolznz
Copy link
Collaborator Author

rolznz commented Oct 2, 2023

Hi @hirenchauhan2 the change needs to be made on the bc-modal lit component. Thanks!

@hirenchauhan2
Copy link
Contributor

hirenchauhan2 commented Oct 2, 2023

@rolznz What do you think of this kind of design for loading state, it's pulse animation from Tailwind. Please share your thoughts on this. I've adjusted these items in proper spacing which resembles final state of the modal. (Using mock data for dev/testing)

Loading state

image

Connected state

image

@hirenchauhan2
Copy link
Contributor

@rolznz Please check the raised PR #70 and let me know if there are any changes needed.

@hirenchauhan2
Copy link
Contributor

This is how I mocked the state of the modal

image

@rolznz rolznz closed this as completed in #70 Oct 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants