Conversation
|
Eric, great work so far! It looks like you're getting pretty familiar with the codebase. Your code looks very well thought-out. I added some inline comments (most of which should be really easy to fix). I'd also like to hear your opinion on the following questions:
|
| }, | ||
| }; | ||
| // Convert numeric fields to numbers | ||
| if (field === 'birth_month' || field === 'birth_day') { |
There was a problem hiding this comment.
Some minor styling issues, like single vs. double quotes. Should be super easy to fix automatically—just run npm run lint -- --fix (which should catch most of them); fix the outstanding ones manually
| }; | ||
|
|
||
| // Add all filters | ||
| addFilter('school'); |
| const [yearOptions, setYearOptions] = useState<DropdownOption[]>([]); | ||
| const [collegeOptions, setCollegeOptions] = useState<DropdownOption[]>([]); | ||
| const [majorOptions, setMajorOptions] = useState<DropdownOption[]>([]); | ||
| const [selectedDate, setSelectedDate] = useState<Date | null>(null); |
There was a problem hiding this comment.
Can we move the date picker into a new component? might make the code a little cleaner to read.
| }) => ( | ||
| <div className={styles.calendarHeader}> | ||
| <button onClick={decreaseMonth} disabled={prevMonthButtonDisabled}> | ||
| {"<"} |
There was a problem hiding this comment.
is there any way we can use the FontAwesome caret-left and caret-right icons instead of the unicode greater-than and less-than characters? might match the UI a bit more.
| value={date.getMonth()} | ||
| onChange={({ target: { value } }) => changeMonth(parseInt(value))} | ||
| > | ||
| {Array.from({ length: 12 }, (_, i) => ( |
There was a problem hiding this comment.
in general we try to avoid too much JS inside JSX html code. Can we move this to a variable that's generated before the return statement?
| } | ||
| } | ||
|
|
||
| .datePickerPopper { |
There was a problem hiding this comment.
it looks like you have a good grasp on SCSS! hope you agree with the decision to use it over vanilla CSS.
| position: absolute; | ||
| z-index: 1000; | ||
|
|
||
| :global { |
There was a problem hiding this comment.
it sucks that we have to do this, but i guess there's no way around it.
2 participants
Added a filter for birthday, sends birth day and birth month to backend API when fetching people using elastic search. This gets updated when a date gets selected from the calendar select. Used DatePicker from "react-datepicker" (added this to the packages). Added some basic styling but can be improved with more ideas.