-
-
Notifications
You must be signed in to change notification settings - Fork 854
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
Create calendar react-play #198
Conversation
Someone is attempting to deploy a commit to a Personal Account owned by @atapas on Vercel. @atapas first needs to authorize it. |
Review ready |
@vincentBCP kindly edit and update the PR description. |
Updated the PR description. |
Thanks @vincentBCP. You also need to create the issue for this play. I have done it: #199 Could you please check and add a bit about the play there? Thanks. We will start the review shortly. |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
@vincentBCP It is a cool play. Just played around with the preview URL. Looks awesome.
I have provided my initial review comments. @koustov will jump into doing the reviews of it as well.
One thing I noticed is, that the UI breaks a bit on mobile. If you can take care of the responsiveness as well, will be great.
Thanks for the review @atapas . I'll work on the suggestions and issues. |
Review ready |
@vincentBCP Would you take up validation for cases like this? Like start date is ahead of the end date? |
Sure. I'll add this validation. |
I already pushed the changes for time validation, endTime should not be before the startTime now. |
Thanks for the review @koustov . I'll be working on request changes. |
Review ready |
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.
LGTM
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.
LGTM
@vincentBCP Please resolve the merge conflicts |
Resolved merge conflicts. |
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.
LGTM
@all-contributors please add @vincentBCP for Code |
I've put up a pull request to add @vincentBCP! 🎉 |
Description
A simple calendar app to add, update, and delete events.
Dependecies:
Fixes # (issue) #199
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: