-
-
Notifications
You must be signed in to change notification settings - Fork 794
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
Added React Todo Application #100
Conversation
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/atapas/react-play/EbQLckCE769pmdoMyRcpY3r1R1ea |
@atapas review it Sir |
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.
@nirban256 mostly looks good. Please take care of the review comments to get it merged asap.
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.
@nirban256 A few more comments about the styling.
I suggest not adding a todo if the input has no value. |
Yes I have provided that comment too. |
@atapas I have removed the local Storage functionality as it was not working and made the other changes, but couldn't change the page level scroll |
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.
@nirban256 A few more cosmetic changes.
made the changes @atapas Sir |
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 one more comment I gave and
DO NOT COMMIT the yarn.lock file as there are no changes in the package.json file.
I added the yarn.lock in the gitignore file and made the change. Is it correct now? @atapas |
Yes that's right.. add it in .gitignore but also do not push your yarn lock file... |
@atapas Can you tell me how can I not push the yarn.lock file? |
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.
@nirban256 looks good.
I shall remove it in a different PR |
Description: Added a React Todo Application which keeps track of daily todos that need to be done.
Fixes # (issue) : 91
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce.
Checklist: