-
Notifications
You must be signed in to change notification settings - Fork 1.9k
add modal for reset button #163
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
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.
Thank you so very much for this @philiplee13 - it's a change that I'm sure many others will appreciate! 😁
I've tested it locally and everything is good to go - I'd just ask that you trim off the comments you've added. I think the variable/function names are descriptive enough that we can omit them, otherwise we might duplicate meaning. Comments might be useful when we need to dictate why something was done the way it is, for better or worse.
Let me know if you need any help! We're almost across the finish line! 👏🏽
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.
A very big thank you for picking this up and following through @philiplee13!
Hey @seanprashad - Sorry this took a little bit, still new to the whole react eco system / web development in general.
I have the changes below - basically just a simple modal from "reactstrap" and when the "Reset" button is toggled - it changes the state to show the modal
gh-screen-recording.mov
I originally had tried to make a separate folder under "components" for the modal + logic for it - but I found that moving the "resetHandler" was a bit more tricky than I had planned - so I opted in to just adding the state / modal directly into the "table/index.js"