Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add support for seconds in date_utils #4690

Merged
merged 5 commits into from
Apr 15, 2024
Merged

Add support for seconds in date_utils #4690

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
49da266
add second support to isTimeDisabled, added test
Apr 15, 2024
2bf5d63
Merge branch 'Hacker0x01:main' into main
yykcool Apr 15, 2024
1fc36fa
update isTimeInDisabledRange to support seconds
Apr 15, 2024
7325c37
fix: clearer test
Apr 15, 2024
9af5770
fix: remove errorneous test assumptions
Apr 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 16 additions & 11 deletions src/date_utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,8 @@ export function isTimeInList(time, times) {
return times.some(
(listTime) =>
getHours(listTime) === getHours(time) &&
getMinutes(listTime) === getMinutes(time),
getMinutes(listTime) === getMinutes(time) &&
getSeconds(listTime) === getSeconds(time),
);
}

Expand All @@ -619,16 +620,20 @@ export function isTimeInDisabledRange(time, { minTime, maxTime }) {
if (!minTime || !maxTime) {
throw new Error("Both minTime and maxTime props required");
}
const base = newDate();
const baseTime = setHours(setMinutes(base, getMinutes(time)), getHours(time));
const min = setHours(
setMinutes(base, getMinutes(minTime)),
getHours(minTime),
);
const max = setHours(
setMinutes(base, getMinutes(maxTime)),
getHours(maxTime),
);
let baseTime = newDate();
baseTime = setHours(baseTime, getHours(time));
baseTime = setMinutes(baseTime, getMinutes(time));
baseTime = setSeconds(baseTime, getSeconds(time));

let min = newDate();
min = setHours(min, getHours(minTime));
min = setMinutes(min, getMinutes(minTime));
min = setSeconds(min, getSeconds(minTime));

let max = newDate();
max = setHours(max, getHours(maxTime));
max = setMinutes(max, getMinutes(maxTime));
max = setSeconds(max, getSeconds(maxTime));

let valid;
try {
Expand Down
11 changes: 7 additions & 4 deletions test/date_utils_test.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ import { setHours } from "date-fns/setHours";
import { addQuarters } from "date-fns/addQuarters";
import { ptBR } from "date-fns/locale/pt-BR";
import { registerLocale } from "../src/date_utils";
import { addYears } from "date-fns";
import { addYears, setSeconds } from "date-fns";

registerLocale("pt-BR", ptBR);

Expand Down Expand Up @@ -888,9 +888,12 @@ describe("date_utils", () => {
describe("isTimeInDisabledRange", () => {
it("should tell if time is in disabled range", () => {
const date = newDate("2016-03-15");
const time = setHours(setMinutes(date, 30), 1);
const minTime = setHours(setMinutes(date, 30), 0);
const maxTime = setHours(setMinutes(date, 30), 5);
let time = setHours(date, 1);
time = setMinutes(time, 30);
time = setSeconds(time, 30); // 2016-03-15 01:30:30

const minTime = setHours(setMinutes(date, 30), 0); //2016-03-15 00:30:00
const maxTime = setHours(setMinutes(setSeconds(date, 35), 30), 1); //2016-03-15 01:30:35
expect(isTimeInDisabledRange(time, { minTime, maxTime })).toBe(false);
});

Expand Down
25 changes: 25 additions & 0 deletions test/include_times_test.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,4 +54,29 @@ describe("TimeComponent", () => {
});
expect(enabledTimeItemsHasNoAriaDisabled).toBe(true);
});

it("should factor in seconds", () => {
const includeHoursMinutesSeconds = [
utils.addHours(utils.addSeconds(today, 30), 1), //01:00:30
utils.addSeconds(today, 30), //00:00:30
];
const { container: timeComponent } = render(
<TimeComponent
format="HH:mm:ss"
includeTimes={includeHoursMinutesSeconds}
/>,
);

const disabledTimeItems = timeComponent.querySelectorAll(
".react-datepicker__time-list-item--disabled",
);

// 01:00:00 and 00:00:00 should be correctly disabled because they are not included
expect(
Array.from(disabledTimeItems).map((node) => node.textContent),
).toContain("01:00:00");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this check to see if the seconds are visible? Unless i'm misunderstanding the test.

🔸 Improve Test Coverage (Important)

Image of Curtis Larson Curtis Larson

Copy link
Contributor Author

@yykcool yykcool Apr 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

expect(
Array.from(disabledTimeItems).map((node) => node.textContent),
).toContain("00:00:00");
});
});
Loading