[Feat/#13] 프로젝트 사이드바 구현, 토글 UI/애니메이션 구현 및 wds 아이콘 추가#15
Conversation
There was a problem hiding this comment.
리뷰 몇 개 남겨두긴 했는데 변경하구 다시 리뷰할게용~ 고생햇습니다 일단 바꿔야 할 것 같은 건!!
- (임시로 한 애들 빼고) 디자인 시스템 안 맞춰서 한 rgba, #~으로 색상 넣은 애들 네이밍 맞춰서 수정해 주기 => 이러면 나중에 색상 다같이 바꾸거나 할 때 하나하나 찾아서 수정해야 해요 ㅠㅠ
- px 단위 다 rem으로 바꾸기
- svg tsx... import 해서 사용하거나 svgr 세팅해서 svg 형태 뽑아주기! -> 지금 상태면 아이콘이 어떻게 생겼는지 다른 사람이 확인이 안 돼용...
- 그리고 제가 확인했을 때 펼치는 아이콘이 /project 경로에서는 확인이 안 되는 것 같은데 이것두 한 번만 확인해 주세용
- 폰트도 자간이랑 폰트 직접 설정하지 말구
globals.css에 있는 친구 불러서 사용하기!
고생햇어용 일단 저 밥 먹어야 해서 밥 먹구 다시 확인할게욥
sinji2102
left a comment
There was a problem hiding this comment.
뭔가 애니메이션 안 튀게 하려구 고민한 흔적 + 매직넘버 뽑구 아리아 라벨까지 꼼꼼하게 작성한 게 보여서 좋네용 리뷰 꼼꼼하게 확인하구 반영해 주기!
| const DEFAULT_PROJECT_NAME = '플로밋 기획'; | ||
| const DEFAULT_USER_NAME = '황수민'; | ||
| const DEFAULT_USER_EMAIL = 'tnals655@naver.com'; | ||
|
|
||
| const SIDEBAR_EXPANDED_WIDTH = 220; | ||
| const SIDEBAR_COLLAPSED_WIDTH = 56; |
There was a problem hiding this comment.
P1) 혹시 이렇게 상수화 해둔 이유가 잇을까용?... 나중에 수정할 용으로 사용한 상수화면 이러면 파일마다 사용해야 해서 constant 폴더에 음... 네이밍 exampleCosntant 정도로 상수화 파일 하나 생성하는 건 어떻게 생각하나요??
그리구 이건 그냥 궁금한 건데 WIDTH를 따로 상수화 해둔 이유가 있을까요?? 매직 넘버로 사용하려던 의도였으면 매직 넘버 파일 하나 만드는 건 어떻게 생각하시나욥... ㅎㅎ 이건 좀 얘기를 해봐야 할 듯!
There was a problem hiding this comment.
\frontend/src/constants/exampleConstant.ts 생성 및 분리해두었습니다.
width 값은 상수화하지 않고, 사이드바에서만 사용하는 값이라 내부 사용 위치에 직접 두도록 수정했습니다.
There was a problem hiding this comment.
P1) 의도한 내용은 \frontend/src/constants/exampleConstant.ts에
const DEFAULT_PROJECT_NAME = '플로밋 기획';
const DEFAULT_USER_NAME = '황수민';
const DEFAULT_USER_EMAIL = 'tnals655@naver.com';
을 객체로 만들어서 공유해서 사용하면 다들 재사용해서 사용할 수 잇을 것 같다는 뜻이었어요!!
지금 위 내용들 사라져서 사이드바에 프로필이 사라진 것 같은데 한 번 확인해 주세요!
| projectName = DEFAULT_PROJECT_NAME, | ||
| userName = DEFAULT_USER_NAME, | ||
| userEmail = DEFAULT_USER_EMAIL, |
There was a problem hiding this comment.
P3) UX 관점에서... 로딩 안 됐을 때 보여줄 디폴트 상수들 임시로 박아두지 말고 따로 빼서 세팅하는 게 좋을 것 같아요!
There was a problem hiding this comment.
로딩 전 임시 디폴트 값이 노출되지 않도록 기본값은 제거했고, 별도로 관리할 값은 상수 파일로 분리해두었습니다.
| <SearchMenuButton isCollapsed={isCollapsed} /> | ||
| <AlarmMenuButton isCollapsed={isCollapsed} /> | ||
| <ProjectSettingMenuButton isCollapsed={isCollapsed} /> |
There was a problem hiding this comment.
P1) 제가 이해한 바로는 세 개의 코드가 다 같은 디자인 + 같은 역할을 하는 것 같은데 하나의 컴포넌트로 만들고 아이콘 + 텍스트만 props로 넘기는 건 어떻게 생각하시나요? 수신함이 걱정이면 컴포넌트를 넘길 수 있게 하거나 수신함만 분기 처리를 하면 좋을 것 같아요~
There was a problem hiding this comment.
공통 컴포넌트로 묶었습니다. SidebarMenuButton을 추가해서 아이콘, 라벨, 뱃지 여부만 props로 받도록 정리했고, 각 버튼 컴포넌트는 공통 컴포넌트를 사용하는 형태로 변경했습니다.
There was a problem hiding this comment.
P3) 공통 컴포넌트 사용 관련해서 리뷰 남겼는데 한 번 확인해 주세요~
| className="h-screen shrink-0 overflow-hidden border-r border-[rgba(112,115,124,0.16)] bg-[rgba(247,247,248,1)] px-4 py-2" | ||
| initial={false} | ||
| animate={{ width: isCollapsed ? SIDEBAR_COLLAPSED_WIDTH : SIDEBAR_EXPANDED_WIDTH }} | ||
| transition={{ duration: 0.24, ease: 'easeInOut' }} |
There was a problem hiding this comment.
P3 ) 제가 라이브러리 미숙 이슈로 확실하게 이해한 건지는 모르겠지만...!! 상단의 setTimeOut과 duration이 같은 숫자여야 할 것 같은데 위 코드처럼 상수화해서 매직넘버로 관리하는 건 어떨까요~~
| const collapseTimer = window.setTimeout(() => { | ||
| setIsCollapseSettled(true); | ||
| }, 240); |
There was a problem hiding this comment.
P4) 이거 제가 라이브러리... 잘 못 써서 확실하진 않은데 AnimatePresence 같은 거 사용하면 타이머 없이두 가능할 것 같기도 하고... 아닌 것 같기도 하고... 제가 다시 한 번 알아보긴 할 텐데 같이 한 번 봐주세요!
관련 아티클 일단 남기고 갑니디ㅏ~
https://velog.io/@keumky1/2.-Framer-Motion-%EC%9D%91%EC%9A%A9-layout-AnimatePresence
There was a problem hiding this comment.
위에서 언급한대로 onAnimationComplete 콜백으로 변경해두었습니다
There was a problem hiding this comment.
지금 사이드바 케이스에서는 요소를 없애는 문제라기보다 width 애니메이션이 언제 끝났는지가 중요해서 AnimatePresence보다는 onAnimationComplete를 사용해서 수정했습니당
There was a problem hiding this comment.
와웅 굿~... 사실 잘 몰라서 긴가민가 했는데 알아서 잘 수정해 줬네용
There was a problem hiding this comment.
리뷰 반영하느라 고생 많았어요!!! 관련해서 살짝... 수정사항 더 있어서 살짝만 남겨뒀는데 반영 부탁드려요!!
그리고 너무 많아서 다 못 남겼는데 테스트용 페이지 외에두 폰트랑 색상 타이포랑 색상에 안 맞는 애들 + px 사용하는 것들 있어서 한 번 전체적으로 확인해 주세요~
그리구 AI 이용해서 공부하면... 속도도 빠르고 좋긴 한데 지금 너무 투머치로 사용되어서 코드가 복잡해지는 것 같아서!!! 조금 더 명확하게 구체화해서 사용하거나 요구사항을 명확하게(개발자 관점에서) 써야 할 것 같아요!! AI 쓰면 PR 날리기 전에 복잡해진 부분이나 관련 없는 코드들 없는지 한 번만 확인해 주기 ㅎㅎ 의존도 쪼금만 낮추면 좋을 것 같아요.... 🥲
그리구 common 폴더명 복수형으로 commons로 쓰기!! 폴더명은 복수로 쓰는 게 컨벤션임미다
| <SearchMenuButton isCollapsed={isCollapsed} /> | ||
| <AlarmMenuButton isCollapsed={isCollapsed} /> | ||
| <ProjectSettingMenuButton isCollapsed={isCollapsed} /> |
There was a problem hiding this comment.
헉 제가 의도한 리뷰는... 공컴을 만들어서 아래처럼 사용하면 좋을 것 같다는 뜻이었어요~
| <SearchMenuButton isCollapsed={isCollapsed} /> | |
| <AlarmMenuButton isCollapsed={isCollapsed} /> | |
| <ProjectSettingMenuButton isCollapsed={isCollapsed} /> | |
| <SidebarMenuButton icon={IconSearch} isCollapsed={isCollapsed} label="검색" labelWidth={48} /> | |
| <SidebarMenuButton | |
| icon={IconBell} | |
| isCollapsed={isCollapsed} | |
| label="수신함" | |
| labelWidth={64} | |
| badgeText="+99" | |
| /> | |
| <SidebarMenuButton icon={IconSetting} isCollapsed={isCollapsed} label="설정" labelWidth={48} /> |
이렇게 사용하면 기존에 있던 Button.tsx 세 개 파일은 삭제해두 되겟죠!!! 크게 변경될 일이 없는 컴포넌트라 나중에 유지보수 차원에서도 더 좋을 것 같아요~
그리구 나중을 대비해서 각 버튼에 onClick 처리두 미리 해놓으면 좋을 것 같아요!
There was a problem hiding this comment.
SidebarMenuButton 공통 컴포넌트로 통일수정햇습니다.
| const DEFAULT_PROJECT_NAME = '플로밋 기획'; | ||
| const DEFAULT_USER_NAME = '황수민'; | ||
| const DEFAULT_USER_EMAIL = 'tnals655@naver.com'; | ||
|
|
||
| const SIDEBAR_EXPANDED_WIDTH = 220; | ||
| const SIDEBAR_COLLAPSED_WIDTH = 56; |
There was a problem hiding this comment.
P1) 의도한 내용은 \frontend/src/constants/exampleConstant.ts에
const DEFAULT_PROJECT_NAME = '플로밋 기획';
const DEFAULT_USER_NAME = '황수민';
const DEFAULT_USER_EMAIL = 'tnals655@naver.com';
을 객체로 만들어서 공유해서 사용하면 다들 재사용해서 사용할 수 잇을 것 같다는 뜻이었어요!!
지금 위 내용들 사라져서 사이드바에 프로필이 사라진 것 같은데 한 번 확인해 주세요!
There was a problem hiding this comment.
P1) 지금 써있는 친구들은 예제로 사용할 값이 아니라 계속 사용할 값들이니까 다른 상수 파일들로 빼고 해당 파일에는 프로필 객체처럼 나중에 API 연결하면 사용 안 되지만 연결 전까지 다같이 사용할 예정인 값들을 넣으면 좋을 것 같아요~
| <SearchMenuButton isCollapsed={isCollapsed} /> | ||
| <AlarmMenuButton isCollapsed={isCollapsed} /> | ||
| <ProjectSettingMenuButton isCollapsed={isCollapsed} /> |
There was a problem hiding this comment.
P3) 공통 컴포넌트 사용 관련해서 리뷰 남겼는데 한 번 확인해 주세요~
| const collapseTimer = window.setTimeout(() => { | ||
| setIsCollapseSettled(true); | ||
| }, 240); |
There was a problem hiding this comment.
와웅 굿~... 사실 잘 몰라서 긴가민가 했는데 알아서 잘 수정해 줬네용
#️⃣연관된 이슈
closes #13
📝작업 내용
스크린샷 (선택)
💬리뷰 요구사항(선택)