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

Generic modal #64

Merged
merged 9 commits into from
Oct 4, 2022
Merged

Generic modal #64

merged 9 commits into from
Oct 4, 2022

Conversation

jhughes982
Copy link
Collaborator

  • Added a generic modal that can display as an error modal or an info modal,
  • Modal type (error vs. info) is dictated by passed in boolean prop isError,
  • Modal styles are automatically populated based on the modal type,

@jhughes982 jhughes982 added the enhancement New feature or request label Oct 4, 2022
@jhughes982 jhughes982 requested a review from stefpiatek October 4, 2022 11:24
@jhughes982 jhughes982 self-assigned this Oct 4, 2022
@jhughes982 jhughes982 temporarily deployed to test-env October 4, 2022 11:57 Inactive
@jhughes982 jhughes982 mentioned this pull request Oct 4, 2022
Copy link
Collaborator

@stefpiatek stefpiatek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fun, enjoyed this! Appreciate your work on it

width: 100%;
height: 100vh;
background: rgba(0, 0, 0, 0.75);
z-index: 10;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh this is fun

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which part? 😅

Comment on lines 69 to 77
.then((response) => console.debug("Bundle submitted", bundle, response))
.catch((error) => console.error(error));
.catch((error) => {
console.error(error)
setModal({
message: 'Something went wrong submitting the bundle. Please try again later.',
isError: true,
});
});
actions.setSubmitting(false);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably also worth going through the response entries and if any of the response codes don't start with 2, then add a modal message to saying that there was an error with the FHIR resource. If you'd like some example bundles which will cause an error let me know and I can make some for you

@@ -104,6 +113,9 @@ const ReportForm: FC<Props> = (props: Props) => {
};

return (
<>
<ModalWrapper isError={modal?.isError} modalMessage={modal?.message} onClear={() => setModal(null)} />
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have expected the indentation to change here.. strange

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, my auto formatter has stopped working for some reason. I must have missed this one! I'll make sure to get prettier set up again!

isError={isError}
header={isError ? 'Error' : "Info"}
show={!!modalMessage}
footer={<button className={classes.modal__btn} onClick={onClear}>Okay</button>}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh one thing is that maybe the okay should also change colour if there's an error - red and purple look a bit jarring to my eyes at least

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, good call!

@jhughes982 jhughes982 temporarily deployed to test-env October 4, 2022 16:19 Inactive
@jhughes982 jhughes982 temporarily deployed to test-env October 4, 2022 16:25 Inactive
@jhughes982 jhughes982 merged commit 84633ed into main Oct 4, 2022
@jhughes982 jhughes982 deleted the generic-modal branch October 4, 2022 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants