-
Notifications
You must be signed in to change notification settings - Fork 0
[Feat/NST-15] 일정생성뷰 작성 #49
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
base: main
Are you sure you want to change the base?
Conversation
FpRaArNkK
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.
코드가 점점 매서워지시는군요... 고생하셨습니다!
AppThemeButton이 기본적으로 .disable로 초기화가 되어있는데, 해당 화면은 처음부터 .able상태여야 하므로 해당 코드를 추가하였습니다. AppThemeButton의 초기화상태에서 update(.disable)을 변경하는건 어떤가요??
이게 더 좋은 방향인 것 같아요! 제가 지금은 볼륨이 큰 캘린더 관련 작업을 진행중이라서 혹시 가능하시다면 해당 부분 수정 반영 부탁드려도 될까요?
| case .toggleCell(let indexPath): | ||
| newState.selectedCells = newState.selectedCells.symmetricDifference([indexPath]) |
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.
와 테이블에서의 Cell 선택/비선택 로직을 symetricDifference로 깔끔하게 해결하셨네요
너무 좋습니다 👍👍👍
| struct AvailableTime: Codable { | ||
| let date: String | ||
| let startTime: String | ||
| let endTime: String | ||
| } |
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.
요건 따로 Entity로 뺴주시면 좋을 것 같아요!
| extension NSTDateUtility { | ||
| ///타임테이블 뷰 : "요일 월/일" | ||
| static func dateList(_ dateStrings: [String]) -> [String] { |
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.
화면에서의 Date 표시를 위해 View에서 extension 하신 것은 좋은 선택인 것 같아요!
하지만 이렇게 될 경우 해당 View가 포함된 프로젝트에서는 어디에서든지 이 코드를 사용할 수 있게 될 거에요.
extension을 AvailabilityCreateView로 바꿔서 뷰 내부 함수로 처리하는 건 어떨까요?
+ Reactor에서 일정 데이터를 관리하는 로직이 여기에 포함되어있는건 아닌지 더블 체크 부탁드려요!
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.
이 함수를 비슷한 화면인 일정조회화면에서도 사용해서.. 고민이네요
| rootView.confirmButton.rx.tap | ||
| .map { AvailabilityCreateReactor.Action.tapConfirmAvailableTimes } | ||
| .bind(to: reactor.action) | ||
| .disposed(by: disposeBag) |
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.
혹시 해당 버튼을 통해 POST API를 호출한다면,
중간에 debounce를 걸어주시면 좋을 것 같습니다!
이미 다른 코드로 바껴있군요 ㅋㅋ |
thingineeer
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.
굿
| struct AvailableTime: Codable { | ||
| let date: String | ||
| let startTime: String | ||
| let endTime: String | ||
| } |
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.
파일 따로 빼도 될 것 같아요!
| [scrollView, confirmButton].forEach { | ||
| self.addSubview($0) | ||
| } | ||
| scrollView.addSubview(contentView) | ||
| [availbilityLabel, schedulePickerView].forEach { | ||
| contentView.addSubview($0) | ||
| } | ||
| } |
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.
요기 제가 만들어둔 addSubviews(view1, view2, view2) 이렇게 사용하는게 좋을 것 같아요!
| let dataSource = RxCollectionViewSectionedReloadDataSource<SectionModel<String, String>>( | ||
| configureCell: { _, collectionView, indexPath, _ in | ||
| guard let cell = collectionView.dequeueReusableCell( | ||
| withReuseIdentifier: SchedulePickerCell.identifier, for: indexPath |
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.
SchedulePickerCell.swift 파일에 있는 identifier 속성 지우고 SchedulePickerCell.className 사용하시면 됩니다!
ISSUE
close : #42
변경사항
일단 해당 화면에서 쓰이는 셀 색상 바꾸는 로직을 bind()로 옮겼습니다.
PR Point