Replaced moment with dayjs fixed issue #34 #47
Conversation
|
@rohit-raje-786 hey actually react-datetime has a dependency on moment so when you are replacing it with dayjs isn't it causing some issues? |
|
I guess currently you haven't removed moment from your dependency and that's why it's working fine but it won't work if we remove moment from our dependency |
|
Hey @ruddi10 have a look at it now I have replaced rect-datetime with @material-ui/pickers.And I have removed moment.js and react-datetime |
|
@aquibbaig I think in the older version of the dashboard we were using material-ui/pickers and there were some reasons that the maintainers wanted to go with react-datetime so what's your take on using something other than this package ? If so do you have some good alternative in your mind? |
| @@ -0,0 +1,11 @@ | |||
| # This configuration file was automatically generated by Gitpod. | |||
There was a problem hiding this comment.
shouldn't be a part of this PR
There was a problem hiding this comment.
Hey, thanks for this PR, I am posting some initial feedback on your work
- Start Time -> Picker should be in a center-aligned vertically
- For the consistent look and feel, use the same font family and same font colors in pickers.
- Remove the background on click of the picker (or come up w a better design)
| const [startTime, setStartTime] = useState(dayjs(selectedStartTimestamp)); | ||
| const [endTime, setEndTime] = useState(dayjs(selectedEndTimestamp)); |
There was a problem hiding this comment.
I'd say keep the start and end times in the redux store (and persist them too) so that it is maintained across refreshes (mostly people do not like dates getting reset)
| @@ -44,13 +51,13 @@ const TimeQuerier: React.FC = () => { | |||
| if (valueAsString !== "" && valueAsNumber >= minStepValue) | |||
| setStepTime(valueAsNumber); | |||
| }; | |||
| const valid = (current: moment.Moment) => { | |||
| return current.isBefore(moment()); | |||
| const valid = (current: dayjs.Dayjs) => { | |||
| if (typeof date !== "string") { | ||
| setStartTime(date); | ||
| setError(undefined); | ||
| } | ||
| }; | ||
| const handleEndChange = (date: moment.Moment | string) => { | ||
| const handleEndChange = (date: dayjs.Dayjs | string) => { | ||
| if (typeof date !== "string") { | ||
| setEndTime(date); | ||
| setError(undefined); |
There was a problem hiding this comment.
error is either null or Error
Replaced moment with dayjs to fixed issue #34