Feat: 일정 찾기 페이지 필터 추가 #44
Conversation
pepperdad
requested review from
Choi-Jinwook and
NacreousCloud
and removed request for
Choi-Jinwook
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -24,6 +24,7 @@ | |||
| "@notice/*": ["notice/*"], | |||
| "@faq/*": ["faq/*"], | |||
| "@mypage/*": ["mypage/*"], | |||
| "@findSchedule/*": ["findSchedule/*"], | |||
| "@create-schedule/*": ["create-schedule/*"] | |||
| } | |||
| }, | |||
There was a problem hiding this comment.
이 코드 패치는 상대적으로 작은 수정 사항입니다. 추가된 부분은 "@findSchedule/" 항목이며, "findSchedule/" 경로를 가진 파일에 대한 권한을 설정합니다.
버그 위험 요소는 주어진 코드 조각에서는 명확히 확인되지 않습니다. 하지만 전체 시스템 아키텍처와 상황에 따라 발생할 수 있는 다른 문제를 고려해야 합니다.
코드 개선 제안:
- 리뷰가능한 코드 양 또는 새로운 코드 기능에 따라 맥락이 다르기 때문에 더 자세하거나 구체적인 개선 제안을 할 수 있습니다.
| const [theme, setTheme] = useState("전체"); | ||
| const [date, setDate] = useState<DateProps>({ | ||
| start: undefined, | ||
| end: undefined, | ||
| }); | ||
| const [showCalendar, setShowCalendar] = useState({ | ||
| start: false, | ||
| end: false, | ||
| }); | ||
| const [expense, setExpense] = useState<string>(""); | ||
| const [personnel, setPersonnel] = useState<string>(""); |
There was a problem hiding this comment.
[Lv2] 필터를 하나로 묶어서 관리할 수 있을 것 같습니다!
const [filter, setFilter] = useState({
theme: "전체",
date: { start: undefined, end: undefined },
showCalendar: { start: false, end: false },
expense: "",
personnel: "",
}이렇게 하나의 객체에서 관리하면 라디오 버튼에 대한 클릭처럼 중복되는 부분은 하나의 함수에서 관리가 가능하고, props로 넘겨줄 때도 {...filter} 처럼 더 깔끔하게 넘겨줄 수 있을 것 같은데 어떻게 생각하시나요??
There was a problem hiding this comment.
useReducer로 분리하는것도 한번 제안드려봅니다ㅎㅎ 둘다 성능에 큰 차이는 없으니 선호하시는 방향으로 해보세요!
| </div> | ||
| {visible && ( | ||
| <div className="absolute top-[38px] left-0 z-30"> | ||
| <Calendar value={date || undefined} onClickDay={handleDateChange} /> |
There was a problem hiding this comment.
[Lv2] date의 타입이 이미 Date | undefined 라서 || undefined는 안붙여도될것같습니다!
| const [theme, setTheme] = useState("전체"); | ||
| const [date, setDate] = useState<DateProps>({ | ||
| start: undefined, | ||
| end: undefined, | ||
| }); | ||
| const [showCalendar, setShowCalendar] = useState({ | ||
| start: false, | ||
| end: false, | ||
| }); | ||
| const [expense, setExpense] = useState<string>(""); | ||
| const [personnel, setPersonnel] = useState<string>(""); |
There was a problem hiding this comment.
useReducer로 분리하는것도 한번 제안드려봅니다ㅎㅎ 둘다 성능에 큰 차이는 없으니 선호하시는 방향으로 해보세요!
| @@ -151,3 +170,4 @@ a { | |||
| font-weight: 700; | |||
| src: url('/fonts/NotoSansKR-Bold.ttf'); | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
이 코드 패치에 대해 간단한 코드 리뷰를 도와드리겠습니다.
-
@apply 문법을 사용하여 a 태그에 스타일 속성을 추가합니다. 이 코드는 문제가 없어 보입니다.
-
.radio 클래스에 관한 스타일 속성들이 정의되어 있습니다. 라디오 버튼을 커스터마이징하는 것 같습니다. 오래된 브라우저에서도 동작할 수 있도록 -webkit-appearance와 -moz-appearance 속성이 설정되어 있는 것이 좋습니다.
-
.radio:checked 스타일은 체크된 라디오 버튼에 대한 스타일 속성을 정의하고 있습니다. 기본적으로 문제가 없어 보이지만, 맥 OS에서 Checkbox나 Radio(라디오) 버튼에 적용되는 background-color 속성과 box-shadow 속성은 크로스 브라우징을 위해 다양한 테스트가 필요할 수 있습니다.
-
.truncate-text 클래스는 텍스트를 잘라서 표시하는 스타일 속성을 정의하고 있습니다. 이 코드는 문제가 없어 보입니다.
-
fonts 디렉토리에서 NotoSansKR-Bold.ttf 폰트 파일을 로딩하는 스타일 속성이 있는것이 좋습니다. 폰트 파일의 경로가 올바른지 확인하세요.
다음으로 주의해야 할 점은 크로스 브라우징을 고려하고, 코드의 일관성과 가독성을 유지하는 것입니다. 이러한 점들에 유의하여 코드를 개선할 수 있습니다.
구현 사항
테스트
TODO