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.

Sign up for GitHub

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

Jump to bottom

[Feat] #400 - 솝탬프 신고 기능 및 네트워크 오류 모달 추가 #403

Merged
merged 9 commits into from
Oct 16, 2024
Merged

[Feat] #400 - 솝탬프 신고 기능 및 네트워크 오류 모달 추가 #403

merged 9 commits into from
Oct 16, 2024

Conversation

dlwogus0128
Copy link
Contributor

@dlwogus0128 dlwogus0128 commented Oct 15, 2024 •
edited
Loading

🌴 PR 요약

🌱 작업한 브랜치

🌱 PR Point

  • 솝탬프 신고 버튼을 추가하고, 서버에서 url을 받아와 신고 버튼을 눌렀을 때 사파리뷰를 띄워주고 있습니다.
  • 솝탬프의 네트워크 오류 모달을 추가했습니다.

📌 참고 사항

  • 솝탬프 신고 버튼에서 사용되는 url을 viewWillAppear 시에 받아오도록 구현했는데.. 괜찮은걸까요? 버튼을 눌렀을 때 url를 받아와 신고뷰를 띄워주어야 할 지.. 고민하다가 이렇게 했는데.. 조언이 필요합니다.!
input.viewWillAppear
      .withUnretained(self)
      .sink { owner, _ in
        owner.fetchMissionList(type: input.missionTypeSelected.value)
        owner.useCase.fetchIsActiveGenerationUser()
        owner.useCase.getReportUrl()
      }.store(in: cancelBag)
  • 기존 솝탬프의 네트워크 모달이 솝탬프 디자인에 맞게 .. 테마가 설정되어 있는 것 같더라고요..? 원래는 버튼 색상이 soptampError200 이었는데, 디자인 분들이 넘겨주신 사진은 black60 컬러로 되어 있어서 변경해주었습니다..
  • 금방 할 줄 알았는데 아직 솝탬프 쪽 숙지가 잘 안돼서.. 혼자 삽질을 좀 했어요.. 🥹 (오늘도삽질대마왕)
  • [Fix] #396 - 솝마디 결과뷰 컴포넌트 수정 #398 (comment)
  • 옛날에 윤서 언니한테 리뷰받았던 것 중 웹뷰를 router의 pushSOPTWebView로 사용했으면 좋겠다는 내용이 있었는데, 그 때 router의 루트뷰와 self(DailySoptuneResultVC)의 루트뷰가 달라 적용하지 못했었어요. 이번에 신고 사파리뷰를 띄울 때도 router의 presentSafari를 쓰려고 했는데 같은 문제가 있어서..🤔
  • (세진님한테 질문도 하고.. 혼자 고민을 했는데..) 최상단의 루트뷰컨은 이미 사파리뷰의 부모뷰(솝탬프 뷰)를 present하고 있어서, 더 이상 present할 공간이 없기에 router의 해당 함수를 사용하면 이동되지 않는 것이었고
  • 지금 상황에서는 사파리뷰의 부모뷰(솝탬프 뷰) 네비 컨트롤러를 루트로 해서 present해줘야 함을 알았습니다. 그래서 아래와 같이 self?.rootController?.present(safariViewController, animated: true)로 처리해주었습니다..
missionList.onReportButtonTap = { [weak self] url in
    let safariViewController = SFSafariViewController(url: URL(string: url)!)
    safariViewController.playgroundStyle()
    self?.rootController?.present(safariViewController, animated: true)
}

📸 스크린샷

솝탬프 신고
Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-10-15.at.19.48.27.mp4
솝탬프 네트워크 에러

📮 관련 이슈

@dlwogus0128 dlwogus0128 added Feat 새로운 기능 구현 재현✦ labels Oct 15, 2024
@dlwogus0128 dlwogus0128 requested a review from yungu0010 October 15, 2024 10:59
@dlwogus0128 dlwogus0128 self-assigned this Oct 15, 2024
@height Height
Copy link

height bot commented Oct 15, 2024

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

yungu0010
yungu0010 reviewed Oct 15, 2024
View reviewed changes
...Projects/Features/StampFeature/Sources/MissionListScene/ViewModel/MissionListViewModel.swift Outdated
@@ -57,6 +59,7 @@ extension MissionListViewModel {
.sink { owner, _ in
owner.fetchMissionList(type: input.missionTypeSelected.value)
owner.useCase.fetchIsActiveGenerationUser()
owner.useCase.getReportUrl()
Copy link
Contributor

@yungu0010 yungu0010 Oct 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

viewWillAppear보단 viewDidLoad가 적합한 것 같아요.
viewWillAppear는 다른 화면에서 돌아올 때마다 계속 호출이 되어서 앱을 사용하는 동안 바뀌지 않는 값은 한 번만 호출하면 된다고 생각합니다 !

+) 신고 url이 앱을 실행하는 동안 바뀔일이 없다보니 viewDidLoad보단 앱을 처음 실행할 때 호출해서 ExternalURL에 저장하고 상수를 사용하는 것이 더 좋아보입니다:)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

아!!! 감사합니다 반영해놓을게요!!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

신고 url이 앱을 실행하는 동안 바뀔일이 없다보니 viewDidLoad보단 앱을 처음 실행할 때 호출해서 ExternalURL에 저장하고 상수를 사용하는 것이 더 좋아보입니다:)

혹시 요거 좀만 더 자세히 설명해주실 수 있을까요..? 👀 ExternalURL에 상수로 지정하면, 서버에서 받아오는 값을 저장할 수 있나해서요.... 그리고 앱을 처음 실행할 때면 api 호출 시점을 언제로 잡으면 좋을까요..?! 홈뷰에 진입할 때인가요?

Copy link
Contributor

Choose a reason for hiding this comment

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

헉 죄송합니다 !! ExternalURL이 아니라 UserDefaults가 맞겠네요 !_! 호출 시점은 홈뷰에 진입할 때로 생각했습니다!
딥링크 이동 시 home뷰가 다시 로드되긴 하지만, 솝탬프 진입 시 호출하는 것 보단 아무래도 홈뷰가 더 호출 횟수가 적으리라 생각됩니다..!

yungu0010
yungu0010 reviewed Oct 15, 2024
View reviewed changes
Copy link
Contributor

@yungu0010 yungu0010 left a comment

Choose a reason for hiding this comment

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

저도 Router에 대한 이해가 부족했던 것 같습니다😭

그러면 SoptuneResultVC에서 콕 찌르기 버튼을 클릭했을 때 나오는 bottomSheet 역시 present 방식인데, 이때도 SoptuneResultVC의 부모뷰가 아니라 SoptuneReusltVC 자체를 전달해주는게 맞나요?_?

@dlwogus0128
Copy link
Contributor Author

저도 Router에 대한 이해가 부족했던 것 같습니다😭

그러면 SoptuneResultVC에서 콕 찌르기 버튼을 클릭했을 때 나오는 bottomSheet 역시 present 방식인데, 이때도 SoptuneResultVC의 부모뷰가 아니라 SoptuneReusltVC 자체를 전달해주는게 맞나요?_?

  1. BottomSheetManager의 내부 present 코드를 보면, 부모 뷰가 있을 경우 (현재 상황에서는 SoptuneResultVC) 그 부모 뷰에서 present 되도록 짜여져 있더라구요 (부모뷰가 없을 경우, 최상단 뷰컨에서 present 되도록)
view?.present(viewController, animated: true) ?? UIApplication
            .getMostTopViewController()?
            .present(viewController, animated: true)
  1. 라우터 내부 코드를 보면 이 manager의 present를 활용해서 넘겨주고 있는데
public func showBottomSheet(manager: BottomSheetManager, toPresent: UIViewController, on view: UIViewController?) {
    manager.present(toPresent: toPresent, on: view)
}

이 때문에 웹뷰와 달리, 바텀시트는 router의 showBottomSheet 함수를 사용해도 present 된다고 이해했어요!

  1. 따라서 SoptuneReusltVC 자체를 전달해주는게 맞나요?_? 이 말이 맞다고 생각합니다 👀

@dlwogus0128 dlwogus0128 merged commit 283729a into develop Oct 16, 2024
@dlwogus0128 dlwogus0128 deleted the feat/#400-soptamp-report-and-error-modal branch October 16, 2024 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yungu0010 yungu0010 yungu0010 approved these changes

Assignees

@dlwogus0128 dlwogus0128

Labels
Feat 새로운 기능 구현 size/L 재현✦
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Feat] 솝탬프 신고 기능 및 네트워크 오류 모달 추가
2 participants
@dlwogus0128 @yungu0010