-
Notifications
You must be signed in to change notification settings - Fork 3
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
Mdct 2000 #4236
Conversation
@@ -24,6 +25,7 @@ export const Form = ({ | |||
...props | |||
}: Props) => { | |||
const { fields, options } = formJson; | |||
const selectedReport = localStorage.getItem("selectedReport"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is gonna be a problem for non-report forms, like the admin banner page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gmrabian I got a solution added in for the admin banner, let me know what you think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment - need solution for admin banner form
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you could even abstract this one level further and put it in the ReportPageWrapper, which renders each of the StandardReportPage, DrawerReportPage, and ModalDrawerReportPage
Wrap this in a spinner: https://github.com/CMSgov/mdct-mcr/blob/main/services/ui-src/src/components/reports/ReportPageWrapper.tsx#L77 ?
That's what I understood from the slack thread
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now we're talking! That's clean as Mr. Clean
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool! 👍
Description
Added a loading state for hydrating field values
How to test
Click through a form that's been filled out and watch the spinner spin!
Changed Dependencies
Code author checklist
Reviewer checklist (two different people)