Skip to content

refactor:TopBar#30

Open
jongbin26 wants to merge 12 commits intomainfrom
refactor/top-bar
Open

refactor:TopBar#30
jongbin26 wants to merge 12 commits intomainfrom
refactor/top-bar

Conversation

@jongbin26
Copy link
Member

개요

TopBar 컴포넌트가 복잡해져 리팩토링을 진행했습니다.

작업 사항

  • 컴포넌트 이름에 Children이 붙은 경우 커스텀 훅의 합성 컴포넌트로 이용됩니다.
  • 계층에 맞게 컴포넌트를 분리했습니다.
  • 모달의 사용을 UserMenuContainer.js 파일에 집중시켰습니다.
  • 개별 컴포넌트 내부에 components 폴더를 만드는 구조를 만들었습니다.

예정 사항

  • password 변경 모달에서 Input을 관리하는 과정에서 ref 객체가 외부로 노출되는 상황입니다. 커스텀 훅을 통해 훅 내부에 로직을 주입할 예정입니다.

논의 사항

  • 지저분한 Children을 컴포넌트로 강제 분리하다 보니 props 전달이 너무 많아지는 상황입니다. 좋은 방법에 대해 논의해보면 좋을 듯 합니다!
  • 현재 UserMenuContainer.js 파일에서 모달 사용이 점차 많아지고 있습니다. 모달 사용을 최대한 한 파일 내에서 처리하기 위해 한 파일에 집중시켰지만, Provider 등으로 모달을 하나의 로직으로 관리하는 방법이 없을까 고민해보면 좋을 것 같습니다. -> 모달은 한 번에 하나밖에 뜨지 않기 때문
  • 현재 파일 구조가 TopBar 내부에 components 폴더를 만들어서 TopBar에서 파생된 컴포넌트들을 모두 집어넣고 있습니다. TopBar라는 컴포넌트 내부 요소들을 집중시킬 수 있지만 프로젝트가 커져 컴포넌트가 많아졌을 때 컴포넌트 내부에 components 파일이 중복돼 depth가 두꺼워지는 단점이 있을 것 같습니다. 다른 아키텍쳐가 필요할까 싶네요...!
  • 현재는 API 호출부가 적어 util.js에 API 관련 로직도 넣어놨습니다. 하나의 함수때문에 API 폴더를 만들어 관리할 필요성이 있을까요?

리팩토링 서로 얘기하면서 진행하면 좋을 것 같아서 많은 comment 부탁드립니당🙋🏻‍♂️

@jongbin26 jongbin26 requested a review from Dodolist September 15, 2024 08:19
@jongbin26 jongbin26 self-assigned this Sep 15, 2024
@jongbin26 jongbin26 changed the title Refactor/top bar refactor:TopBar Sep 15, 2024
@jongbin26 jongbin26 requested review from 200516bb, Dodolist, alicehrk and baebae02 and removed request for 200516bb, Dodolist, alicehrk and baebae02 September 15, 2024 08:20
@alicehrk alicehrk requested review from Dodolist and removed request for alicehrk September 15, 2024 08:26
Copy link
Collaborator

@Dodolist Dodolist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

전체적으로 확인해보니 아직 정리가 필요한 부분이 보여요!
util 내부에 util.js안에 코드가 남아있는 부분도 있고
style도 분리하면서 원래 코드에도 제거를 해야하는데 코드가 남아있어보여요.

변경 해주시면 다시 리뷰 진행할게요!

@jongbin26
Copy link
Member Author

PR 리류 남겨주신 사항 모두 반영했습니다!

@jongbin26
Copy link
Member Author

병합 충돌도 변경해놨어요-!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants