-
Notifications
You must be signed in to change notification settings - Fork 0
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
Added close button to modal #194
Conversation
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.
Works great. Could you still run Prettier and put the "float: right" into a styled component?
mscr-ui/src/modules/form/index.tsx
Outdated
{(modalType == ModalType.RegisterNewFull || | ||
modalType == ModalType.RevisionFull) && | ||
renderFileDropArea()} | ||
<div id={'modalTop'}></div> |
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.
The indentations are a bit all over the place, maybe run Prettier to get the formatting in order? I don't always remember to do that, but it would be a good habit to do when you've finished developing.
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.
formatting is fixed
mscr-ui/src/modules/form/index.tsx
Outdated
@@ -543,7 +544,8 @@ export default function FormModal({ | |||
} | |||
|
|||
return ( | |||
<Modal | |||
<> |
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.
You don't need to add a fragment if you're returning just one element, like here everything is already inside the Modal element. It doesn't have any ill effect, but just reading the code it makes you expect a list of elements.
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.
fixed
mscr-ui/src/modules/form/index.tsx
Outdated
</ModalTitle> | ||
</div> | ||
<div className="col-4"> | ||
<Button style={{ float: 'right' }} variant="secondaryNoBorder" |
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 we should work towards having all styling done in styled components, so that it's easier to change. Then, when you want to change how a component looks, you can just go to the .styled file of the component and find all style declarations there, instead of some things being inline.
Added X icon to the form modals and mapping modals to close the modals