Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat: 레스토랑 카드 및 마커 클릭 이벤트 변경 #198
feat: 레스토랑 카드 및 마커 클릭 이벤트 변경 #198
Changes from 11 commits
462bbb7
161d6fe
0838f20
d09af48
11cf5f8
8748b91
11ba021
43ca047
fdeb97a
f731c1d
b4ad157
d9bd80d
dbb297f
a6ae8ef
5b69f96
3df8276
c3a5c15
e8e9e31
ab9e38d
9141668
34b12a6
750fe7f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
혹시 center 상태로는 이 mainPosition 의 상태를 대체하기는 힘들까요? (제가 개인적으로 mainPosition을 center로 바꾸어 보았는데 실패하긴 했습니다.🥹)
무언가.. 중복되는 상태가 두개 있는 느낌이라서 하나로 만들고픈 욕구가 샘솟네요..
그리고! 타입을 좁혀주면 더 좋을 것 같아요!
Check warning on line 40 in frontend/src/components/@common/Map/Map.tsx
GitHub Actions / 🍔테스트 딱 대라 💢👊
Check warning on line 44 in frontend/src/components/@common/Map/Map.tsx
GitHub Actions / 🍔테스트 딱 대라 💢👊
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.
quadrant가 사분면을 뜻하는 군요 ㅋㅋㅋㅋ
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.
슬슬... zIndex단이 많이 나오는 것 같네요..!
지금 할일이 아닐 수 있겠지만, 나중에라도 다같이 zIndex 단을 따로 정해보는게 좋을 것 같아요!
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.
요 애니메이션은 많이 쓰일거 같네용 ~~ 공통 css로 빼도 좋을 거 같아여!!
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.
오홍~! 깔끔하네요! 옛날 수학 공부하는 생각이...ㅎㅎ
이것도... 함수 추상화 측면적인 이야기이긴 한데... 아직 한 군데에서밖에 안쓰여서.. 일단 남겨보겠습니다! 반영은 원하신다면 해주세요! 지금도 괜찮다고 생각해요!
함수 네이밍이
getQuadrant
이고 인자가 중앙과 타겟이 있는거를 보아하니, 중앙에서 타겟이 어느 분면에 있는지 파악하는 함수라고 바로 와 닿았어요! 이 사분면을 구분하는 두 개의 인자가 지금은 위도, 경도인Coordinate
로 타입이 좁혀져 있는데, 그냥 위도 경도가 아니더라도 숫자쌍으로도 받을 수 있게 타입을 확장해도 좋겠다고 생각했어요!그렇지만 아마도..? 제레미의 원래 노림수대로 아직 쓰이는 곳이 위도, 경도에 종속되어 있으니 만약! 나중에 다른 곳에서도 사용한다면 그 때 확장하는 것도 찬성이에요~!