-
Notifications
You must be signed in to change notification settings - Fork 0
[REFACTOR] 맵뷰컨트롤러 분리 #118
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] 맵뷰컨트롤러 분리 #118
Conversation
0Hooni
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.
질문 좀 남겨놨습니다!
확인 후 Approve 남기도록 하겠습니다
| fastlane/Preview.html | ||
| fastlane/screenshots/**/*.png | ||
| fastlane/test_output | ||
| Poppool/Poppool/Infrastructure/*.mobileprovision |
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.
프로비저닝은 한번 설치되면 Xcode에 남으니 파일이랑 ignore도 지우셔도 될거같아요!
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.
테스트 했던거 그대로 남아있어요!!
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.
DTO 제거 #117 에서 수정되었습니다
| var mainView: MapView { get } | ||
| var currentMarker: NMFMarker? { get set } | ||
| var currentStores: [MapPopUpStore] { get set } | ||
| var currentCarouselStores: [MapPopUpStore] { get set } | ||
| var isMovingToMarker: Bool { get set } | ||
| var currentTooltipView: UIView? { get set } | ||
| var currentTooltipStores: [MapPopUpStore] { get set } | ||
| var currentTooltipCoordinate: NMGLatLng? { get set } | ||
| var individualMarkerDictionary: [Int64: NMFMarker] { get set } | ||
| var clusterMarkerDictionary: [String: NMFMarker] { get set } | ||
| var clusteringManager: ClusteringManager { get } |
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.
이거 안쓰시는 파일이기도 하고 파일명도 좀 이상한것 같아요...!!
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.
이 친구도 MapInteractionHandling과 같은 리뷰입니다
| import ReactorKit | ||
| import UIKit | ||
|
|
||
| extension MapViewController { |
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.
혹시 프로토콜 채택을 다른곳에서 하셨나요? 🤔
| import NMapsMap | ||
| import UIKit | ||
|
|
||
| protocol MarkerStyling { |
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.
프로토콜이니까 MarkerStyling보단 MarkerStylable같은 형용 형태가 좋다 생각합니다...!!
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.
구조체 두개는 단순 프로토콜 채택 말고는 해당 파일에 둘 이유가 없어보여요!! 혹시 이유가 있을까요??
저라면 두 개의 구조체는 파일을 분리해서 둘 것 같아요!
|
맵뷰컨트롤러 책임분리하면서 리액터의 정리 이전 단순 제스처 들 먼저 직관적으로 알기쉽게 정리해두자 라는 생각에 |
📌 이슈
✅ 작업 사항
맵뷰컨트롤러의 핸들러 제스처들을 익스텐션 파일로 분리 프로토콜 생성
분리과정에서 파생된 마커 제스처 버그 수정