[Feat] #130 - 마이페이지 러닝 기록 UI 구현 #135
Conversation
lsj8706
left a comment
lsj8706
left a comment
There was a problem hiding this comment.
수고하셨습니다~~!
쉽지 않은 UI인데 잘 작업해주셨네요! 👍
| private let secondHorizontalDivideLine = UIView() | ||
|
|
||
| private lazy var recordDistanceLabel = setGreyTitle().then { | ||
| $0.text = "거리" |
There was a problem hiding this comment.
서버 통신까지 연결하면 이런 디폴트 텍스트들은 다 지워주세요~!
| return stackView | ||
| } | ||
|
|
||
| func setBlackTitle() -> UILabel { |
There was a problem hiding this comment.
함수 네이밍에 대해 한번 고민 해봅시다!
이 함수 이름을 처음 읽는 사람 입장에서 setBlackTitle은 Label, Button 중에서 어떤 UI를 Black으로 만든다는 건지 알기 어려울 것 같아요
makeBlackTitleLabel과 같이 더 상세한 네이밍이 좋을 것 같습니다~!
| do { | ||
| let responseDto = try result.map(BaseResponse<ActivityRecordInfoDto>.self) | ||
| guard let data = responseDto.data else { return } | ||
| //self.setData(activityRecordList: data.records) |
| self.emptyView.delegate = self | ||
| } | ||
|
|
||
| private func register() { |
There was a problem hiding this comment.
이 함수 네이밍도 그냥 register 보다는 registerCell 처럼 좀 더 구체적으로 만들면 좋을 것 같아요!
| if let selectedRows = activityRecordTableView.indexPathsForSelectedRows { | ||
| for indexPath in selectedRows { | ||
| activityRecordTableView.deselectRow(at: indexPath, animated: true) | ||
| } | ||
| } |
There was a problem hiding this comment.
이런 작업들은 따로 함수로 빼서 만들어봅시다!
| self.deleteRecordButton.isEnabled = false | ||
| self.deleteRecordButton.setTitle(title: "삭제하기") | ||
| self.activityRecordTableView.reloadData() | ||
| isEditMode = false |
There was a problem hiding this comment.
isEditMode.toggle()을 else문 끝나고 넣으면 121줄도 없애고 한줄로 처리할 수 있을 것 같아요!
| if isEditMode { | ||
| // 선택된 셀에 대한 표시 업데이트 | ||
| if let selectedRecords = tableView.indexPathsForSelectedRows, selectedRecords.contains(indexPath) { | ||
| activityRecordCell.activityRecordContainerView.image = ImageLiterals.imgRecordContainerSelected | ||
| } else { | ||
| activityRecordCell.activityRecordContainerView.image = ImageLiterals.imgRecordContainer | ||
| } | ||
|
|
||
| } else { | ||
| activityRecordCell.selectionStyle = .none | ||
| // 선택된 셀들을 순회하면서 미선택 이미지로 변경 | ||
| for indexPath in tableView.indexPathsForSelectedRows ?? [] { | ||
| guard let cell = tableView.cellForRow(at: indexPath) as? ActivityRecordInfoTVC else { continue } | ||
| cell.activityRecordContainerView.image = ImageLiterals.imgRecordContainer | ||
| } | ||
| } |
There was a problem hiding this comment.
이렇게 VC에서 Cell의 선택/비선택 UI를 처리하는 방법은 물론 동작은 하지만 Cell 자신의 역할을 VC가 대신 하고 있는 것이기 때문에 SRP에서 조금 벗어난다고 생각합니다! (+ Cell과 VC의 결합도가 증가)
그리고 cellForRowAt은 여러번 실행되는 함수인데 여기서 selectedRecords.contains(indexPath) 과 같이 내부적으로 반복문이 도는 코드를 사용하면 나중에 selectedRecords의 크기가 커졌을 때 성능저하로 이어질 수 있습니다.
이후에 cell 내부적으로 isSelected 여부에 따라 UI가 바뀌도록 하는 방향을 한번 고민해보세요~!!
|
일단 머지하고.. 반영할게욥 리뷰 감사합니다... |
|
진짜 잘해따 ,,,, 어케했어요,, 짱이다!! |
3 participants
🌱 작업한 내용
🌱 PR Point
📸 스크린샷
생략
📮 관련 이슈