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.

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

[Timeline]: Earliest date defaulted to current date when found date was later than current day #3458

Merged
merged 5 commits into from
Dec 16, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
34 changes: 25 additions & 9 deletions @navikt/core/react/src/timeline/hooks/useTimelineRows.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,17 +134,33 @@ export const useTimelineRows = (
[rows, startDate, endDate, direction],
);

const earliestDate = (earliest: Date, period: Period) =>
period.start < earliest ? period.start : earliest;
export const useEarliestDate = ({
startDate,
rows,
}: {
startDate?: Date;
rows: Omit<Period, "id" | "endInclusive">[][];
KenAJoh marked this conversation as resolved.
Show resolved Hide resolved
}) =>
useMemo(() => {
if (startDate) {
return startDate;
}

const earliestFomDate = (rader: Period[][]) =>
rader.flat().reduce(earliestDate, new Date());
const startDates = rows
.flat()
.filter((period) => period.start)
.map((period) => period.start);

export const useEarliestDate = ({ startDate, rows }: any) =>
useMemo(
() => (startDate ? startDate : earliestFomDate(rows)),
[startDate, rows],
);
if (startDates.length === 0) {
return new Date();
}

const earliestDate = startDates.reduce((earliest, current) =>
current < earliest ? current : earliest,
);

return earliestDate;
}, [startDate, rows]);
Copy link
Contributor

Choose a reason for hiding this comment

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

The useMemo in useEarliestDate seems to be running on every render. I think it's because of rows. We can fix it later since it wasn't added in this PR, but I think we should reconsider all use of useMemo in this component.

Copy link
Contributor

@JulianNymark JulianNymark Dec 16, 2024

Choose a reason for hiding this comment

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

Is this potentially something that we can just delete entirely when we have upgraded to react 19 and decided to use the compiler... 🤔 (and the compiler should be able to optimise re-renders?)

Actually, I don't think it will make sense for a design system to "use the react compiler" (we wouldn't be able to use it on behalf of our consumers 😅 ), so nvm this.


const latestDate = (latest: Date, period: Period) =>
period.end > latest ? period.end : latest;
Expand Down
133 changes: 133 additions & 0 deletions @navikt/core/react/src/timeline/tests/useTimelineRows.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
import { renderHook } from "@testing-library/react";
import { addDays, isSameDay } from "date-fns";
import { describe, expect, test } from "vitest";
import {
useEarliestDate,
useLatestDate,
useTimelineRows,
} from "../hooks/useTimelineRows";

describe("useEarliestDate", () => {
test("returns the provided startDate if it exists", () => {
const startDate = new Date(2023, 0, 1);
const { result } = renderHook(() =>
useEarliestDate({ startDate, rows: [] }),
);
expect(result.current).toEqual(startDate);
});

test("returns the earliest date from the rows if startDate is not provided", () => {
const rows = [
[{ start: new Date(2023, 0, 1) }],
[{ start: new Date(2022, 0, 1) }],
];

/* @ts-expect-error asdas */
const { result } = renderHook(() => useEarliestDate({ rows }));
expect(result.current).toEqual(new Date(2022, 0, 1));
});

test("returns the earliest date from the rows if startDate is not provided and date is later than todays date", () => {
const earliestDate = addDays(new Date(), 400);
const rows = [
[{ start: earliestDate }],
[{ start: addDays(earliestDate, 40) }],
];

/* @ts-expect-error asdas */
const { result } = renderHook(() => useEarliestDate({ rows }));
expect(result.current).toEqual(earliestDate);
});

test("returns the current date if no startDate and rows are empty", () => {
const { result } = renderHook(() => useEarliestDate({ rows: [] }));
expect(isSameDay(result.current, new Date())).toBeTruthy();
});
});

describe("useLatestDate", () => {
test("returns the provided endDate if it exists", () => {
const endDate = new Date(2023, 0, 1);
const { result } = renderHook(() => useLatestDate({ endDate, rows: [] }));
expect(result.current).toEqual(endDate);
});

test("returns the latest date from the rows plus one day if endDate is not provided", () => {
const rows = [
[{ start: new Date(2023, 0, 1), end: new Date(2023, 0, 10) }],
[{ start: new Date(2022, 0, 1), end: new Date(2022, 0, 5) }],
];
const { result } = renderHook(() => useLatestDate({ rows }));
expect(result.current).toEqual(addDays(new Date(2023, 0, 10), 1));
});

test("returns the current date plus one day if no endDate and rows are empty", () => {
const { result } = renderHook(() => useLatestDate({ rows: [] }));
expect(result.current).toEqual(addDays(new Date(0), 1));
});
});

describe("useTimelineRows", () => {
const rows = [
{
label: "Row 1",
periods: [
{
start: new Date(2023, 0, 1),
end: new Date(2023, 0, 10),
status: "active",
},
{
start: new Date(2023, 0, 15),
end: new Date(2023, 0, 20),
status: "inactive",
},
],
},
{
label: "Row 2",
periods: [
{
start: new Date(2022, 0, 1),
end: new Date(2022, 0, 5),
status: "active",
},
],
},
];

test("returns the correct timeline rows", () => {
const startDate = new Date(2022, 0, 1);
const endDate = new Date(2023, 0, 31);
const direction = "left";
const { result } = renderHook(() =>
useTimelineRows(rows, startDate, endDate, direction),
);

expect(result.current).toHaveLength(2);
expect(result.current[0].periods).toHaveLength(2);
expect(result.current[1].periods).toHaveLength(1);
});

test("handles empty rows", () => {
const startDate = new Date(2022, 0, 1);
const endDate = new Date(2023, 0, 31);
const direction = "left";
const { result } = renderHook(() =>
useTimelineRows([], startDate, endDate, direction),
);

expect(result.current).toHaveLength(0);
});

test("handles different directions", () => {
const startDate = new Date(2022, 0, 1);
const endDate = new Date(2023, 0, 31);
const direction = "right";
const { result } = renderHook(() =>
useTimelineRows(rows, startDate, endDate, direction),
);

expect(result.current[0].periods[0].start).toEqual(new Date(2023, 0, 15));
});
});
Loading