Chore/create datepicker component #131
Conversation
| @@ -119,6 +119,8 @@ | |||
| "@fortawesome/react-fontawesome": "^0.2.0", | |||
| "@popperjs/core": "^2.11.6", | |||
| "classnames": "^2.3.2", | |||
| "date-fns": "^2.11.0", | |||
| "react-datepicker": "^2.14.0", | |||
There was a problem hiding this comment.
Question; what is the impact on library footprint after adding these new deps?
package.json
Outdated
| @@ -119,6 +119,8 @@ | |||
| "@fortawesome/react-fontawesome": "^0.2.0", | |||
| "@popperjs/core": "^2.11.6", | |||
| "classnames": "^2.3.2", | |||
| "date-fns": "^2.11.0", | |||
There was a problem hiding this comment.
Suggestion; can we avoid adding another date utility library given that the standard library is quite feature rich now, and we're already using moment elsewhere ?
| @@ -0,0 +1,4 @@ | |||
| A datepicker is a graphical user interface (GUI) element that allows users to easily select a date from a calendar. It is commonly used in web forms and applications to streamline the process of entering dates. A typical datepicker consists of an input field, where users can manually enter a date, and a graphical calendar interface that pops up when users interact with the input field. | |||
There was a problem hiding this comment.
Suggestion; can we follow the same template used by other components?
| onTimeChange?: (time: string, date: Date | null) => void; | ||
| onClose?: () => void; | ||
| popoverCs?: customStyles; | ||
| isBlock?: boolean; |
There was a problem hiding this comment.
Suggestion; isBlock isn't really clear as a name. Would suggest a more descriptive name and/or supporting comments to make it clear what the purpose is.
| </Typography> | ||
| <input | ||
| style={{width: "120px", marginLeft: "10px"}} | ||
| type="time" |
There was a problem hiding this comment.
Question; have we tested the experience of the time input on various platforms and devices?
| "data-cy-today": dataCyToday = "date-picker-today-button", | ||
| }: DatePickerProps) => { | ||
| const [selectedDate, setSelectedDate] = useState<Date | null>(defaultDate || null); | ||
| const [selectedTime, setSelectedTime] = useState(defaultTime); |
There was a problem hiding this comment.
Question; we seem to have duplicate selected time state. selectedTime here and then inputValue in the Time picker? Any chance we can avoid having both?
| }; | ||
|
|
||
| const handleToday = () => { | ||
| const todaysDate = new Date(); |
There was a problem hiding this comment.
Suggestion; having large mapping function like this as an inline callback is going to make it difficult to test, would suggest moving it outside the render function.
| }} | ||
| data-cy={dataCy} | ||
| > | ||
| <Box cs={{border: "1px solid #aeaeae"}}> |
There was a problem hiding this comment.
Suggestion; can we use theme props here instead of hard-coded colours?
| import DatePicker from "./DatePicker"; | ||
|
|
||
| describe("DatePicker", () => { | ||
| test("renders the DatePicker component", () => { |
There was a problem hiding this comment.
Suggestion; some more thorough tests here please
| import styled from "styled-components"; | ||
|
|
||
| const StyledDatePickerWrapper = styled.div` | ||
| .react-datepicker { |
There was a problem hiding this comment.
Suggestion; can we use theme props throughout this component?
added datepicker component
