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

[Fix] #395 - 딥링크 변경 사항 반영 및 baseURL trailing slash 수정 #397

Merged
merged 7 commits into from
Oct 3, 2024

Conversation

yungu0010
Copy link
Contributor

🌴 PR 요약

🌱 작업한 브랜치

🌱 PR Point

  • 딥링크 path 변경 사항에 맞게 코드를 수정했어요.
  • baseURL에 trailing slash가 생기는 오류를 해결했어요.
  • 솝마디 뷰 레이아웃을 수정했어요.

📌 참고 사항

딥링크 코드 수정

  • ApplicationCoordinator에서 SoptuneResultCoordinator를 직접적으로 호출할 일이 없어 코드를 삭제했어요.

baseURL trailing slash 오류 해결

image
  • BaseService 객체에서 url을 만들 때 발생한 오류로 사진과 같이 마지막 path가 empty인 경우 /가 자동으로 추가되는 문제가 있어 Moya의 URL+Moya 파일을 참고하여 수정했어요.

솝마디 뷰 레이아웃 수정

  • 지난번 논의한대로 adjusted는 이미지에만 적용하도록 수정했어요.
  • 피그마를 참고하여 navigationBar를 hidden처리 했어요.
  • 잘못된 레이아웃도 함께 수정했습니다 !

📸 스크린샷

기능 스크린샷
딥링크 변경 사항 반영 및 baseURL trailing slash 수정
trailing slash 수정 전 image
trailing slash 수정 후 image

📮 관련 이슈

@yungu0010 yungu0010 added Fix 문제 해결, 코드 수정 윤서🍉 labels Oct 1, 2024
@yungu0010 yungu0010 requested a review from dlwogus0128 October 1, 2024 08:32
@yungu0010 yungu0010 self-assigned this Oct 1, 2024
Copy link

height bot commented Oct 1, 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.

Copy link
Contributor

@dlwogus0128 dlwogus0128 left a comment

Choose a reason for hiding this comment

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

PR이. 아주 이븐하게. 익었군요 . 생존입니다.
ㅎㅎ~ 수고 많으셨습니다! 얼른 저도 작업 끝내 올게요 총총.. 🫶🏻

make.top.equalTo(view.safeAreaLayoutGuide).offset(2.adjustedH)
make.leading.equalToSuperview().inset(8.adjusted)
make.top.equalTo(view.safeAreaLayoutGuide).offset(2)
make.leading.equalToSuperview().inset(8)
make.size.equalTo(40)
Copy link
Contributor

Choose a reason for hiding this comment

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

저도 이렇게 맞추겠습니다!


return coordinator
}

@discardableResult
Copy link
Contributor

Choose a reason for hiding this comment

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

감사감사합ㄴ디ㅏ 🥹~..

public var isDestination = false

public func execute(with coordinator: Coordinator, queryItems: [URLQueryItem]?) -> Coordinator? {
guard let coordinator = coordinator as? DailySoptuneCoordinator else { return nil }
guard let coordinator = coordinator as? ApplicationCoordinator else { return nil }

Copy link
Contributor

Choose a reason for hiding this comment

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

조아요

@@ -52,7 +52,8 @@ open class BaseService<Target: TargetType> {
}()

private let endpointClosure = { (target: API) -> Endpoint in
let url = target.baseURL.appendingPathComponent(target.path).absoluteString
let url = target.path.isEmpty ? target.baseURL.absoluteString : target.baseURL.appendingPathComponent(target.path).absoluteString

var endpoint: Endpoint = Endpoint(url: url,
Copy link
Contributor

Choose a reason for hiding this comment

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

요걸 바로 발견하구 . 해결해버링 . 짱짱 개발자

@@ -45,7 +45,7 @@ extension DefaultDailySoptuneUseCase: DailySoptuneUseCase {
.withUnretained(self)
.sink { event in
print("GetDailySoptuneResult State: \(event)")
} receiveValue: { _, resultModel in
} receiveValue: { owner, resultModel in
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
Contributor Author

Choose a reason for hiding this comment

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

혹시 어떤 방식이 더 편하신가요 ? 기존 코드를 보니 withUnretainedguard let이 혼재되어서 이번 기회에 하나로 통합하면 좋을 것 같아서요 !!

Copy link
Contributor

@dlwogus0128 dlwogus0128 Oct 1, 2024

Choose a reason for hiding this comment

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

withUnretained를 써주는 게 가독성면에서 더 조을 것 같아서..! 이 쪽으로 통합했으면 좋겠습니다 !-!

@yungu0010 yungu0010 merged commit 96aa943 into develop Oct 3, 2024
@yungu0010 yungu0010 deleted the fix/#395-DailySoptuneDeeplink branch October 3, 2024 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fix 문제 해결, 코드 수정 size/L 윤서🍉
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Fix] 딥링크 변경 사항 반영 및 baseURL trailing slash 수정
2 participants