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

DatePicker, DateRangePickerをdayjsでやりとりするようにwrap #1080

Merged
merged 19 commits into from
Nov 11, 2022
3 changes: 2 additions & 1 deletion .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,6 @@
},
"eslint.options": {
"extensions": [".js", ".jsx", ".md", ".ts", ".tsx"]
}
},
"editor.defaultFormatter": "dbaeumer.vscode-eslint"
Copy link
Contributor

Choose a reason for hiding this comment

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

この差分はミスってるだけですかね?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ミスですね

}
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
},
"dependencies": {
"@popperjs/core": "^2.4.0",
"dayjs": "^1.11.6",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"dayjs": "^1.11.6",
"dayjs": "1.11.6",

"moment": "^2.29.3",
"react-dates": "^21.8.0",
"react-popper": "^2.3.0",
Expand Down
23 changes: 12 additions & 11 deletions src/components/DatePicker/DatePicker.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { ArgsTable, Description, Stories, Title } from "@storybook/addon-docs";
import { Story } from "@storybook/react/types-6-0";
import moment from "moment";
import dayjs from "dayjs";
import React from "react";
import DatePicker from "./DatePicker";
import "dayjs/locale/ja";

export default {
title: "Components/Inputs/DatePicker",
Expand Down Expand Up @@ -41,9 +42,9 @@ export default {
};

export const Basic: Story = () => {
moment.locale("en");
const [date, setDate] = React.useState(moment());
const handleChangeDate = (date: moment.Moment | null) => {
dayjs.locale("en");
const [date, setDate] = React.useState(dayjs());
const handleChangeDate = (date: dayjs.Dayjs | null) => {
if (date === null) {
return;
}
Expand All @@ -57,17 +58,15 @@ export const Basic: Story = () => {
};

export const Error: Story = () => {
return <DatePicker date={moment()} error={true} onDateChange={() => {}} />;
return <DatePicker date={dayjs()} error={true} onDateChange={() => {}} />;
};

export const Localize: Story = () => {
moment.locale("ja", {
weekdaysShort: ["日", "月", "火", "水", "木", "金", "土"],
});
const renderMonthText = (day: moment.Moment) => day.format("YYYY年M月");
dayjs.locale("ja");
const renderMonthText = (day: dayjs.Dayjs) => day.format("YYYY年M月");
const displayFormat = () => "YYYY/MM/DD";
const [date, setDate] = React.useState(moment());
const handleChangeDate = (date: moment.Moment | null) => {
const [date, setDate] = React.useState(dayjs());
const handleChangeDate = (date: dayjs.Dayjs | null) => {
if (date === null) {
return;
}
Expand All @@ -77,6 +76,8 @@ export const Localize: Story = () => {
<div style={{ height: "400px" }}>
<DatePicker
date={date}
locale={"ja"}
weekdaysShort={["日", "月", "火", "水", "木", "金", "土"]}
displayFormat={displayFormat}
renderMonthText={renderMonthText}
onDateChange={handleChangeDate}
Expand Down
52 changes: 46 additions & 6 deletions src/components/DatePicker/DatePicker.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import * as React from "react";
import * as Styled from "./styled";
import "react-dates/initialize";
import dayjs from "dayjs";
import moment from "moment";
import { dayjsToMoment, momentToDayjs } from "../../utils/time";
import {
RenderMonthProps,
SingleDatePicker,
Expand All @@ -16,31 +18,63 @@ function isOutsideRange() {
}

export type DatePickerProps = Partial<
Omit<SingleDatePickerShape, "date" | "onFocusChange">
Omit<
SingleDatePickerShape,
"date" | "onFocusChange" | "onDateChange" | "renderMonthText"
>
> &
// MEMO: Add RenderMonthProps to pass type check.
RenderMonthProps & {
date: moment.Moment | null;
onDateChange: (date: moment.Moment | null) => void;
Omit<RenderMonthProps, "renderMonthText"> & {
date: dayjs.Dayjs | null;
onDateChange: (date: dayjs.Dayjs | null) => void;
renderMonthText?: ((month: dayjs.Dayjs) => React.ReactNode) | null;
locale?: string;
weekdaysShort?: string[];
error?: boolean;
};

const DatePicker = React.forwardRef<HTMLDivElement, DatePickerProps>(
(inProps, ref) => {
const props = useLocaleProps({ props: inProps, name: "DatePicker" });
const { date, error = false, ...rest } = props;
const {
date,
error = false,
onDateChange,
renderMonthText: renderMonthTextProps,
Copy link
Contributor

Choose a reason for hiding this comment

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

renderMonthTextProps だと複数を指しているように見えてしまうので単数にするのが良さそうなのと、XXXProps だと名前が関数っぽくないので何か違う名前がつけられると良さそう。
あとは、これ今の構成だとそもそも別名つけなくても良さそうに見えるのですがどうでしょう。

renderMonthElement,
locale = "en",
weekdaysShort,
...rest
} = props;

const [focused, setFocused] = React.useState<boolean>(false);
const onFocusChange = ({ focused }: { focused: boolean }) => {
setFocused(focused);
};
const handleDateChange = (date: moment.Moment | null) => {
const dayjsize = momentToDayjs(date);
onDateChange(dayjsize);
};
const handleRenderMonthText = (month: moment.Moment) => {
const dayjsize = momentToDayjs(month);
if (!renderMonthTextProps || !dayjsize) return;
return renderMonthTextProps(dayjsize);
};
Copy link
Contributor

Choose a reason for hiding this comment

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

renderMonthTextProps の有無に関わらず dayjsize が作られてしまうので renderMonthTextProps の存在確認と dayjsize の null 確認は別の条件分岐で書いた方がよさそうです。

Copy link
Contributor

Choose a reason for hiding this comment

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

nits
linter 案件ではありますが(自分の惰性が働き strict-boolean-expressions を有効にしていないのが原因です...。)、論理否定を使った falsy 判定はあまり行儀が良くない気がします。

Copy link
Contributor

Choose a reason for hiding this comment

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

そこまで気にすることでもないので falsy に関しては任せます


Copy link
Contributor Author

Choose a reason for hiding this comment

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

SingleDatePickerはmomentなので、外側でdayjsでlocaleを設定しても影響しない。
外からdayjsのlocale設定をコンポーネントへ持ち込むことはできなかったので locale と(必要なら) weekdayShort を渡すやり方にしてみた

if (weekdaysShort) {
moment.locale(locale, {
weekdaysShort: weekdaysShort,
});
} else {
moment.locale(locale);
}

return (
<Styled.Container ref={ref} error={error}>
<SingleDatePicker
id="datePicker"
focused={focused}
date={date}
date={dayjsToMoment(date)}
isOutsideRange={isOutsideRange}
numberOfMonths={1}
enableOutsideDays={true}
Expand All @@ -61,7 +95,13 @@ const DatePicker = React.forwardRef<HTMLDivElement, DatePickerProps>(
</Spacer>
</Styled.NavNext>
}
// eslint-disable-next-line react/jsx-handler-names
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// eslint-disable-next-line react/jsx-handler-names

renderMonthText={
renderMonthTextProps ? handleRenderMonthText : renderMonthTextProps
}
renderMonthElement={renderMonthElement as never}
onFocusChange={onFocusChange}
onDateChange={handleDateChange}
{...rest}
/>
</Styled.Container>
Expand Down
16 changes: 16 additions & 0 deletions src/utils/time.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import dayjs from "dayjs";
import moment from "moment";

// moment.jsで動いているreact-datesと互換性を持たせるためのメソッド
// DatePickerとDateRangePickerのリニューアルが完了したら除去する
export function dayjsToMoment(date: dayjs.Dayjs | null): moment.Moment | null {
if (!date) return null;
const dateString = date.format();
return moment(dateString);
}

export function momentToDayjs(date: moment.Moment | null): dayjs.Dayjs | null {
if (!date) return null;
const dateString = date.format();
return dayjs(dateString);
}
5 changes: 5 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -6446,6 +6446,11 @@ dayjs@^1.10.4:
resolved "https://registry.yarnpkg.com/dayjs/-/dayjs-1.11.3.tgz#4754eb694a624057b9ad2224b67b15d552589258"
integrity sha512-xxwlswWOlGhzgQ4TKzASQkUhqERI3egRNqgV4ScR8wlANA/A9tZ7miXa44vTTKEq5l7vWoL5G57bG3zA+Kow0A==

dayjs@^1.11.6:
version "1.11.6"
resolved "https://registry.yarnpkg.com/dayjs/-/dayjs-1.11.6.tgz#2e79a226314ec3ec904e3ee1dd5a4f5e5b1c7afb"
integrity sha512-zZbY5giJAinCG+7AGaw0wIhNZ6J8AhWuSXKvuc1KAyMiRsvGQWqh4L+MomvhdAYjN+lqvVCMq1I41e3YHvXkyQ==

[email protected], debug@^2.2.0, debug@^2.3.3, debug@^2.6.0, debug@^2.6.9:
version "2.6.9"
resolved "https://registry.yarnpkg.com/debug/-/debug-2.6.9.tgz#5d128515df134ff327e90a4c93f4e077a536341f"
Expand Down