feat-fe: Sidebar 컴포넌트 개선 #773
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
1728478745.813049 |
|
1728478749.440239 |
|
1728522108.324619 |
There was a problem hiding this comment.
러기 고생하셨어요! 공고 상태 관리 관련하여 한 가지 궁금한 게 있어 일단 코멘트 먼저 드릴게요. 기능이나 디자인 측면에서는 모두 의도하신대로 잘 구현된 것 같습니다.
오픈되어 있는 공고가 하나도 없는 경우에는 "모집 공고" 버튼 아래로 영역 전체가 텅 비게 될텐데요, 이 부분을 어떻게 처리할지는 기획 측면의 이슈여서 저도 고민해 보겠습니다.
아이콘 부분에 대한 제 의견은 다음과 같습니다.
- React-Icons의 아이콘들을 여럿 살펴봤는데, 러기가 선택해주신 아이콘 이상으로 공고 상태의 의미가 잘 드러나는 아이콘을 찾기는 어려웠어요.
- 마감된 공고에 한해서는, 체크 표시보다는 자물쇠 표기가 어떨까 제안드려 봅니다.
- 아이콘들의 사이즈가 글자 대비 크게 느껴져서, 이것의 크기를 조금 줄이는 방안을 제안드려 볼게요.
| export interface RecruitmentStatusObject { | ||
| status: RecruitmentStatusType; | ||
| isPending: boolean; | ||
| isOngoing: boolean; |
There was a problem hiding this comment.
공고가 가질 수 있는 3가지 상태에 대해 각기 다른 boolean 필드로 정의해 주셨네요. 유니언 타입이 아니라 이렇게 상태별 필드값을 따로 가져야 할 이유가 있을까요? 하나의 status에 대한 관리를 isPending, isOngoing, isClosed의 3개 필드로 관리하시는게 DX 측면에서 번거로우시진 않을까 하여 여쭤봅니다.
There was a problem hiding this comment.
이 부분은 개발자가 선택적으로 사용할 수 있게 만들어서 오히려 관리하기 편하다고 생각했는데요! 예를들어 if문을 사용하는 경우를 본다면,
if(Recruitment.isPending) {}
if(Recruitment.status === "Pending") {} // string을 상수로 분리해주어야하는 번거로움이렇게 두 가지 경우를 사용할 수 있을거라 생각했어요. 또 이번에 만든 Icon 객체인
const IconObj: Record<RecruitmentStatusObject['status'], IconType> = {
Pending: GrDocumentTime,
Ongoing: GrDocumentUser,
Closed: GrDocumentVerified,
};
const Icon = iconObj[status]이런 형식으로 사용할 수도 있으니, 두 가지 경우를 모두 사용한다면 유연하게 사용할 수 있어서 좋지 않을까? 생각해요.
사용하는 입장에선,필드값이 몇개인지 생각할 필요 없이 단순히 사용하는 값을 가져오기만 하면 되니 개발경험 측면에서도 전혀 번거롭지 않다고 생각해요!
아르가 말씀해주신 'DX 측면에서 번거로우시진 않을까'에 대해서 공감하지 못하고 있는데, 혹시 어떤 상황을 말씀하시는지가 궁금하네용!
There was a problem hiding this comment.
개별 공고에 대한 상태를 정의한 유니온 타입의 RecruitmentStatusType이 이미 있는데, 여기에 각 상태별 boolean 값을 별도의 필드로 하나씩 떼어서 RecruitmentStatusObject를 따로 정의하신 것이 이후의 상태 관리에 번거로움을 더하진 않을까 우려되어서 드린 말씀이었어요.
전체 코드를 보니 Recruitment.status === "Pending"을 Recruitment.isPending으로 줄이는 것 이외의 이점은 크게 없어보이는데, 이에 비해 getTimeStatus의 리턴값으로는 { status: 'Pending'} 대신 { status: 'Pending', isPending: true, isOngoing: false, isClosed: false }이 되는 트레이드 오프가 있어 보였거든요.
현재로서는 관리해야 할 상태의 종류가 3개 뿐이고, 추후 변동될 가능성도 많지 않아서 관리 포인트에 큰 문제가 생길 것 같진 않습니다. 일단 요렇게 유지하되, 추후에 공고 관련 상태에 대해 변경해야 할 사안이 생기면 다시 검토해도 좋을 것 같습니다.
| <S.Divider /> | ||
|
|
||
| {sidebarContentList.map(({ title, posts }) => { | ||
| if (posts.length === 0) return; |
There was a problem hiding this comment.
'렌더링하지 않는다'는 return null로 명시적으로 표현해주세요.
| const pendingPosts = useMemo(() => options?.filter(({ status }) => status.isPending), [options]); | ||
| const onGoingPosts = useMemo(() => options?.filter(({ status }) => status.isOngoing), [options]); | ||
| const closedPosts = useMemo(() => options?.filter(({ status }) => status.isClosed), [options]); |
목적
작업 세부사항
참고 사항
COM_SIDEBAR
closes #772
기술 공유 사항
DashboardLayout은 Sidebar 섹션과 Main 섹션으로 나뉩니다. Main섹션은 full스크린 에서 Sidebar만큼 width값을 뺀 크기를 차지해야 합니다.
피드백 받고 싶어요.