Skip to content

Conversation

@lee-yeonwoo
Copy link
Collaborator

@lee-yeonwoo lee-yeonwoo commented May 10, 2023

🌱 작업한 내용

  • 보관함 편집 기능 구현

🌱 PR Point

5/18 피알 포인트

  • 삭제하기 올라오고, 탭바 내려가는 애니메이션... 뭔가 이상하네요 .. 어쩔 땐 되고 어쩔땐 혼자 막 이상하게 올라가고..
  • 배너도... 무한 스크롤을 구현을 하는데 부자연스러운 것 같아서 고쳐야 할 것 같아요 ..

<구 피알포인트>

  • 보관함 뷰가 .. combine함수 로 되어 있잖아요 ,,?! 알람팝업창 연결도 복잡하고, delete함수 연결하는 것도 어렵네요 ㅜㅠㅠ 다음 이슈에서 해결하던지 해야 할 것 같아요 ...
  • 위의 부분 때문에 주석처리가 좀 많습니다 ㅠㅠ

📸 스크린샷

Simulator Screen Recording - iPhone SE (3rd generation) - 2023-05-10 at 22 40 26

📮 관련 이슈

@lee-yeonwoo lee-yeonwoo self-assigned this May 10, 2023
@lee-yeonwoo lee-yeonwoo added Feat 새로운 기능 구현 연우🐰 labels May 10, 2023
Copy link
Collaborator

@lsj8706 lsj8706 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 기능 구현은 Combine과 사실상 무관해요..!
PrivateCourseListView에서 didSelectItemAt만 아래처럼 수정하면 될 것 같아요!

func collectionView(_ collectionView: UICollectionView, didSelectItemAt indexPath: IndexPath) {
  if !isEditMode {
       cellDidTapped.send(indexPath.item)
  }
}

기존 코드에 Combine이 부분 적용되어 있지만 이 기능과는 독립적인 것으로 보이니 무시하고 하던대로 작업하시면 됩니다. Protocol-Delegate 패턴의 전형적인 사례이니 제대로 공부해서 적용해봅시다~! 재현이와 같이 온라인으로라도 상의하면서 작업하면 좋을 것 같아요!


private let scrapProvider = Providers.scrapProvider

private var courseId: Int?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 변수는 삭제할 코스의 Id를 담고 있는 걸까요..?
그렇다면 이 상태에서는 코스를 1개만 삭제할 수 있을 거 같은데 저희가 원하는건 여러개를 동시에 삭제할 수 있는 기능 같아요!
변수 명도 courseId라고 하면 너무 추상적이니까 courseIdsToDelete 이런식으로 구체적으로 적어주세요

private var courseIdsToDelete: [Int] = []이런식으로 어레이로 만들어서 여러개를 한번에 지울 수 있도록 할 것 같아요

Comment on lines 27 to 33
private var selectedIndex: Int? {
didSet {
if selectedIndex == nil {
deleteCourseButton.setEnabled(false)
}
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

마찬가지로 이러면 하나의 cell만 선택가능 할 것 같아요 어레이로 선택된 Cell의 인덱스들을 관리해야할 것 같습니다

Comment on lines 11 to 13
// protocol PrivateCourseListViewDelegate: AnyObject {
// func deleteCourseButtonDidTap()
//}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

protocol-delegate 패턴을 이용해 상위 VC인 CourseStorageVC에 선택된 코스들의 id 어레이를 넘겨주는 방식으로 해야할 것 같습니다.

Suggested change
// protocol PrivateCourseListViewDelegate: AnyObject {
// func deleteCourseButtonDidTap()
//}
protocol PrivateCourseListViewDelegate: AnyObject {
func deleteCourseButtonDidTap(courseIdsToDelete: [Int])
}

let deleteVC = RNAlertVC(description: "코스를 정말로 삭제하시겠어요?")
deleteVC.rightButtonTapAction = { [weak self] in
deleteVC.dismiss(animated: false)
// self?.deleteCourse()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기서는 앞서 protocol-delegate 패턴으로 만들어둔 함수를 실행시켜서 상위뷰인 CourseStorageVC에 선택된 코스들의 id들을 넘겨줘야 해요.
위에 단 리뷰가 반영되었다는 가정하에

 delegate?.deleteCourseButtonDidTap(courseIdsToDelete: 선택된 코스 아이디 어레이)

이렇게 되겠네요

Copy link
Collaborator

@lsj8706 lsj8706 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

컨플릭트 해결해주세요...!!!

Comment on lines +62 to +69
// 제스처 인식기 추가
let swipeLeftGesture = UISwipeGestureRecognizer(target: self, action: #selector(handleSwipeGesture(_:)))
swipeLeftGesture.direction = .left
bannerCollectionView.addGestureRecognizer(swipeLeftGesture)

let swipeRightGesture = UISwipeGestureRecognizer(target: self, action: #selector(handleSwipeGesture(_:)))
swipeRightGesture.direction = .right
bannerCollectionView.addGestureRecognizer(swipeRightGesture)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

콜렉션뷰로 배너를 만들었는데 또 UISwipeGestureRecognizer를 추가한 이유가 있을까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

자동으로 넘어가는 배너인지 모르고 수동으로 하려다가.. 뭔가 미는 제스쳐를 추가해야 할 것 같아서 했는데 배너는 자동으로 넘어가게 다 수정해야합니다

}
}

private func hiddenViews(withDuration: TimeInterval = 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

함수명이 애매하네요! hideHiddenViews를 의도하신 걸까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

맞아요..바꿨어요 ..

Comment on lines 169 to 172
<<<<<<< HEAD
view.addSubviews(naviBar, viewPager, deleteCourseButton)
view.bringSubviewToFront(viewPager)
=======
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

컨플릭트들 해결 안하고 커밋하신거 같은데 항상 해결하고 빌드도 해서 잘 실행되는거 확인하고 푸시해야돼요..!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ㅠㅠ넵

Copy link
Collaborator

@lsj8706 lsj8706 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코멘트 확인해주세요!

Comment on lines 84 to 89
@discardableResult
func setButtonTitle(_ leftButtonTitle: String, _ rightButtonTitle: String) -> Self {
self.yesButton.setTitle(rightButtonTitle, for: .normal)
self.noButton.setTitle(leftButtonTitle, for: .normal)
return self
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이거는 원래 없던걸 지운걸까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

넵 ! delegate부분 아마 다 안쓰는 거였어서 지웠습니다

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이거 안쓰나요..? 저 쓴 코드 있는디..!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이거는 delegate랑은 관련 없고 그냥 버튼 타이틀 텍스트 바꾸는 코드 같아요

Comment on lines 489 to 495
<<<<<<< HEAD
do {
let responseDto = try result.map(BaseResponse<ActivityRecordInfoDto>.self)
} catch {
print(error.localizedDescription)
}
=======
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기 컨플릭트 남아있는거 같아요

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아이거 없애도 계속 생기네..용..?

}

@objc func deleteRecordButtonDidTap() {
let deleteAlertVC = RNAlertVC(description: "러닝 기록을 정말로 삭제하시겠어요?").setButtonTitle("취소", "삭제하기")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기에 .setButtonTitle 다시 살려주세요...!!!!

// 수정 모드일 때
self.setEditMode()
})
let deleteAlertVC = RNAlertVC(description: "러닝 기록을 정말로 삭제하시겠어요?").setButtonTitle("취소", "삭제하기")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이것도 다시 살려주세요~~

Copy link
Collaborator

@lsj8706 lsj8706 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수고 많으셨습니다~~!!

@lee-yeonwoo lee-yeonwoo merged commit d9ad297 into Runnect:develop May 19, 2023
@lee-yeonwoo lee-yeonwoo deleted the #134-보관함편집기능구현 branch May 19, 2023 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feat 새로운 기능 구현 연우🐰

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[Feat] 보관함 편집 기능 구현

4 participants