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

Fix controlled date picker set to null #4459

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
15 changes: 10 additions & 5 deletions packages/date-adapters/src/date-fns/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,20 +152,25 @@ export class AdapterDateFns implements SaltDateAdapter<Date, Locale> {

/**
* Formats a Date object using the specified format string.
* Returns empty string when null or undefined date is given.
*
* @param date - The Date object to format.
* @param format - The format string to use.
* @param locale - The locale to use for formatting.
* @returns The formatted date string.
*/
public format(
date: Date,
date: Date | null | undefined,
format: RecommendedFormats = "dd MMM yyyy",
locale?: Locale,
): string {
const dateFnsFormat = this.mapToDateFnsFormat(format);
return formatDateFns(date, dateFnsFormat, {
locale: locale ?? this.locale,
});
if (this.isValid(date)) {
const dateFnsFormat = this.mapToDateFnsFormat(format);
return formatDateFns(date, dateFnsFormat, {
locale: locale ?? this.locale,
});
}
return "";
}

/**
Expand Down
9 changes: 7 additions & 2 deletions packages/date-adapters/src/dayjs/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,17 +158,22 @@ export class AdapterDayjs implements SaltDateAdapter<Dayjs, string> {

/**
* Formats a Day.js date object using the specified format string.
* Returns empty string when null or undefined date is given.
*
* @param date - The Day.js date object to format.
* @param format - The format string to use.
* @param locale - The locale to use for formatting.
* @returns The formatted date string.
*/
public format(
date: Dayjs,
date: Dayjs | null | undefined,
format: RecommendedFormats = "DD MMM YYYY",
locale?: string,
): string {
return date.locale(locale ?? this.locale).format(format);
if (this.isValid(date)) {
return date.locale(locale ?? this.locale).format(format);
}
return "";
}

/**
Expand Down
11 changes: 8 additions & 3 deletions packages/date-adapters/src/luxon/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,18 +173,23 @@ export class AdapterLuxon implements SaltDateAdapter<DateTime, string> {

/**
* Formats a Luxon DateTime object using the specified format string.
* Returns empty string when null or undefined date is given.
*
* @param date - The Luxon DateTime object to format.
* @param format - The format string to use.
* @param locale - The locale to use for formatting.
* @returns The formatted date string.
*/
public format(
date: DateTime,
date: DateTime | null | undefined,
format: RecommendedFormats = "dd MMM yyyy",
locale?: string,
): string {
const luxonFormat = this.mapToLuxonFormat(format);
return date.setLocale(locale ?? this.locale).toFormat(luxonFormat);
if (this.isValid(date)) {
const luxonFormat = this.mapToLuxonFormat(format);
return date.setLocale(locale ?? this.locale).toFormat(luxonFormat);
}
return "";
}

/**
Expand Down
15 changes: 10 additions & 5 deletions packages/date-adapters/src/moment/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,20 +149,25 @@ export class AdapterMoment implements SaltDateAdapter<Moment, string> {

/**
* Formats a Moment.js date object using the specified format string.
* Returns empty string when null or undefined date is given.
*
* @param date - The Moment.js date object to format.
* @param format - The format string to use.
* @param locale - The locale to use for formatting.
* @returns The formatted date string.
*/
public format(
date: Moment,
date: Moment | null | undefined,
format: RecommendedFormats = "DD MMM YYYY",
locale?: string,
): string {
return date
.clone()
.locale(locale ?? this.locale)
.format(format);
if (this.isValid(date)) {
return date
.clone()
.locale(locale ?? this.locale)
.format(format);
}
return "";
}

/**
Expand Down
6 changes: 5 additions & 1 deletion packages/date-adapters/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,11 @@ export interface SaltDateAdapter<
* @param locale - The locale to use for formatting.
* @returns The formatted date string.
*/
format(date: TDate, format?: RecommendedFormats, locale?: TLocale): string;
format(
date: TDate | null | undefined,
format?: RecommendedFormats,
locale?: TLocale,
): string;

/**
* Compares two date objects.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ const adapters = [adapterDateFns, adapterDayjs, adapterLuxon, adapterMoment];
function assertDateChange(
spy: any,
expectedValue: string,
expectedDate: DateFrameworkType | undefined,
expectedDate: DateFrameworkType | null,
adapter: SaltDateAdapter<DateFrameworkType>,
) {
const lastCallArgs = spy.args[spy.callCount - 1];
Expand Down Expand Up @@ -159,7 +159,7 @@ describe("GIVEN a DateInputSingle", () => {
cy.realPress("Tab");
cy.get("@dateChangeSpy").should("have.callCount", 3);
cy.get("@dateChangeSpy").then((spy) =>
assertDateChange(spy, "", undefined, adapter),
assertDateChange(spy, "", null, adapter),
);
cy.get("@dateValueChangeSpy").should(
"have.been.calledWith",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,9 @@ describe("GIVEN a DatePicker where selectionVariant is single", () => {
"have.value",
updatedFormattedDateValue,
);

cy.findByRole("button", { name: "Set null" }).realClick();
cy.findByRole("textbox").should("have.value", "");
});

it("SHOULD preserve original time during date selection", () => {
Expand Down
4 changes: 1 addition & 3 deletions packages/lab/src/date-input/DateInputRange.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ export const DateInputRange = forwardRef<
endInputRef: endInputRefProp,
locale,
parse: parseProp,
placeholder = "dd mmm yyyy",
placeholder = format,
Copy link
Contributor

Choose a reason for hiding this comment

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

nice

Copy link
Contributor

Choose a reason for hiding this comment

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

ah.. the case has changed :(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Casing actually matters, in certain library, lower case and upper case means different things.

readOnly: readOnlyProp,
validationStatus: validationStatusProp,
variant = "primary",
Expand Down Expand Up @@ -542,7 +542,6 @@ export const DateInputRange = forwardRef<
ref={handleStartInputRef}
tabIndex={isDisabled ? -1 : 0}
placeholder={placeholder}
size={placeholder.length}
value={
isReadOnly && !dateValue?.startDate
? emptyReadOnlyMarker
Expand Down Expand Up @@ -575,7 +574,6 @@ export const DateInputRange = forwardRef<
ref={handleEndInputRef}
tabIndex={isDisabled ? -1 : 0}
placeholder={placeholder}
size={placeholder.length}
value={
isReadOnly && !dateValue?.endDate
? emptyReadOnlyMarker
Expand Down
25 changes: 13 additions & 12 deletions packages/lab/src/date-input/DateInputSingle.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ export const DateInputSingle = forwardRef<
inputRef: inputRefProp = null,
locale,
parse: parseProp,
placeholder = "dd mmm yyyy",
placeholder = format,
readOnly: readOnlyProp,
startAdornment,
validationStatus: validationStatusProp,
Expand Down Expand Up @@ -215,16 +215,18 @@ export const DateInputSingle = forwardRef<
? dateAdapter.getTime(date)
: null;

// Update date string value when selected date changes
// biome-ignore lint/correctness/useExhaustiveDependencies: Update date string value ONLY when selected date changes, not when date string itself change
useEffect(() => {
if (date && dateAdapter.isValid(date)) {
const formattedValue = dateAdapter.format(date, format, locale);
const hasValueChanged = formattedValue !== dateValue;
if (hasValueChanged) {
lastAppliedValue.current = formattedValue;
setDateValue(formattedValue);
onDateValueChange?.(null, formattedValue);
}
const formattedValue = dateAdapter.format(date, format, locale);
const hasValueChanged = formattedValue !== dateValue;
if (
// don't want to reset "error" input values
(dateAdapter.isValid(date) || date === null) &&
hasValueChanged
) {
lastAppliedValue.current = formattedValue;
setDateValue(formattedValue);
onDateValueChange?.(null, formattedValue);
}
}, [date, dateAdapter.format, format, locale]);

Expand Down Expand Up @@ -265,7 +267,7 @@ export const DateInputSingle = forwardRef<
const parse = parseProp ?? dateAdapter.parse.bind(dateAdapter);
const parseResult = dateValue?.length
? parse(dateValue, format, locale)
: { date: undefined };
: { date: null };
let { date: parsedDate, ...parseDetails } = parseResult;
let formattedValue = "";
if (dateAdapter.isValid(parsedDate)) {
Expand Down Expand Up @@ -358,7 +360,6 @@ export const DateInputSingle = forwardRef<
ref={handleInputRef}
tabIndex={isDisabled ? -1 : 0}
placeholder={placeholder}
size={placeholder.length}
value={isReadOnly && !dateValue ? emptyReadOnlyMarker : dateValue}
{...restDateInputProps}
onBlur={handleBlur}
Expand Down
4 changes: 2 additions & 2 deletions packages/lab/src/date-picker/DatePickerSingleInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -180,9 +180,9 @@ export const DatePickerSingleInput = forwardRef<

return (
<DateInputSingle
value={value || ""}
value={value}
className={clsx(withBaseName(), className)}
date={selectedDate || null}
date={selectedDate}
readOnly={readOnly}
ref={ref}
onDateChange={handleDateChange}
Expand Down
58 changes: 57 additions & 1 deletion packages/lab/stories/date-input/date-input.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { FlexLayout, StackLayout } from "@salt-ds/core";
import type { DateFrameworkType } from "@salt-ds/date-adapters";
import {
DateInputRange,
Expand All @@ -7,11 +8,12 @@ import {
type DateInputSingleDetails,
type DateInputSingleProps,
type DateRangeSelection,
type SingleDateSelection,
useLocalization,
} from "@salt-ds/lab";
import type { Meta, StoryFn } from "@storybook/react";
import { fn } from "@storybook/test";
import type { SyntheticEvent } from "react";
import { type SyntheticEvent, useState } from "react";

export default {
title: "Lab/Date Input",
Expand Down Expand Up @@ -124,3 +126,57 @@ EmptyReadOnlyMarker.args = {
readOnly: true,
onDateValueChange: fn(),
};

export const ControlledSingle: StoryFn<
DateInputSingleProps<DateFrameworkType>
> = (args) => {
const { dateAdapter } = useLocalization();
const [selectedDate, setSelectedDate] = useState<
SingleDateSelection<DateFrameworkType> | null | undefined
>(args?.date ?? null);

function handleDateChange<TDate extends DateFrameworkType>(
event: SyntheticEvent,
date: TDate | null,
details: DateInputSingleDetails,
) {
console.log(
`Selected date: ${dateAdapter.isValid(date) ? dateAdapter.format(date, "DD MMM YYYY") : date}`,
);
const { value, errors } = details;
if (errors?.length && value) {
console.log(
`Error(s): ${errors
.map(({ type, message }) => `type=${type} message=${message}`)
.join(",")}`,
);
if (value) {
console.log(`Original Value: ${value}`);
}
}
setSelectedDate(date);
args?.onDateChange?.(event, date, details);
}

return (
<StackLayout style={{ width: "400px" }}>
<DateInputSingle
date={selectedDate}
onDateChange={handleDateChange}
{...args}
/>

<FlexLayout>
<button onClick={() => setSelectedDate(null)}>Set null</button>
<button onClick={() => setSelectedDate(new Date())}>Set today</button>
<button
onClick={() =>
setSelectedDate(dateAdapter.add(new Date(), { days: 1 }))
}
>
Set tomorrow
</button>
</FlexLayout>
</StackLayout>
);
};
38 changes: 25 additions & 13 deletions packages/lab/stories/date-picker/date-picker.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -204,19 +204,30 @@ export const SingleControlled: StoryFn<
);

return (
<DatePicker
selectionVariant={"single"}
{...args}
onSelectionChange={handleSelectionChange}
selectedDate={selectedDate}
>
<DatePickerTrigger>
<DatePickerSingleInput />
</DatePickerTrigger>
<DatePickerOverlay>
<DatePickerSinglePanel />
</DatePickerOverlay>
</DatePicker>
<>
<DatePicker
selectionVariant={"single"}
{...args}
onSelectionChange={handleSelectionChange}
selectedDate={selectedDate}
>
<DatePickerTrigger>
<DatePickerSingleInput />
</DatePickerTrigger>
<DatePickerOverlay>
<DatePickerSinglePanel />
</DatePickerOverlay>
</DatePicker>
<button onClick={() => setSelectedDate(null)}>Set null</button>
<button onClick={() => setSelectedDate(new Date())}>Set today</button>
<button
onClick={() =>
setSelectedDate(dateAdapter.add(new Date(), { days: 1 }))
}
>
Set tomorrow
</button>
</>
);
};

Expand Down Expand Up @@ -519,6 +530,7 @@ export const SingleWithInitialError: StoryFn<
<FormLabel>Select a date</FormLabel>
<DatePicker
selectionVariant="single"
defaultSelectedDate={dateAdapter.parse("bad date", "DD MMM YYYY").date}
{...args}
onSelectionChange={handleSelectionChange}
onOpen={setOpen}
Expand Down
Loading