-
Notifications
You must be signed in to change notification settings - Fork 3
동아리 상세 페이지 탭 정보 리팩토링 #247
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
동아리 상세 페이지 탭 정보 리팩토링 #247
Conversation
✅ Deploy Preview for hanjari ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
vlmbuyd
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다~ 리뷰 한 번 확인해주세요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P3] layout 디렉토리에 배치되어 있고, 태그를 사용하신 의도가 해당 컴포넌트는 단순 아이템에 대한 개별 카드보다는 레이아웃의 역할에 더 적합하다고 판단하신 것 같아요
하지만, domains/shared/components/card에 개별 아이템 카드를 모아놓은 디렉토리가 있기 때문에 Card보다는 Section 등의 네이밍으로 기존 카드 컴포넌트들과 혼동되지 않도록 레이아웃의 뉘앙스를 더 살려보는 건 어떨까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ClubDetailSection도 괜찮은 네이밍 같네요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오호 lib랑 utils랑 비슷한 의미인줄 알고 헷갈렸는데 감사합니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P2] 해당 타입들이 사용되는 맥락을 살펴봤을 때 여러 도메인에 걸쳐 사용되기 보다는
동아리 소개(introduction) 부분에서 주로 사용되는 타입으로 보입니다.
shared보다는 domains/club/introduction에서 관리하는 것이 좋아보이는데 shared에 두신 이유가 있으신가요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
introduction에 두는게 더 맞는 것 같네요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P3] 개인적으로 Info나 Data 같은 접미사는 네이밍만 길어지게 할 뿐, 실질적인 의미를 더하지 못한다고 생각합니다.
(대부분의 코드가 정보나 데이터를 다루기 때문입니다.)
'동아리 상세'를 다룬다는 맥락은 이미 충분하므로 club-detail로 수정하여 가독성을 높여보는 건 어떨까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
동아리 상세 페이지를 club-detail이라는 네이밍으로 관리하고 있어서 club-detail의 무언가라는 의미를 주기 위해서 -info라는 접미사를 붙였습니다 club-detail 페이지에서 계속 작업하던 저의 입장에서 club-detail에 속하는 컴포넌트지만 아무런 접미사 없는 네이밍을 가질 때 더 혼동이 생길 것 같다고 생각했는데 큰 문제가 되지 않는다면 네이밍을 유지해도 괜찮을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
말씀하신 의도(club-detail의 무엇인지 명시)는 충분히 이해했습니다.
다만, 아키텍처 구조상 domains/shared 디렉토리의 목적과 향후 유지보수성을 고려했을 때 아래와 같은 우려점이 있어 의견 드립니다.
1. shared의 목적과 네이밍의 괴리
domains/shared는 특정 도메인(클럽 상세)뿐만 아니라 어드민, 검색 등 다양한 맥락에서 재사용하기 위해 만든 공간입니다.
여기에 club-detail-info처럼 특정 페이지 맥락이 강한 접미사가 붙기 시작하면, 추후 club-detail-header, club-detail-footer 등 페이지 종속적인 컴포넌트들이 shared 내부에 무분별하게 쌓이는 파편화가 발생할 우려가 있습니다.
2. 기능 중심의 네이밍
첨부한 이미지의 예시처럼, shared/components 내부의 다른 디렉토리들은 (사용되는 모든 맥락을 이름에 담을 수 없기에) 도메인 맥락보다는 컴포넌트의 '기능'에 초점을 맞춰 네이밍하고 있습니다.
따라서 해당 컴포넌트 또한 Text와 같이 기능 위주의 네이밍을 우선적으로 고려해 주시기를 제안드립니다.
만약 기능만으로 명명하기 모호하다면, -info처럼 너무 구체적인 접미사보다는 차라리 club-detail 정도로 범용성을 열어두는 편이 낫다고 생각합니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P2] 해당 타입들이 사용되는 맥락을 살펴봤을 때 여러 도메인에 걸쳐 사용되기 보다는
동아리 소개(introduction) 부분에서 주로 사용되는 타입으로 보입니다.
shared보다는 domains/club/introduction에서 관리하는 것이 좋아보이는데 shared에 두신 이유가 있으신가요?
같은 맥락에서 shared보다는 introduction에서 관리하는게 좋을 것 같습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
폴더 구조 리팩토링 작업해주신 거에는 아직 api 관련 훅들도 query type 상관없이 use 접두사를 붙이는 형식으로 되어있는 것 같은데 이 부분은 각자 작업하면서 수정하면 되는 건가요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넵 api 관련 훅은 네이밍 컨벤션 참고하여 각자 작업하면서 수정해나가면 될것 같습니다


#️⃣ 연관된 이슈
📝 작업 내용
탭 정보 수정
동아리 기본 정보 fallback message 삭제 -> 전달받은 정보만 표시
기타 스타일 변경 대응
고민했던 점
만들고 나서 코드를 보니
<ClubDetailCard/>컴포넌트를<ClubDetailText/>컴포넌트 내부의 최상위에 넣어두면 호출부에서<ClubDetailCard/>컴포넌트 호출 없이<ClubDetailText/>,<ClubDetailSchedule/>,<ClubDetailRecruit/>만 호출할 수 있어서 더 간결하고 보기 좋지 않을까? 하는 생각이 들었습니다.그렇게 수정했을 때 코드는 더 짧아지겠지만 컴포넌트화 했을 때의 가장 큰 이점 중 하나인 재사용성의 측면에서 봤을 때 ClubDetailText 안에 ClubDetailCard를 고정해버리면, 나중에 이 컴포넌트를 카드 형태가 아닌 곳에서 사용할 수 없게 됩니다.
또한 지금 현재 코드가 반복적인
<ClubdetailCard/>컴포넌트로 인해 비효율적이라고 느껴질 수 있지만 오히려 '어디에 담길지', '무엇을 보여줄지'를 명확하게 보여줄 수 있는 구조이기도 하다고 생각했습니다.이러한 이유로 현재 구조 결정!!
💬 리뷰 요구사항(선택)