-
Notifications
You must be signed in to change notification settings - Fork 65
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
feat(component): convert Modal to FC #317
Conversation
cf441ee
to
0c2534c
Compare
01bd5d3
to
c86e988
Compare
c86e988
to
592d43a
Compare
}, []); | ||
|
||
useEffect(() => { | ||
return () => { |
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 fully clear on why a function is returned from this useEffect() - would you mind explaining?
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.
Returning a function on a useEffect
is roughly the same as componentWillUnmount
. A common use-case for using this cleanup functionality is removing event listeners on the DOM:
useEffect(() => {
window.addEventListener('resize', handleResize);
return () => {
window.removeEventListener('resize', handleResize);
};
});
This is extremely useful when components get added/removed from the DOM frequently.
Here's some more information on the effect cleanup: https://reactjs.org/docs/hooks-reference.html#cleaning-up-an-effect
In our use-case we want the users focus to return back to their last focusable element. We use portals to add the modal onto the page so it'll mount/unmount every time it open/closes. Since we want to return focus, we run it on the effect cleanup.
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.
Oh thanks! I remember that now that you mention it. It's been a while since I've used the useEffect hook, will brush up on it again for my current work.
} | ||
}; | ||
} | ||
return !(isOpen && modalContainer) ? null : createPortal(renderedContent, modalContainer); |
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 is the difference between this and return isOpen && modalContainer ? createPortal(renderedContent, modalContainer) : null
?
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.
Good catch, I can change it over.
592d43a
to
34c0e8e
Compare
What
Convert
Modal
to functional component.Till Hacktember Ends