-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
(feature) swapRange optional prop #4654
Conversation
…component that allows the functionality of swapping dates if we want to allow the end user to pick dates in any order. This is a real request from the end users, and it will be handy if you work with big date ranges.
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.
✅ This pull request was sent to the PullRequest network for review. Expert reviewers are now being matched to your request based on the code's requirements. Stay tuned!
What to expect from this code review:
- Comments posted to any areas of potential concern or improvement.
- Detailed feedback or actions needed to resolve issues that are found.
- Turnaround times vary, but we aim to be swift.
@mirus-ua you can click here to see the review status or cancel the code review job.
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.
PullRequest Breakdown
Reviewable lines of change
+ 73
- 1
51% JavaScript
49% JavaScript (tests)
Type of change
Feature - These changes are adding a new feature or improvement to existing code.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4654 +/- ##
=======================================
Coverage 96.99% 97.00%
=======================================
Files 28 28
Lines 2632 2635 +3
Branches 1114 1116 +2
=======================================
+ Hits 2553 2556 +3
Misses 79 79 ☔ View full report in Codecov by Sentry. |
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.
Looks good & nice feature. Just added a small comment about a potential future bug if someone edits the else if
statement ordering.
Reviewed with ❤️ by PullRequest
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.
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.
Reading the code carefully, I didn't see any defects. I gave a comment related to fixed dates in tests.
Hopefully this review helped you.
Reviewed with ❤️ by PullRequest
Great feature, I can't wait to get this feature in npmjs :) |
name:
swapRange
propabout: Gives more flexibility on input flow
Problem
When a user works with extensive date ranges, it could be tedious to scroll back and forth if the user makes a mistake.
Changes
The change adds an optional property
swapRange
toDatePicker
component that allows the user to swap dates if we want the end user to pick dates in any order. This is a real request from the end users, and it will be handy if you work with big date ranges.Screenshots
Contribution checklist