-
Notifications
You must be signed in to change notification settings - Fork 111
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
Reset modal after adding event #442
base: main
Are you sure you want to change the base?
Changes from all commits
300aa5f
7502e4f
99c0d3d
40586ee
b1d9446
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -2,7 +2,7 @@ import { AnimatePresence } from "framer-motion"; | |||||
import Backdrop from "./Backdrop"; | ||||||
import { motion } from "framer-motion"; | ||||||
|
||||||
const Modal = ({ children, context }) => { | ||||||
const Modal = ({ children, context, onBackdropClick }) => { | ||||||
const dropIn = { | ||||||
hidden: { | ||||||
y: "-100vh", | ||||||
|
@@ -32,7 +32,12 @@ const Modal = ({ children, context }) => { | |||||
onExitComplete={() => null} | ||||||
> | ||||||
{context.isOpen && ( | ||||||
<Backdrop onClick={context.handleClose}> | ||||||
<Backdrop | ||||||
onClick={() => { | ||||||
context.handleClose(); | ||||||
if (typeof onBackdropClick === "function") onBackdropClick(); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this conditional should silently pass if a "function" typed If someone is adding more modals later but accidentally passes an Instead, it might be better to just check for the existence of
Suggested change
|
||||||
}} | ||||||
> | ||||||
<motion.div | ||||||
className="modal overflow-y-auto" | ||||||
onClick={e => e.stopPropagation()} | ||||||
|
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.
Optionally you could add a check for the
onBackdropClick
type withprop-types
. That way you get an error on page load if an invalidonBackdropClick
prop is passed in instead of at runtime.Like 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.
If a redesign for the event modal is in the works like @intelagense mentioned then this may not be that important. Just my two cents.
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'm not sure the redesign will be coming anytime soon.