PopUpRegisterViewControler 분리 #106
Conversation
There was a problem hiding this comment.
분리를 했는데도 많이 아직도 많네요 🥲
이번에 리팩토링때 클린아키텍처 구조로 더 분리를 해보죠 👍🏻
There was a problem hiding this comment.
스타일 맞추는거 너무 좋습니다👍🏻
다만 View에서 then을 안쓴곳도 보이는데, then을 사용하는쪽으로 통일하시면 좋겠습니다!
dongglehada
left a comment
dongglehada
left a comment
There was a problem hiding this comment.
작업량이 많으셨을 것 같은데 수고 하셨습니다!! tf, lb 같은 줄여쓴 변수명이 있는데 해당 부분의 확인이 필요할 것 같습니다!
|
|
||
| final class PopUpImagesCollectionView: UICollectionView { | ||
| // MARK: - Properties | ||
| enum Constant { |
There was a problem hiding this comment.
저희가 맞추기로 하였던 상수값을 빼는 부분을 적용해 주셨군요 좋습니다 :>
|
|
||
| var onImageSelected: ((Int) -> Void)? | ||
| var onMainImageToggled: ((Int) -> Void)? | ||
| var onImageDeleted: ((Int) -> Void)? |
There was a problem hiding this comment.
Rx가 아니라 클로저로 스트림을 연결한 이유가 따로 있을까요?
There was a problem hiding this comment.
단순이벤트라 판단되어 연산자체인에 오버헤드인지 .. 빌드에서 무한빌드가 되는케이스가 있었습니다
우선적으로 빌드확인을위해 빼뒀었지만 더나은 방법이 있다면 고려해보고 수정해도 좋을것 같습니다
매서운 리뷰 ..
| @@ -1,110 +1,898 @@ | |||
| import UIKit | |||
|
|
|||
| import CoreLocation | |||
There was a problem hiding this comment.
CoreLoacation은 퍼스트 파티인 것으로 알고 있는데 띄워쓰기를 하지 않아도 괜찮지 않을까요? @0Hooni @zzangzzangguy 의견도 궁금합니다!
There was a problem hiding this comment.
스트림이 길어지는 부분이 있어서 제가 내용을 정확히 확인하는 것에는 조금 난이도가 있습니다 ㅎㅎ. ㅠ 해당 작업 내용은 다시 한 번 확인해 보겠습니다!!
|
|
||
| // 유틸리티 메서드 | ||
| func makeRoundedTextField(_ placeholder: String) -> UITextField { | ||
| let tf = UITextField() |
There was a problem hiding this comment.
tf 로 줄여쓰기!! 이런 부분들 수정이 필요할 것 같습니다 !!
📌 이슈
#102
✅ 작업 사항
PopUpRegisterView파일로 UI 구성 로직 이동DateTimePickerManager생성 및 적용PopUpImagesCollectionView로 이미지 피커·컬렉션뷰 로직 이동PopUpStoreRegisterReactor로 Action/Mutation/State 분리, 네트워크·유효성·에러 처리 담당UITextField패딩,UIView.findFirstResponder()등 extension으로 분리disposeBag네이밍,self.표기, 프로토콜 extension 위치, 메서드 순서 정리🚀 테스트 방식
👀 ETC
UICalendarView기반 인라인 캘린더 도입 검토