feat: 지도 탭 api 연결 #247

anyukyung · 2023-12-07T18:11:42Z

🧑‍🚀 PR 요약

해당 pr에서 작업한 내역을 적어주세요.

지도 탭에 반경 내 게시물들을 보여주는 api를 연결했습니다.

어노테이션 선택 → 해당 영상으로 스크롤

RPReplay_Final1701972326.MP4

영상 스크롤 → 어노테이션 포커싱

RPReplay_Final1701972434.MP4

현재 어노테이션으로 포커싱은 하고있으나, selection 문제로 인해서 하이라이트는 안되는 상황입니다..
임의로 어노테이션을 select을 하는 순간 로직이 꼬여서 좀 더 공부해서 개선해보겠습니다 !

썸네일 concurrent하게 다운로드

이 과정에서 thumbnailImage의 타입이 Data인 모델이 필요했습니다.
근데 공통으로 쓰고있는 Post모델은 URL 타입이어서 전부 바꿀까 하다가
우선은 MapModels 내부에 Post를 만들어두고 썼습니다.. ! Worker에서 data로 변환해줘서 interactor에 뿌리는게 다 괜찮으시면 나중에 공통 모델을 바꾸어야 할 것 같습니다.

            let posts = try await data.concurrentMap { postDTO -> Models.Post in
                let thumnailImageData = try await self.provider.request(url: postDTO.board.videoThumbnailURL)
                return .init(member: postDTO.member.toDomain(),
                             board: postDTO.board.toDomain(),
                             tag: postDTO.tag,
                             thumnailImageData: thumnailImageData)
            }

Linked Issue

close #226

anyukyung · 2023-12-07T18:31:37Z

앗 ㅠㅠ 또 썸네일 오타났네 고치겠습니당

chopmozzi

고생하셨습니다~~

iOS/Layover/Layover/Scenes/Map/MapModels.swift

chopmozzi · 2023-12-07T19:01:05Z

iOS/Layover/Layover/Scenes/Map/MapInteractor.swift

-        posts = request.videos
-        index = request.index
-        presenter?.presentPlaybackScene()
+    func fetchPosts() -> Task<[MapModels.Post], Never> {


보니까 받는 데서는 결과를 쓰지 않는 모양인 것 같은데 그냥 반환 성공하면 true리턴?이 낫지 않나요

추후에 Task 리턴을 들어내야하긴 하지만..?
테스트하는 입장에서 생각해보면 빈배열보다는 지웅님 말씀처럼 불리언 타입이 나을 것 같네용..
이참에 그냥 없애버릴까요..

Test를 위해 Bool을 쓰긴 하는거니까요... 먼가 애매애매하군요.. 고민이 필요해 보입니다..!

멘토님 말씀처럼 정답이 없다고 하셔가지고
-> 박터지는 싸움의 현장(어떻게 Task 블록으로만 감싸진 메서드를 테스트할 것인가)
논의가 필요할 것 같습니다.

저는 개인적으로 멘토님이 처음 말씀해주신 의견에 동의하긴 합니다 !
어쨌든 테스트로 인해서 프로덕트 코드가 변경되어야한다는 점이 최대 단점같아서요. 여러분 생각도 궁금하네용

이건 추후 리팩토링 할 떄 의제로 남겨놓을게요 ADR ADR...

iOS/Layover/Layover/Scenes/Playback/PlaybackWorker.swift

first, Last를 통한 바인딩으로 개선

iOS/Layover/Layover/Scenes/Map/MapViewController.swift

loinsir · 2023-12-08T13:54:37Z

iOS/Layover/Layover/Scenes/Map/MapRouter.swift

        destination.parentView = .other
-        destination.index = source.index
+        destination.index = source.postPlayStartIndex
        destination.posts = source.posts


지난번 얘기한 대로 datapass 메서드로 분리되었으면 좋겠습니다

저희 datapass 메소드 분리하나요? 👀

엇 저희 지난번에 요 내용 토대로 분리하기로 합의한 게 아니었나요...?

합의요? 따로 의사결정을 한 기억은 없는데.. 일단 알겠습니다~ 나중에 저도 반영하도록 할게요

datapass 분리 (inout 사용) vs 그냥 참조해서 변경 두가지 의견이 있었던걸로 기억합니다 !!
둘중에 어떤 방법으로 할지 정확히 정하진 않은 것 같은데 (기억이 가물가물..ㅎ) 지금 정하면 좋을 듯 ?

처음 Clean Swift의 Router의 의도가 데이터 전달 과 네비게이팅 분리라는 관점에서 저는 datapass분리도 괜찮다고 생각해요~ inout을 쓰면 순수함수를 잘 못지키는 것 같아 별로 쓰고 싶지 않았을 뿐, Clean Swift의 의도에 따라가려면 datapass가 더 적합한것 같습니다.

이전에 단순 참조해서 변경한 코드들이 있어서, 저도 datapass 메소드 분리하는 방향으로 수정해보겠습니다 :D

아 이전에 https://clean-swift.com/pass-data-backward-more-elegantly-without-using-delegation/
요 내용 유경님께서 공유해주셨을때, 이견이 없으셔서 전 이렇게 하기로 합의한 것으로 받아들였었습니다...!

아 저거는 메소드 보다는 상위 뷰컨트롤러 찾아서 passing하는 방법을 보고 공유했던거라 ㅋㅋㅋㅋ 밑에 메소드를 제대로 안봤었네용
그치만 datapass 메소드를 분리하는 방법이 의도가 명확해서 그렇게 해도 좋을 것 같습니다!

iOS/Layover/Layover/Scenes/Map/MapViewController.swift

loinsir · 2023-12-08T13:59:10Z

iOS/Layover/Layover/Scenes/Map/MapWorker.swift

+            let posts = try await data.concurrentMap { postDTO -> Models.Post in
+                let thumbnailImageData = try await self.provider.request(url: postDTO.board.videoThumbnailURL)
+                return .init(member: postDTO.member.toDomain(),
+                             board: postDTO.board.toDomain(),
+                             tags: postDTO.tag,
+                             thumbnailImageData: thumbnailImageData)
+            }


우선 Model내에 선언해주신것 좋은 것 같습니다.
저도 처음엔 TaskGroup을 몰라서 concurrentMap으로 하긴 했는데, TaskGroup으로 만드는게 장기적으로 더 좋을 것 같습니다.
리팩토링 할 때 함께 의논해봐요. 아니면 도메인 모델쪽에 따로 빼서 선언해주셔도 좋을 것 같습니다.
명명은 ThumbnailLoadedPost...?

모델은 우선 인환님이 추천해주신 ThumbnailLoadedPost로 선언해두고 나머지 interactor들도 ThumbnailLoadedPost 사용하는 것으로 변경되면 그때 Post로 리네임해도 되겠네요 좋습니다 👍

또 서버에 문의해본 결과 post의 배열이 지금 따로 유의미하게 정렬되어서 오진 않아서,
말씀하신대로 TaskGroup으로 충분히 개선해볼 수 있을 것 같습니다 !

loinsir

고생하셨습니다. 오타? 만 수정하심 될 것 같슴다!!!

anyukyung added 5 commits December 7, 2023 14:31

✨ 위도경도 기반 fetchPosts 구현

5b34e7a

♻️ fetchPosts 메서드 분리

eea935d

✨ 스크롤과 맵 핀 포커싱 기능 구현

6653a8f

🐛 어노테이션이 deselect되지 않는 버그 픽스, 썸네일 concurrent download로 변경

734d7a2

✨ 셀에 썸네일 뷰 추가, 재생화면 연결

b07d399

anyukyung added 🍎 iOS ✨ feature labels Dec 7, 2023

anyukyung requested review from chopmozzi and loinsir December 7, 2023 18:11

anyukyung self-assigned this Dec 7, 2023

chopmozzi and others added 2 commits December 8, 2023 03:12

Merge remote-tracking branch 'refs/remotes/origin/iOS/feat#226'

58ae6e1

🔥 불필요한 코드 및 주석 삭제

1e9a48c

chopmozzi added 2 commits December 8, 2023 03:50

🔧 playback scene 수정

c98c037

Merge remote-tracking branch 'refs/remotes/origin/iOS/feat#226'

bb0dce4

chopmozzi approved these changes Dec 7, 2023

View reviewed changes

🔧 오탈자 수정

7153470

anyukyung commented Dec 7, 2023

View reviewed changes

iOS/Layover/Layover/Scenes/Playback/PlaybackWorker.swift Outdated Show resolved Hide resolved

chopmozzi and others added 5 commits December 8, 2023 04:52

♻️ 리뷰 사항 반영

4723e92

first, Last를 통한 바인딩으로 개선

Merge remote-tracking branch 'refs/remotes/origin/iOS/feat#226'

a73f4d0

Merge branch 'iOS/dev' into iOS/feat#226

d2068f1

🐛 EditTagVC의 routeToBack 로직 수정

3391f5a

🐛 스크롤에따라 맵핀 하이라이트되도록 수정

68f1618

anyukyung linked an issue Dec 8, 2023 that may be closed by this pull request

feat: 맵핀 하이라이트 개선 #251

Closed

1 task

loinsir reviewed Dec 8, 2023

View reviewed changes

iOS/Layover/Layover/Scenes/Map/MapViewController.swift Outdated Show resolved Hide resolved

loinsir reviewed Dec 8, 2023

View reviewed changes

iOS/Layover/Layover/Scenes/Map/MapViewController.swift Outdated Show resolved Hide resolved

chopmozzi reviewed Dec 8, 2023

View reviewed changes

iOS/Layover/Layover/Scenes/Map/MapViewController.swift Show resolved Hide resolved

loinsir reviewed Dec 8, 2023

View reviewed changes

loinsir approved these changes Dec 8, 2023

View reviewed changes

anyukyung added 2 commits December 9, 2023 00:07

Merge branch 'iOS/dev' into iOS/feat#226

2a3c40c

♻️ 오탈자 수정, 모델 분리, datapass 메소드 추가 등 코드 리뷰 반영

0eb01e5

anyukyung merged commit 6556ef2 into iOS/dev Dec 8, 2023
1 check passed

anyukyung linked an issue Dec 8, 2023 that may be closed by this pull request

feat: 지도 탭 api 연결 #226

Closed

1 task

anyukyung deleted the iOS/feat#226 branch January 10, 2024 13:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: 지도 탭 api 연결 #247

feat: 지도 탭 api 연결 #247

anyukyung commented Dec 7, 2023

anyukyung commented Dec 7, 2023

chopmozzi left a comment

chopmozzi Dec 7, 2023

anyukyung Dec 7, 2023

chopmozzi Dec 7, 2023

loinsir Dec 8, 2023 •

edited

Loading

anyukyung Dec 8, 2023

loinsir Dec 8, 2023 •

edited

Loading

loinsir Dec 8, 2023

chopmozzi Dec 8, 2023

loinsir Dec 8, 2023 •

edited

Loading

chopmozzi Dec 8, 2023

anyukyung Dec 8, 2023

chopmozzi Dec 8, 2023

anyukyung Dec 8, 2023

loinsir Dec 8, 2023 •

edited

Loading

anyukyung Dec 8, 2023 •

edited

Loading

loinsir Dec 8, 2023

anyukyung Dec 8, 2023

anyukyung Dec 8, 2023 •

edited

Loading

loinsir left a comment

feat: 지도 탭 api 연결 #247

feat: 지도 탭 api 연결 #247

Conversation

anyukyung commented Dec 7, 2023

🧑‍🚀 PR 요약

어노테이션 선택 → 해당 영상으로 스크롤

영상 스크롤 → 어노테이션 포커싱

썸네일 concurrent하게 다운로드

Linked Issue

anyukyung commented Dec 7, 2023

chopmozzi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

loinsir Dec 8, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

loinsir Dec 8, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

loinsir Dec 8, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

loinsir Dec 8, 2023 • edited Loading

Choose a reason for hiding this comment

anyukyung Dec 8, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anyukyung Dec 8, 2023 • edited Loading

Choose a reason for hiding this comment

loinsir left a comment

Choose a reason for hiding this comment

loinsir Dec 8, 2023 •

edited

Loading

loinsir Dec 8, 2023 •

edited

Loading

loinsir Dec 8, 2023 •

edited

Loading

loinsir Dec 8, 2023 •

edited

Loading

anyukyung Dec 8, 2023 •

edited

Loading

anyukyung Dec 8, 2023 •

edited

Loading