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

chore: 재생화면 동작 개선 #172

Merged
merged 13 commits into from
Dec 4, 2023
Merged

chore: 재생화면 동작 개선 #172

merged 13 commits into from
Dec 4, 2023

Conversation

chopmozzi
Copy link
Collaborator

🧑‍🚀 PR 요약

  • collectionView cell 사이에 생기는 여백 파악하기
  • 수정하기
  • 셀이 내비바에도 보이도록 하기
  • 무한스크롤 시 텔레포트를 위한 셀은 그냥 동영상 로드 안 해버리기
  • Slider cell에서 collectionView로 빼기

📌 변경 사항

변경 사항은 아닙니다만.
원래 HomeView, MapView에서 넘어올 때 collectionView Delegate로 빼려 했는데(select did item) Home View나 Map View 따로 작업하실 것 같아 뒤로 미루겠습니다.

📸 ScreenShot
제목 내용
Simulator Screenshot - iPhone 13 mini - 2023-12-02 at 14 45 21 Simulator Screenshot - iPhone 13 mini - 2023-12-02 at 20 03 22

UIVisualEffectBackdropView라고 NavigationBar와 TabBar에 뿌옇게 효과가 생겨 해당 부분을 투명하게 만들어 주었습니다.
텔레포트를 위해 더미 셀을 만드는데, 더미 셀에도 동영상 로드를 하고 있어 좀 느린 것 같아 해당 부분은 동영상 로드 안하고 넘어갈 수 있도록 했습니다.

제목 내용
스크린샷 2023-12-02 오후 8 09 25 스크린샷 2023-12-02 오후 8 09 45

기존에는 cell마다 LOSlider가 들어가 셀이 늘어날 수록 Slider가 중첩돼서 생겼는데, 이 문제를 해결하고자 Slider를 CollectionView에서 하나만 쓸 수 있도록 했습니다.

Linked Issue

close #163

@loinsir
Copy link
Collaborator

loinsir commented Dec 2, 2023

UIVisualEffectBackdropView라고 NavigationBar와 TabBar에 뿌옇게 효과가 생겨 해당 부분을 투명하게 만들어 주었습니다.

이건 iOS 15 이후 부터 생긴 appearance 관련된 부분 아닌가요?
해결하려면 해당 부분에 관련된 것을 만져야할 것 같습니다.
https://www.zehye.kr/ios/2021/10/19/iOS_ios15_navigation_bar/

@chopmozzi
Copy link
Collaborator Author

아 branch 위치 실환가

Comment on lines 13 to 16
struct PlaybackVideo: Hashable {
var id: UUID = UUID()
let post: Post
let playbackInfo: PlaybackInfo
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

지웅님 이거 근데 궁금한게 있는데 UUID 넣어주신 이유가 있나요?

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

Choose a reason for hiding this comment

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

아 그러면 이제 아래 PlaybackInfo가 Hashable하니까 이건 안써도 되는...?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

넹넹 리뷰 반영사항에 수정해뒀어요

@chopmozzi
Copy link
Collaborator Author

@loinsir @anyukyung 재생화면의 VIP 싸이클에 변화가 생겼습니다.
cell을 configure하는 부분은 거의 그대로구요.
snapshot apply하는 시점에서 presenter에서 Post를 PlaybackVideo로 변환시켜 viewModel로 건넵니다.
따라서 transVideo를 하는 시점이 router에서 Presenter로 바뀐 부분이 있습니다.
그리고 LOSlider에 playerView를 빼고 duration을 넣으려 했는데 CMTime이슈가 있었구요. 애초에 해당 셀렉터 함수에 playerView를 실행을 때리고 있더라구요. 그래서 로직 고칠겸 다 들어내고 Slider에 playerView동기화 시키는 부분도 VIP 싸이클로 적용했습니다.
그리고 홈화면에서는 fetchPosts인가 하여튼 그 메소드로 비디오를 들어갈 때 로드 해서 HomeView dataStore의 posts에 값을 넣어줘 해당 값을 destination.posts = source.posts로 넣어줬는데, 맵에는 아직 그 동작이 없어 넣지 않았습니다.
그리고 예시 동영상 중에 코알라나오는 동영상은 재생시간이 정해져있지 않아 재생바 터져서 다른 동영상으로 대체했습니다.

@chopmozzi chopmozzi requested a review from loinsir December 3, 2023 17:50
@chopmozzi chopmozzi requested a review from anyukyung December 3, 2023 17:50
@@ -39,7 +39,7 @@ final class HomeRouter: NSObject, HomeRoutingLogic, HomeDataPassing {
else { return }
destination.parentView = .home
destination.index = source.postPlayStartIndex
destination.videos = transData(videos: source.posts ?? [])
destination.posts = source.posts
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

Comment on lines +28 to +30
info.tag.forEach { tag in
playbackView.tagStackView.addTag(tag)
}
Copy link
Collaborator

@loinsir loinsir Dec 4, 2023

Choose a reason for hiding this comment

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

지웅님 이 부분 재사용 이슈는 없나요? 다시 set될때 tag 버튼이 중복으로 들어갈 것 같은데 처음에 초기화 주는 부분이 없는 것 같아서요

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

재사용 이슈 생겨서 stackView remove하는 식으로 해결했습니다. UI에 관련된 것이기 때문에 prepareForReuse안쓰고 셀 초기화 하는 부분에서 해줬어용

try await Task.sleep(nanoseconds: 1_000_000_00)
playerSlider.isHidden = false
} catch {
os_log("Falie Waiting show Player Slider")
Copy link
Collaborator

Choose a reason for hiding this comment

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

"Fail"? 오타 이신건가용?

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

@loinsir loinsir left a comment

Choose a reason for hiding this comment

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

복잡한 로직 구현하시느라 정말 고생하셨습니다~


func getDuration() -> Float64 {
CMTimeGetSeconds(playerView.player?.currentItem?.duration ?? CMTime(value: 0, timescale: 1))
}
}

// MARK: PlaybackView 내부에서만 쓰이는 Method

private extension PlaybackView {
func updateSlider(currentTime: CMTime) {
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.

interactor의 controlPlaybackMovie에서 slider의 위치와 동영상 위치 동기화할 때 쓰고 있습니다.

Comment on lines +126 to +128
guard let tabBarHeight: CGFloat = self.tabBarController?.tabBar.frame.height else {
return
}
Copy link
Member

Choose a reason for hiding this comment

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

왜 .. 왜 얘만 줄바꿈이...?!

func makeInfiniteScroll(posts: [Post]) -> [Post] {
var tempVideos: [Post] = posts
let tempLastVideo: Post = posts[tempVideos.count-1]
let tempFirstVideo: Post = posts[1]
Copy link
Member

Choose a reason for hiding this comment

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

ouf of range 날 가능성은 없..겠죠?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

예외 처리 넣어주도록 하겠습니다. 1개면 나겠네요

Copy link
Member

@anyukyung anyukyung left a comment

Choose a reason for hiding this comment

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

복잡복잡.. 하다보니 저는 이해하기도 힘드네요 헤헿,,
고생하셨습니다..

Conflicts:
	iOS/Layover/Layover/Scenes/Home/HomeRouter.swift
@chopmozzi chopmozzi merged commit 83de941 into iOS/dev Dec 4, 2023
1 check passed
@chopmozzi chopmozzi deleted the feat#163 branch December 4, 2023 09:39
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