feat: 어드민 대시보드 페이지 추가 #197
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
기존에는 각자 브랜치에서 삭제를 해왔었는데, 삭제용 브랜치를 따로 파서 진행하는 것이 더 좋아보이네요~! |
src/components/Input.tsx
Outdated
| return ( | ||
| <input | ||
| className={`border-lightGray focus:outline-mainGreen w-full rounded-lg border p-3 text-lg focus:outline-2 ${className}`} | ||
| className={twMerge( |
There was a problem hiding this comment.
공통 Input 컴포넌트에 cn 유틸함수가 적용되어있지 않았군요. 다른 컴포넌트들과 일관성을 위해 twMerge보다 유틸 함수 cn을 활용해도 좋을 것 같습니다!(/src/utils/classname.ts)
There was a problem hiding this comment.
기존 twMerge 사용중인 부분들 전체적으로 cn으로 변경 완료하였습니다. 좋은 제안 감사합니다!
There was a problem hiding this comment.
src/components/ui 디렉토리는 shadcn 컴포넌트들이 위치하는 곳으로 알고 있습니다. 현재 디렉토리에 해당 파일이 위치할 경우 헷갈릴 수 있을 것 같아요 다른 적절한 위치로 파일을 이동하는 것은 어떨까요?
There was a problem hiding this comment.
현재 @redzzzi 님께서도 해당 admin 파일 수정 사항이 feature 브랜치에 존재하여, 추후 머지된 이후에 위치 수정하도록 하겠습니다!
src/components/ui/admin.tsx
Outdated
| ); | ||
| }; | ||
|
|
||
| export const AdminNoData = ({ style }: React.ComponentProps<'div'>) => { |
There was a problem hiding this comment.
style prop은 인라인 스타일 객체로 기대돼서 조금 헷갈리거나 타입 문제가 발생할 수 있을 것 같습니다. classname으로 변경하는 것은 어떨까요? 혹은 특별한 이유가 있을까요?
There was a problem hiding this comment.
실수로 className이 아니고 style로 작성하였네요, 꼼꼼하게 확인해주셔서 감사합니다!
| {contest.categoryName} | ||
| </div> | ||
| </div> | ||
| <Link to={`/admin/contest/${contest.contestId}`} className="flex items-center gap-2"> |
There was a problem hiding this comment.
Link가 텍스트와 아이콘 영역에만 적용되어 있어 다른 부분 클릭 시 이동이 되지 않는데요, hover시 색상이 변경되는 부분은 AdminCardRow이다 보니 UX적으로 사용자가 느끼기에 클릭했는데 왜 이동이 안되는거지? 라는 생각이 들 수 있을 것 같습니다!
hover시 색상이 변경되는 부분 어디서 클릭이 일어나도 이동이 가능하면 좋을 것 같아요
There was a problem hiding this comment.
좋은 의견 감사합니다! AdminCardRow 전체에 Link가 걸리도록 수정 완료하였습니다!
| </div> | ||
| </div> | ||
| <div className="flex items-center justify-center gap-6"> | ||
| <Button className="rounded-3xl border border-red-400 px-6 py-2 text-red-400">취소하기</Button> |
There was a problem hiding this comment.
취소하기를 클릭해도 아무런 동작을 하지 않습니다!
There was a problem hiding this comment.
해당 컴포넌트는 대회 생성 부분이라, feat/admin-dashboard 브랜치에 별도 수정사항이 추가로 있습니다!
| toast('카테고리 이름을 입력해주세요.', 'error'); | ||
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
입력값 제출 시에 trim()으로 공백을 제거해주는 것은 어떨까요?
There was a problem hiding this comment.
좋은 생각인 것 같습니다! 수정 완료했습니다.
| {notices?.map((notice) => ( | ||
| <AdminCardRow key={notice.noticeId} className="border-b-lightGray not-last:border-b"> | ||
| <div className="flex gap-5"> | ||
| <div>{notice.noticeId}</div> |
There was a problem hiding this comment.
개인적으로는 목록에 번호를 노출할 때, 고유 id보다는 index를 사용하는 것이 관리자 입장에서 개수나 순서 파악하기에 더 직관적일 것 같습니다. 이 부분 의견 여쭤보고 싶어요!!
There was a problem hiding this comment.
index를 보여줄 경우 API에서 반환되는 순서는 최신순이지만, 화면에 표시되는 순서는 늦게 작성된 공지사항이 1번이 매겨지게 되어 혼란이 있을 수도 있을 것 같습니다. 아예 번호 자체를 표시하지 않는 방식이 저는 제일 좋은 것 같은데 어떻게 생각하실까요?
There was a problem hiding this comment.
오 그것도 좋습니다! 다른 목록도 번호를 표시하지 않는 것으로 통일하도록 하겠습니다.
3 participants
📝 개요
어드민 페이지 진입 시 제일 첫 화면인 대시보드(/admin) 페이지를 추가하는 작업
🎯 목적
리뉴얼 디자인에 맞게 UI 구성 및 API 변경/추가 부분 반영
✨ 변경 사항
🔬 리뷰 요구 사항
제가 관리하는 다른 브랜치인 대회 관리(feat/contest-manage)및 대회 생성(feat/contest-create)랑 연관된 UI 컴포넌트가 많아 재사용을 위해 각 feature 브랜치끼리 머징을 하다 보니 커밋 내역이 각 브랜치끼리 겹치는 상황이 발생했습니다.
보다 편한 코드 리뷰를 위해 앞으로는 feature 브랜치 간 공유되어야 할 수정사항이 있을 경우 머지보다는 cherry pick을 활용하여 재발하지 않도록 주의하겠습니다.
DashboardPage에 연관된 아래 컴포넌트만 중점적으로 봐주시면 감사하겠습니다.
💬 논의 사항
기존 어드민에서 사용하던 컴포넌트들을 삭제하는 작업은 브랜치를 별도로 생성해서 삭제하는 것이 좋을까요, 아니면 각 Feature 브랜치에서 관련된 컴포넌트를 삭제하면 좋을까요?
📸 참고 이미지/영상