Skip to content
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

식당 마커 클릭 시 식당정보 미리보기 구현 #197

Merged
merged 9 commits into from
Dec 13, 2022
Merged

Conversation

junghyunbak
Copy link
Member

링크(관련 이슈)


개요

식당 마커를 클릭하면 하단에 미리보기 모달이 표시됩니다.


내용

일단 기능 구현에 집중했기 때문에 놓친 부분이 많을 수 있습니다.

리뷰하면서도 고쳐나가고, 추후에도 리팩토링이 필요합니다.


비고

restaurant_preview.mp4

리뷰어한테 할 말

로컬에서 실행시켜보시고 잘 작동하는지 확인해주세요!

Comment on lines +82 to +84
const { updateMap } = useMapStore((state) => state);
const { meetLocation } = useMeetLocationStore((state) => state);
const { updateSelectedRestaurantData } = useSelectedRestaurantDataStore((state) => state);
Copy link
Member

Choose a reason for hiding this comment

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

궁금한게 Map 을 전역 상태로 하나 더 생성하셨더라구요. 그러면 컴포넌트에 굳이 mapRef를 따로 생성하는게 의미가 있는지 궁금합니다.

Copy link
Member Author

Choose a reason for hiding this comment

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

말씀하신대로 불필요해서 수정해야 할 부분입니다. 일단 기능 구현이 목적이었기 때문에 useNaverMaps훅 내부에 있는 로직을 MainMap 안에 다시 옮기는 작업을 하지 않았습니다. 😓

Comment on lines 223 to 258
<MapControlBox>
<button
type="button"
onClick={() => {
if (!map || !meetLocation) {
return;
}

map.setCenter(meetLocation);
}}
>
<PointCircleIcon />
</button>
<button
type="button"
onClick={async () => {
const socket = socketRef.current;

if (!(socket instanceof Socket)) {
return;
}

const location = await getCurrentLocation();
socket.emit('changeMyLocation', { userLat: location.lat, userLng: location.lng });
updateUserLocation(location);

if (!map) {
return;
}

map.setCenter(location);
}}
>
<GpsIcon />
</button>
</MapControlBox>
Copy link
Member

Choose a reason for hiding this comment

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

얘를 별도 컴포넌트로 분리하는게 좋을 것 같습니다.

Copy link
Member

@CodeDiary18 CodeDiary18 left a comment

Choose a reason for hiding this comment

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

확인 완료했습니다!

@YuriKwon
Copy link
Member

훨씬 깔끔하네요!!

@junghyunbak junghyunbak merged commit 98ea9bf into dev Dec 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants