added Modal component, demo, and docs #47
Conversation
|
Hey, this is neat. Any reason to use refs in this situation? I had initially thought users could just pass in their own open/close function but that would be outside the modal. what do you think? |
|
Good idea, I'm going to try a different approach and I'll keep you updated. |
|
After thinking about this for a bit I think the best thing is to have defined open/close functionality. If the user wants to run custom functions then we can have props for functions that will execute when the modal is opened/closed. Also, the ref is in order to allow for the modal to close when the background is clicked. If this isn't necessary, I can remove the ref and just have the close functionality via an 'x' button in the corner of the modal. |
|
Here is the new version. There aren't any refs. In order to open, set |
|
@NehemiahK Where are we with this? |
|
|
||
| import './Modal.scss'; | ||
|
|
||
| const Modal = (props) => { |
There was a problem hiding this comment.
Can we please destructure the props. Also, make sure the to install the new dependencies using npm install and install the ESLint plugin from this link;
There was a problem hiding this comment.
Also, if required take a pull from the master so that you have the latest code. Rebase your branch with the master and then push the code.
There was a problem hiding this comment.
@madhuni Thanks for the insights, I have implemented your suggestions. Regarding ES lint, when I start the project after installing the new dependencies I get an error that CRA uses "eslint": "^6.6.0" and the project uses 7.4.0. Did you see this?
I deleted the node_modules and package-lock.json and reinstalled but it didn't help. Thanks for the help.
There was a problem hiding this comment.
Okie let me have a look. Will provide you the solution to resolve this. I didn't see any such problem.
I added the Modal component along with the corresponding demo and docs.