Add support for seconds in date_utils #4690
Conversation
![pullrequest[bot]](https://avatars.githubusercontent.com/in/3434?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ 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.
@yykcool you can click here to see the review status or cancel the code review job.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4690 +/- ##
==========================================
+ Coverage 97.00% 97.01% +0.01%
==========================================
Files 28 28
Lines 2641 2650 +9
Branches 1119 1137 +18
==========================================
+ Hits 2562 2571 +9
Misses 79 79 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
The changes look good but I just have one concern regarding correct testing.
Reviewed with ❤️ by PullRequest
|
|
||
| expect( | ||
| Array.from(disabledTimeItems).map((node) => node.textContent), | ||
| ).toContain("01:00:00"); |
There was a problem hiding this comment.
includeTimes tells the component to mark any pre-existing times (derived from the interval) that are not in the array as disabled.
this part checks that 01:00:00 and 00:00:00 are correctly disabled because they are not included
Problem
missed out support for seconds in functions in
date_utils.jsChanges
added second support
Contribution checklist