-
Notifications
You must be signed in to change notification settings - Fork 8
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
#1583 Abstracted Refund Modal #2680
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.
One small comment and a question but otherwise looks good
interface RefundModalProps { | ||
showModal: boolean; | ||
handleClose: () => void; | ||
mutateAsync: (data: any) => void; // if anyone can figure out how to change this without raising type errors, be my guest |
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.
what did you want to change it to? Like whats wrong with this
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.
Just that it uses any
, but I think I know how to fix it now
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.
Just some merge conflicts then lgtm
Changes
Made an abstract refund modal that is used by the report refund modal and created an edit refund modal that also uses it
Screenshots
If you made UI changes you must post a screenshot of the whole page for each change in 1) a normal sized window and 2) the smallest possible window
If you did any manual testing (e.g., with Postman), put screenshots of the http request and before and after of the db
If none of this applies, you can delete this section
Checklist
It can be helpful to check the
Checks
andFiles changed
tabs.Please review the contributor guide and reach out to your Tech Lead if anything is unclear.
Please request reviewers and ping on slack only after you've gone through this whole checklist.
yarn.lock
changes (unless dependencies have changed)Closes #1583