Skip to content
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

feat: 재생화면 VIP 싸이클 적용 #146

Closed
wants to merge 37 commits into from
Closed

Conversation

chopmozzi
Copy link
Collaborator

@chopmozzi chopmozzi commented Nov 29, 2023

🧑‍🚀 PR 요약

  • 재생화면의 VIP 싸이클 적용
  • 홈화면과 그 외 화면들 무한 스크롤 다르게 적용
  • MemberDTO와 VideoDTO 생성

스크린샷 2023-11-30 오전 10 47 56

스크린샷 2023-11-30 오전 10 53 03

스크린샷 2023-11-30 오전 11 08 57

스크린샷 2023-11-30 오전 11 17 09

보충 설명을 하자면 VC에서는 presenter로 전달받은 viewModel의 값들을 stopPlayer를 통해 멈추고 playPlayer를 통해 실행시키거나 합니다. playbackCollectionView의 setContentOff를 통해 viewModel값에 있는 indexPath만큼 이동시키거나 합니다.

📌 변경 사항

홈화면과 지도화면에서 재생화면으로 넘어온다고 각 Scene의 코드를 건드린 부분이 있습니다.
잘못건드렸거나 의도에 맞지 않게 수정되었다면 얘기해주시면 얼른 고치겠습니다.
그리고 홈화면과 그 외 화면들 무한 스크롤 방식 차이에 대해서 이야기 하고 싶습니다.
홈화면의 경우 아래로 이동(위로 스와이프) 시 비디오를 로드하는 동작만 넣고 처음 셀로는 이동 안하면 UX적으로 좋을 것 같습니다. 새로고침 하는 느낌?

그 외 화면들은 목록 다 불러와 놓고 한바퀴 도는 형식이 되겠네요

📸 ScreenShot
VIP.mov
VIP.mov

Linked Issue

close #122

Conflicts:
	iOS/Layover/Layover/Scenes/Home/HomeViewController.swift
Comment on lines 36 to 51
enum MoveToPlaybackScene {
struct Request {
let index: Int
let videos: [VideoDTO]
}

struct Response {
let index: Int
let videos: [VideoDTO]
}

struct ViewModel {
let index: Int
let videos: [VideoDTO]
}
}
Copy link
Collaborator

@loinsir loinsir Nov 29, 2023

Choose a reason for hiding this comment

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

CleanSwift도 클린 아키텍처의 영향을 받은 만큼 DTO와 도메인 모델은 분리해야 한다고 생각합니다!

Comment on lines 39 to 49
private func transDTO(videos: [VideoDTO]) -> [PlaybackModels.Board] {
videos.map { videoDTO in
return PlaybackModels.Board(
title: videoDTO.title,
content: videoDTO.content,
tags: videoDTO.tags,
sdUrl: videoDTO.sdURL,
hdURL: videoDTO.hdURL,
memeber: videoDTO.member)
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Router가 수행해도 되는 로직일까요?

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 +124 to +127
func addLoopingViewGesture() {
let loopingViewGesture: UITapGestureRecognizer = UITapGestureRecognizer(target: self, action: #selector(moveToPlaybackScene))
loopingPlayerView.addGestureRecognizer(loopingViewGesture)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

밖에서 collectionView(_:didSelectItemAt:)을 안쓰시고 gestureRecognizer를 사용하신 이유가 있을까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

아... 그런... 방법이.... 오마이갓


var videos: [VideoDTO]?

var index: Int?
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 index는 무슨용도인가요?

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
Member

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.

아니오 지금 당장은 홈이나 지도에서 넘어왔을 때의 포커싱입니다.

import Foundation

final class PlaybackConfigurator: Configurator {
typealias ViewController = PlaybackViewController
Copy link
Collaborator

Choose a reason for hiding this comment

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

아래 configure 인자 타입으로 이미 지정하셔서 이 라인 제거해도 될 것 같습니다.

Comment on lines 45 to 56
// Interactor가 해줄 역할? 고민 필요
private func transDTO(videos: [VideoDTO]) -> [PlaybackModels.Board] {
videos.map { videoDTO in
return PlaybackModels.Board(
title: videoDTO.title,
content: videoDTO.content,
tags: videoDTO.tags,
sdUrl: videoDTO.sdURL,
hdURL: videoDTO.hdURL,
memeber: videoDTO.member)
}
}
Copy link
Member

Choose a reason for hiding this comment

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

DTO 내부에 매핑 메소드를 만들어서 worker나 interactor에서 매핑해서 source datastore에 저장해두는 건 어떨까욤
지웅님이 짜신 구조상 가능하다면요!

Copy link
Collaborator

Choose a reason for hiding this comment

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

@chopmozzi @anyukyung 제가 태그 선택 뷰를 만들면서 이 예시를 구현해놓았는데 참고 부탁드립니다 #149

@loinsir
Copy link
Collaborator

loinsir commented Nov 30, 2023

그려주신 그림 잘 보았습니다. 그런데 왜 이전에 재생되고 있는 셀의 인덱스를 interactor에 저장시켜야 하는지 이해가 되질 않습니다. ㅠㅠ

@chopmozzi
Copy link
Collaborator Author

@loinsir 스크롤이 실행되었을 때 이전 셀을 멈춰야 하는데, scrollViewDidEndDecelerating를 사용하고 cell의 화면에 하나만 들어오는 특성상 이전 셀과 현재 셀이 동일 셀인지, 텔레포트 후 이전 셀이 무엇인지 확인하기가 어려워 interactor에서 프로퍼티로 저장해두고 있습니다.

@loinsir
Copy link
Collaborator

loinsir commented Nov 30, 2023

@chopmozzi 그러면 interactor가 view에 대해 알게 되는 느낌인 것 같습니다.
VIP 패턴 특성상, UI 계층으로부터 분리되어야 하는 Interactor는 절대 UIKit이 import 되면 안된다고 생각합니다.
scrollViewDidEndDecelerating 외에 화면에서 cell이 사라질때 호출되는 collectionView(_:didEndDisplaying:forItemAt:) 메서드를 사용하면 충분히 멈춤 동작은 구현할 수 있지 않을까 하는데 어떠할까요?

@chopmozzi
Copy link
Collaborator Author

@anyukyung @loinsir 일단 모델 받는 부분만 수정했습니다. DTO로 처리 안하고 만들어두신 Post를 사용해서 아마 DTO -> Domain과정은 다른 곳에서 일어날거 같습니다. 제가 이거 고치면서 느낀건데 Pr너무 거대해지고 뭐 바뀌었는지 알아보기도 힘들 것 같아서 이슈 따로 파서 진행하겠습니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants