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

Fix DatePicker Time Input #2845

Merged
merged 7 commits into from
May 24, 2024
Merged

Fix DatePicker Time Input #2845

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
dbc8658
fix(date-picker): set `isCalendarHeaderExpanded` to `false` when Date…
chirokas Apr 22, 2024
e7cf05d
fix(date-picker): calendar header controlled state on DatePicker
chirokas Apr 24, 2024
2e8dafe
chore(date-picker): update test
chirokas Apr 29, 2024
c6a40b5
chore(date-picker): remove unnecessary `async` in test
chirokas May 9, 2024
c67c283
Merge branch 'canary' into pr/2845
wingkwong May 16, 2024
e53dc7f
Merge branch 'canary' into fix/date-picker
jrgarciadev May 24, 2024
dff5c7c
Update packages/components/date-picker/__tests__/date-picker.test.tsx
jrgarciadev May 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/pretty-crews-build.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@nextui-org/date-picker": patch
---

Fix calendar header controlled state on DatePicker.
wingkwong marked this conversation as resolved.
Show resolved Hide resolved
81 changes: 81 additions & 0 deletions packages/components/date-picker/__tests__/date-picker.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -458,4 +458,85 @@ describe("DatePicker", () => {
expect(getTextValue(combobox)).toBe("2/4/2019"); // uncontrolled
});
});

describe("Month and Year Picker", () => {
const onHeaderExpandedChangeSpy = jest.fn();

afterEach(() => {
onHeaderExpandedChangeSpy.mockClear();
});

it("should show the month and year picker (uncontrolled)", async () => {
wingkwong marked this conversation as resolved.
Show resolved Hide resolved
const {getByRole} = render(
<DatePicker
showMonthAndYearPickers
calendarProps={{
onHeaderExpandedChange: onHeaderExpandedChangeSpy,
}}
defaultValue={new CalendarDate(2024, 4, 26)}
label="Date"
/>,
);

const button = getByRole("button");

triggerPress(button);

const dialog = getByRole("dialog");

expect(dialog).toBeVisible();

const header = document.querySelector<HTMLButtonElement>(`button[data-slot="header"]`)!;

expect(onHeaderExpandedChangeSpy).not.toHaveBeenCalled();

triggerPress(header);

const month = getByRole("button", {name: "April"});
const year = getByRole("button", {name: "2024"});

expect(month).toHaveAttribute("data-value", "4");
expect(year).toHaveAttribute("data-value", "2024");
expect(onHeaderExpandedChangeSpy).toHaveBeenCalledTimes(1);
expect(onHeaderExpandedChangeSpy).toHaveBeenCalledWith(true);

triggerPress(button);

expect(dialog).not.toBeInTheDocument();
expect(onHeaderExpandedChangeSpy).toHaveBeenCalledTimes(2);
expect(onHeaderExpandedChangeSpy).toHaveBeenCalledWith(false);
});

it("should show the month and year picker (controlled)", async () => {
const {getByRole} = render(
<DatePicker
showMonthAndYearPickers
calendarProps={{
isHeaderExpanded: true,
onHeaderExpandedChange: onHeaderExpandedChangeSpy,
}}
defaultValue={new CalendarDate(2024, 4, 26)}
label="Date"
/>,
);

const button = getByRole("button");

triggerPress(button);

const dialog = getByRole("dialog");
const month = getByRole("button", {name: "April"});
const year = getByRole("button", {name: "2024"});

expect(dialog).toBeVisible();
expect(month).toHaveAttribute("data-value", "4");
expect(year).toHaveAttribute("data-value", "2024");
expect(onHeaderExpandedChangeSpy).not.toHaveBeenCalled();

triggerPress(button);

expect(dialog).not.toBeInTheDocument();
expect(onHeaderExpandedChangeSpy).not.toHaveBeenCalled();
});
});
});
33 changes: 29 additions & 4 deletions packages/components/date-picker/src/use-date-picker-base.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,13 @@ import type {ReactNode} from "react";
import type {ValueBase} from "@react-types/shared";

import {dateInput, DatePickerVariantProps} from "@nextui-org/theme";
import {useState} from "react";
import {useCallback} from "react";
import {HTMLNextUIProps, mapPropsVariants} from "@nextui-org/system";
import {mergeProps} from "@react-aria/utils";
import {useDOMRef} from "@nextui-org/react-utils";
import {dataAttr} from "@nextui-org/shared-utils";
import {useLocalizedStringFormatter} from "@react-aria/i18n";
import {useControlledState} from "@react-stately/utils";

import intlMessages from "../intl/messages";

Expand Down Expand Up @@ -114,8 +115,6 @@ export type UseDatePickerBaseProps<T extends DateValue> = Props<T> &
export function useDatePickerBase<T extends DateValue>(originalProps: UseDatePickerBaseProps<T>) {
const [props, variantProps] = mapPropsVariants(originalProps, dateInput.variantKeys);

const [isCalendarHeaderExpanded, setIsCalendarHeaderExpanded] = useState(false);

const {
as,
ref,
Expand Down Expand Up @@ -144,6 +143,24 @@ export function useDatePickerBase<T extends DateValue>(originalProps: UseDatePic
createCalendar,
} = props;

const {
isHeaderExpanded,
isHeaderDefaultExpanded,
onHeaderExpandedChange,
...restUserCalendarProps
} = userCalendarProps;

const handleHeaderExpandedChange = useCallback(
(isExpanded: boolean | undefined) => {
onHeaderExpandedChange?.(isExpanded || false);
},
[onHeaderExpandedChange],
);

const [isCalendarHeaderExpanded, setIsCalendarHeaderExpanded] = useControlledState<
boolean | undefined
>(isHeaderExpanded, isHeaderDefaultExpanded ?? false, handleHeaderExpandedChange);

const domRef = useDOMRef(ref);
const disableAnimation = originalProps.disableAnimation ?? false;

Expand Down Expand Up @@ -191,6 +208,7 @@ export function useDatePickerBase<T extends DateValue>(originalProps: UseDatePic
pageBehavior,
isDateUnavailable,
showMonthAndYearPickers,
isHeaderExpanded: isCalendarHeaderExpanded,
onHeaderExpandedChange: setIsCalendarHeaderExpanded,
color:
(originalProps.variant === "bordered" || originalProps.variant === "underlined") &&
Expand All @@ -201,7 +219,7 @@ export function useDatePickerBase<T extends DateValue>(originalProps: UseDatePic
: originalProps.color,
disableAnimation,
},
userCalendarProps,
restUserCalendarProps,
),
};

Expand Down Expand Up @@ -250,6 +268,12 @@ export function useDatePickerBase<T extends DateValue>(originalProps: UseDatePic
"data-slot": "selector-icon",
};

const onClose = () => {
if (isHeaderExpanded === undefined) {
setIsCalendarHeaderExpanded(false);
}
};
Comment on lines +270 to +274
Copy link
Contributor

Choose a reason for hiding this comment

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

The onClose function correctly resets the isCalendarHeaderExpanded state when isHeaderExpanded is undefined, which aligns with the PR's objective. However, consider adding a comment explaining why this check is necessary for future maintainability.

+  // Reset the header expansion state if it's not explicitly controlled
   if (isHeaderExpanded === undefined) {
     setIsCalendarHeaderExpanded(false);
   }
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
const onClose = () => {
if (isHeaderExpanded === undefined) {
setIsCalendarHeaderExpanded(false);
}
};
const onClose = () => {
// Reset the header expansion state if it's not explicitly controlled
if (isHeaderExpanded === undefined) {
setIsCalendarHeaderExpanded(false);
}
};


return {
domRef,
endContent,
Expand All @@ -273,6 +297,7 @@ export function useDatePickerBase<T extends DateValue>(originalProps: UseDatePic
userTimeInputProps,
selectorButtonProps,
selectorIconProps,
onClose,
};
}

Expand Down
6 changes: 6 additions & 0 deletions packages/components/date-picker/src/use-date-picker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,17 @@ export function useDatePicker<T extends DateValue>({
userTimeInputProps,
selectorButtonProps,
selectorIconProps,
onClose,
} = useDatePickerBase(originalProps);

let state: DatePickerState = useDatePickerState({
...originalProps,
shouldCloseOnSelect: () => !state.hasTime,
onOpenChange: (isOpen) => {
if (!isOpen) {
onClose();
}
},
});

const baseStyles = clsx(classNames?.base, className);
Expand Down