-
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
Vps 45/daisy UI modals #249
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.
Nice job overall—few things to change :)
<DeleteButton onClick={handleDelete}>Delete</DeleteButton> | ||
</DialogActions> | ||
</Dialog> | ||
<dialog id="my_modal_5" className="modal modal-bottom sm:modal-middle"> |
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.
Could we give it a more descriptive ID?
package-lock.json
Outdated
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.
This file should not be included—we are using yarn
, not npm
!
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.
let's get this merged!
Describe the issue
Modals were using MaterialUI. Need consistent design.
Describe the solution
Refactored modals to use DaisyUI components.
create scenario modal
delete scenario modal
help modal
share modal
reset modal
back modal
image modal
Risk
Buttons are still using MaterialUI.
Definition of Done
Reviewed By
Who reviewed your PR - for commit history once merged