[2228] Allow planning review period timeline #2277
Conversation
S78901
changed the title
|
Added console logs to illustrate the API receiving the date as a string and returning it as a serialized array that can't be utilized. @mjperry91 for your review: Input: API returns: |
…ing-review-timeline-calendar
…ing-review-timeline-calendar
|
|
||
| return () => clearTimeout(timeout); | ||
| } | ||
| return () => {}; |
There was a problem hiding this comment.
Is line 20 really needed?
There was a problem hiding this comment.
I think this is not needed, this was from the MUI example. Deleted.
|
|
||
| return ( | ||
| <div className="datePickerField"> | ||
| <LocalizationProvider dateAdapter={AdapterDayjs}> |
There was a problem hiding this comment.
Looks like Prettier didn't run on this file because this element isn't indented inside the div.
| if (data) { | ||
| dispatch({ type: ADD_REVIEW_PERIOD, payload: data }); | ||
| } else { | ||
| console.log(res?.error); |
There was a problem hiding this comment.
I imagine we need better error handling here like display a message in a dialog.
I'm not familiar with how other similar errors are handled in Check-ins.
There was a problem hiding this comment.
Added a toast for the error to update the user on what went wrong.
There was a problem hiding this comment.
Nice! If you also want to keep the console output, I would change it to use console.error instead of console.log. That will make it appear in red.
| if (data) { | ||
| dispatch({ type: UPDATE_REVIEW_PERIODS, payload: [...periods] }); | ||
| } else { | ||
| console.log(res?.error); |
There was a problem hiding this comment.
I imagine we need better error handling here like display a message in a dialog.
I'm not familiar with how other similar errors are handled in Check-ins.
There was a problem hiding this comment.
Nice! If you also want to keep the console output, I would change it to use console.error instead of console.log. That will make it appear in red.
| launchDate: isoDate | ||
| }; | ||
|
|
||
| setPeriodToAdd(newPeriod); |
There was a problem hiding this comment.
If you move line 424 into the updateReviewPeriodDates function then you can delete lines 424, 434, and 444.
| /> | ||
| <div className="datePickerFlexWrapper"> | ||
| <DatePickerField | ||
| date={dayjs(launchDate)} |
There was a problem hiding this comment.
What happens if you change this line to date={launchDate} ?
Maybe you don't need the dayjs function.
Same on lines 575 and 590.
There was a problem hiding this comment.
This was ok to remove as long as we use null for empty values.
…s://github.com/objectcomputing/check-ins into feature-2228/planning-review-timeline-calendar
…ing-review-timeline-calendar
| if (data) { | ||
| dispatch({ type: UPDATE_REVIEW_PERIODS, payload: [...periods] }); | ||
| } else { | ||
| console.log(res?.error); |
There was a problem hiding this comment.
Nice! If you also want to keep the console output, I would change it to use console.error instead of console.log. That will make it appear in red.
| templates?.sort((t1, t2) => t1.title.localeCompare(t2.title)); | ||
| setTemplates(templates); | ||
| } else { | ||
| console.log(res?.error); |
There was a problem hiding this comment.
Nice! If you also want to keep the console output, I would change it to use console.error instead of console.log. That will make it appear in red.
| dispatch({ type: UPDATE_REVIEW_PERIODS, payload: data }); | ||
| setLoading(false); | ||
| } else { | ||
| console.log(res?.error); |
There was a problem hiding this comment.
Nice! If you also want to keep the console output, I would change it to use console.error instead of console.log. That will make it appear in red.
| if (data) { | ||
| reviews[period.id] = data[0]; | ||
| } else { | ||
| console.log(res?.error); |
There was a problem hiding this comment.
Nice! If you also want to keep the console output, I would change it to use console.error instead of console.log. That will make it appear in red.
No description provided.