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

SQFormDialogStepper, SQFormDialog add loading state the dialogs #70

Closed
Chris-Boyle opened this issue Jan 13, 2021 · 4 comments · Fixed by #71
Closed

SQFormDialogStepper, SQFormDialog add loading state the dialogs #70

Chris-Boyle opened this issue Jan 13, 2021 · 4 comments · Fixed by #71
Assignees
Labels
enhancement New feature or request released

Comments

@Chris-Boyle
Copy link
Contributor

Chris-Boyle commented Jan 13, 2021

In our dialogs we are not currently handling loading state. There are times when we open the dialog and are not ready to render the components. In this scenario we should show a loading spinner until the consumer says it is not loading anymore. Currently a consumer can do this but it would make consumer code much cleaner to have the dialogs handle it.

I am proposing we show the Select Quote circles as a loading spinner.

@Chris-Boyle Chris-Boyle added the enhancement New feature or request label Jan 13, 2021
@Chris-Boyle Chris-Boyle self-assigned this Jan 13, 2021
@20BBrown14
Copy link
Contributor

I am proposing we show the Select Quote circles as a loading spinner.

I suggest we allow the consumer to pass an optional loading component as well

@Chris-Boyle
Copy link
Contributor Author

I am proposing we show the Select Quote circles as a loading spinner.

I suggest we allow the consumer to pass an optional loading component as well

I think we should probably solve for the 80% and just use our SelectQuote loading spinner for now until there is a request to allow consumers to define one.

Chris-Boyle added a commit that referenced this issue Jan 13, 2021
@SeanGroff
Copy link
Contributor

I can see this for the Stepper as it's more of an all in one solution.
I don't believe loading states are a concern of Dialogs though.

I think the first problem to solve is building a shared component that will take up 100% of the parent elements height and width. Then center the loading rings and below the rings an optional loading message. Then this styled loading component can be used anywhere without having to do something like

<Dialog>
  {isLoading ? <div style={loadingStyle}><LoadingSpinner /></div> : content}
</Dialog>

instead it would be:

{isLoading ? <LoadingSpinner /> : content}

Styling is already done for the consumer.

SeanGroff pushed a commit that referenced this issue Jan 14, 2021
## [2.8.0](v2.7.0...v2.8.0) (2021-01-14)

### Features

* 🎸 Added proptypes and moved loading to step ([dd37b16](dd37b16))
* 🎸 Adding loading state to dialogs ([0f4d01d](0f4d01d)), closes [#70](#70)
* 🎸 Moved styling to makeStyles instead of css ([89282c3](89282c3))
* 🎸 removed loading message ([f1fdfb6](f1fdfb6))
* 🎸 removed proptype ([b09c596](b09c596))
* 🎸 Removed unused stories, added default prop value ([56ebd87](56ebd87))
* 🎸 updated proptype and fixed setvalues call of undefined ([4948c98](4948c98))
* 🎸 updated styles for typography to remove warning ([9788a1a](9788a1a))
* 🎸 Updated the typography message style and default messag ([9295b45](9295b45))
@SeanGroff
Copy link
Contributor

🎉 This issue has been resolved in version 2.8.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants