-
Notifications
You must be signed in to change notification settings - Fork 1
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
Refactor/#293: 모달 합성 컴포넌트 적용 && 지도 페이지 의존성 주입 Context API 적용 #296
Conversation
- 새로운 모달이 필요할 때마다 비슷한 구조의 컴포넌트를 중복해서 만드는 것은 비효율적이기 때문에 합성 컴포넌트를 적용 - 모달은 기본적으로 모달의 제목, 버튼을 가지는 구조이기 때문에 이를 구현
- ts 확장자 파일에서는 리액트 컴포넌트를 호출할 수 없어, 모달 컴포넌트를 대신 호출해주는 handleOpenModal 함수 구현
- 모달을 다루는 함수 추가로 분리 - 가독성을 위해서 공백 코드 라인 추가
- 커스텀 훅 사용 - 버튼의 자식 요소였던 span 제거
- Map(Provider) 컴포넌트만 의존해서 렌더링할 수 있음 - Provider에서 의존성을 주입하고, 커스텀 훅으로 로직을 꺼내서 사용하기 때문에 의존성 주입을 외부에 드러내지 않아도 됨
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.
확실히 하나의 파일에 코드가 줄어드는거 같아 보기는 편해지는거 같아요
근데 지도 로직은 아직 어렵네요.. ㅎㅎ
<Suggestion> | ||
<Button data-testid="modal" onClick={handleSuggestionModal}> | ||
<Button data-testid="modal"> | ||
<Icon kind="suggest" color={THEME.TEXT.WHITE} /> |
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.
여기에 onClick 이벤트가 사라지면 건의사항 남기기는 어떻게 처리가 되나요?
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.
건의사항 남기기는 이제 페이지로 만들거여서 일단 건의사항 관련 로직은 없앴어요~
src/utils/map/get-building-info.ts
Outdated
const getBuildingInfo = ( | ||
keyword: string, | ||
): [BuildingType, number] | undefined => { | ||
const splittedKeyword = keyword.split(' ').join('').toUpperCase(); |
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.
split 후 다시 합치는것보다 정규표현식 또는 keyword.replaceAll(" ", "").toUpperCase() 는 어떨까요?
🤠 개요
💫 설명
이렇게 총 3가지가 있는데, 기존에는
props
로 필요한 의존성들을 주입해줬는데 지도를 구성하는 각 컴포넌트들이 받는props
가 중복되기도 하고 중복된props
를 넘겨주니 가독성도 떨어지는 것 같아서Context API
를 사용해서 의존성을 주입해봤어요.사용할 때는, 커스텀 훅을 호출해서 의존성들을 사용할 수 있도록 했어요.
커스텀 훅 많이 활용하기
이번 작업을 하면서, 렌더링 로직에 관여하는 훅을 사용할 때 커스텀 훅으로 최대한 만들어 보려고 했는데 이렇게 해보니
이 2가지의 관심사 분리가 잘 되는 것 같아서 좋은 것 같더라고여(대충 저번에 젭에서 최대한 커스텀 훅을 활용해야 한다고 했던 대화..)
그래서, 앞으로도 렌더링 로직이 복잡하다 싶으면 커스텀 훅으로 최대한 빼는게 좋을 것 같아요. 추상화의 장점도 있어서 좋은 것 같네요
📷 스크린샷 (Optional)