[Fix] #396 - 솝마디 결과뷰 컴포넌트 수정 #398

dlwogus0128 · 2024-10-03T14:36:11Z

🌴 PR 요약

🌱 작업한 브랜치

[Fix] 솝마디 결과뷰 컴포넌트 수정 #396

🌱 PR Point

솝마디 결과뷰에서의 컴포넌트들을 수정했습니다.

콕찌르기 관련 컴포넌트(ProfileImageView, KokButton)들을 DSKit으로 옮겨, 모든 모듈에서 공통적으로 사용할 수 있도록 변경했습니다.
ProfileImageView 버튼 터치 시, 플그 프로필로 연결될 수 있게 구현했습니다.
디자인의 요청에 따라, ProfileImageView의 Border를 삭제하고 DailySoptuneResultVC의 content 배경 색을 변경했습니다.
그 외에도, 컨벤션에 맞게 코드를 수정하거나, 피그마와 UI가 맞지 않는 경우에 대하여 자잘한 수정을 거쳤습니다.

📌 참고 사항

아래에 관해서.. 조언해주실 부분이 있다면 리뷰.. 환영합니다 🥹

CustomProfileImageView의 경우, 피처별로 필요한 기능들이 약간씩 달라 (테두리 색 등) 추가적인 함수들은 각 모듈의 extension에서 구현해두었습니다. (이제 PokeFeature 자체를 임포트했던 부분들을 삭제해, 빌드 시 에러 나는 부분이 수정되었습니다. 죄송합니다 😅)

e.g., PokeFeature의 PokeProfileImageView

extension CustomProfileImageView {
    public func setImage(with url: String, relation: PokeRelation) {
        self.setImage(with: url, placeholder: DSKitAsset.Assets.iconDefaultProfile.image)
        self.setBorderColor(for: relation)
    }
    
    @discardableResult
    public func setBorderColor(for relation: PokeRelation) -> Self {
        self.layer.borderWidth = relation == .nonFriend ? 0 : 2
        self.layer.borderColor = relation.color.cgColor
        return self
    }
}

솝마디 결과 화면에서, 부적 라벨의 간격이 피그마와 약간 달라 수정해두었습니다.
또한, 서버에서 받아오는 문장을 두 줄로 자연스럽게 나누기 위해서 String+에 setLineBreakAtMiddle() 함수를 추가해놓았습니다.
1. 문장의 길이 / 2를 통해 중간 인덱스를 받아
2. 중간 인덱스를 기점으로 앞, 뒤에 처음 발견되는 공백을 기준으로 줄바꿈을 시행합니다.
  (단, 앞부분이 전체 문장의 40% 이상을 가져갈 수 있도록 해, 줄바꿈이 어색하지 않도록 구현했습니다.)

self.contentLabel.text = "\(model.userName)님,\n\(model.title.setLineBreakAtMiddle())"

혹시 이렇게 구현하지 않아도 .. 좋은 방법이 있을까요? 있다면 조언이 필요합뉘다 .. 🥹

이전	이후

pm 분께서 요청해주신 이벤트 트래킹 관련해서, Amplitude 초대 및 구체적인 이벤트 트래킹 구역을 전달받아야 할 것 같아 이후의 이슈에서 진행하고자 합니다 !_! 🫶🏻 @yungu0010

📸 스크린샷

생략합니다.

📮 관련 이슈

Resolved: [Fix] 솝마디 결과뷰 컴포넌트 수정 #396

height · 2024-10-03T14:36:14Z

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 · 2024-10-05T08:22:17Z

...rojects/Features/DailySoptuneFeature/Sources/DailySoptuneScene/VC/DailySoptuneResultVC.swift

@@ -96,6 +89,7 @@ public final class DailySoptuneResultVC: UIViewController, DailySoptuneResultVie

 extension DailySoptuneResultVC {
    private func setUI() {
+        self.navigationController?.isNavigationBarHidden = true


navigationBar.isHidden이랑 이 코드 중 하나만 남겨주세요 !

몰라서 찾아봤는데 , ,
navigationCotnroller?.navigationBar.isHidden은 bar는 존재하지만 일시적으로 숨기는 역할을 하고
navigationController?.isNavigationBarHidden은 bar가 논리적으로 완전히 지워져서 push와 같이 같은 흐름을 공유하는 모든 뷰에서 사라진다고 합니다 !_!

저희는 present 방식이라 아무거나 사용해도 좋을 듯 해요~.~

헉 .. 두 개 다 쓰고 있는지도 .. 몰랐어욯 .. (?)
감사합니다!!!

yungu0010 · 2024-10-05T08:33:48Z

SOPT-iOS/Projects/Modules/DSKit/Sources/Components/PokeComponents/PKokButton.swift

 import Core

-public final class PokeKokButton: UIButton {
+public final class PKokButton: UIButton {


그냥 궁금한건데 PokeKok에서 PKok으로 이름 바꾼 이유가 있을까요?

이거 DSKit 내부의 공통 컴포넌트들 이름을 보니까, 각 기능들의 첫 문자를 따서 붙여놓으셨길래
일관성을 위해서...! 이렇게 수정했어요 ㅎ.ㅎ

아 이거 컴포넌트 이름들은 원하는대로 해도 돼요! PokeKokButton이 살짝 더 이해하기 쉬운 것 같기도...ㅎㅎ
다음부터는 이름이 겹쳐서 prefix가 있어야 하는 경우 아니면 더 가독성 좋은 쪽으로 선택하면 됩니다~!

ㅇㅏ 넵!!! 감사합니다 ㅜ.ㅜ...

yungu0010 · 2024-10-05T08:49:30Z

SOPT-iOS/Projects/Features/PokeFeature/Interface/Sources/Enum/PokeRelation.swift

+public protocol ProfileRelation {
+
+}
+


사용되는 곳이 있나요?

헉 , 이거저거 해보다가 , 실수로 삭제를 안했나봐요 ..ㅋ
감사합니다 ..

yungu0010 · 2024-10-05T08:56:06Z

...rojects/Features/DailySoptuneFeature/Sources/Coordinator/DailySoptuneResultCoordinator.swift

+        dailySoptuneResult.vm.onProfileImageTapped = { [weak self] playgroundId in
+            guard let url = URL(string: "\(ExternalURL.Playground.main)/members/\(playgroundId)") else { return }
+
+            let webView = SOPTWebView(startWith: url)
+            self?.rootController?.pushViewController(webView, animated: true)
+        }


router에 pushSOPTWebView를 사용하면 더 좋을 것 같아요 ~!

yungu0010 · 2024-10-05T09:09:36Z

SOPT-iOS/Projects/Core/Sources/Extension/Foundation+/String+.swift

+        let middleIndex = self.count / 2
+        var spaceIndex: String.Index?
+        var result: String = ""
+
+        // 앞쪽 최소 길이를 설정 (문자열의 40% 정도)
+        let minFrontLength = Int(Double(self.count) * 0.4)
+
+        // 중간 인덱스로부터 앞쪽까지 탐색하면서, 가장 가까운 띄어쓰기 찾기
+        for i in (minFrontLength...middleIndex).reversed() {
+            let index = self.index(self.startIndex, offsetBy: i)
+            // 탐색에서 첫 번째 띄어쓰기 발견 시, break
+            if self[index] == " " {
+                spaceIndex = index
+                break
+            }
+        }


오호 이런 방법도 있었군요!_! 덕분에 배워갑니다 👀

middleIndex와 minFrontLength를 Index로 초기화하고 lastIndex를 사용하면 조금 더 간결한 코드가 될 것 같아요.

let middleIndex = self.index(self.startIndex, offsetBy: self.count / 2) let minFrontIndex = self.index(self.startIndex, offsetBy: Int(Double(self.count) * 0.4) // midFrontIndex와 middleIndex 사이 중 가장 마지막에 있는 공백의 인덱스 spaceIndex = self[minFrontIndex...middleIndex].lastIndex(of: " ")

이렇게 하면 for문과 if문 없이 간결하게 index를 찾을 수 있어요
반대로 아래의 경우에는 firstIndex(of: " ")를 사용하시면 되어요!

감사합니다 🫶🏻

lsj8706

지나가다 눈에 보이는 것이 있어서 몇가지 코멘트 남겨요~
@yungu0010 도 같이 보면 좋을 것 같아요~!

lsj8706 · 2024-10-07T14:42:30Z

.../Projects/Features/DailySoptuneFeature/Sources/Components/DailySoptuneProfileImageView.swift

+import DSKit
+
+extension CustomProfileImageView {
+    public func setImage(with url: String) {


이 함수 DailySoptune 모듈에서만 사용하는 거라면 public을 빼는게 좋아보여요~!
접근 제한자를 잘 쓰는 것이 캡슐화의 시작이라고 생각해요.

헉 네 .!!! 감사합니다

lsj8706 · 2024-10-07T14:45:08Z

.../Projects/Features/DailySoptuneFeature/Sources/Components/DailySoptuneProfileImageView.swift

+
+    // 오늘의 솝마디에서는 Border가 들어가지 않는 것으로 통일
+    @discardableResult
+    public func setBorder() -> Self {


borderWidth를 0으로 주는 것이 이 컴포넌트의 일반적인 사용법은 아니기 때문에 메서드 이름도 다음과 같이 더 특수하게 지으면 어떨까요?

public func setBorderForDailySoptuneProfile() -> Self // 단순 예시

이 메서드의 사용처가 적다면 메서드 추출하지 말고 그냥 직접 borderWidth를 0으로 설정하는 코드를 넣는 것도 괜찮아보여요.

lsj8706 · 2024-10-07T14:49:03Z

...eatures/DailySoptuneFeature/Sources/DailySoptuneScene/Views/DailySoptuneResultPokeView.swift

+        .filter { _ in self.user?.isAnonymous == false }
+        .map { _ in self.user }.asDriver()


Suggested change

.filter { _ in self.user?.isAnonymous == false }

.map { _ in self.user }.asDriver()

.filter { [weak self] _ in self?.user?.isAnonymous == false }

.map { [weak self] _ in self?.user }.asDriver()

오퍼레이터들도 escaping 클로저라 self를 참조할 때 약한 참조를 사용해야 누수를 방지할 수 있어요.

언제 weak를 붙여야 하는지, 왜 붙여야 하는지 챕터원들이랑 깊게 이야기 해보면 좋을 것 같아요!
(참조 타입, heap 메모리, arc 등의 키워드를 포함해서 이해하면 더 좋아요)

깜짝.. 등장해서 .. 리뷰.. 해주시는 ..건가요 ...
감사합니다 ,,,,,,,

lsj8706 · 2024-10-07T14:52:13Z

SOPT-iOS/Projects/Modules/DSKit/Sources/Components/PokeComponents/PKokButton.swift

 import Core

-public final class PokeKokButton: UIButton {
+public final class PKokButton: UIButton {


아 이거 컴포넌트 이름들은 원하는대로 해도 돼요! PokeKokButton이 살짝 더 이해하기 쉬운 것 같기도...ㅎㅎ
다음부터는 이름이 겹쳐서 prefix가 있어야 하는 경우 아니면 더 가독성 좋은 쪽으로 선택하면 됩니다~!

dlwogus0128 · 2024-10-07T15:12:56Z

감사합니다 🥹 코멘트 남겨주신 부분들 다음 이슈에서 잘 반영해놓겠습니다 ,,,

dlwogus0128 added 8 commits October 3, 2024 19:10

[Fix] #396 - 수정된 디자인 반영

1b4c35e

[Fix] #396 - ProfileImageView 공통 컴포넌트로 변경

49c1325

[Fix] #396 - KokButton 공통 컴포넌트로 변경

7cc0d38

[Feat] #396 - 프로필 터치 시 플그 웹뷰로 연결

8eb040c

[Fix] #396 - DailySoptuneResult -> 날짜 setDateFormat 사용

22703b1

[Fix] #396 - DailySoptuneResult -> 행운 라벨 UI 조정

0f33412

[Feat] #396 - DailySoptuneResult -> 콕찌르기 이후 UI 변경

539eda5

[Fix] #396 - ApplicationCoordinator에서 DailySoptuneDetail Flow 삭제

365c8a2

dlwogus0128 requested a review from yungu0010 October 3, 2024 14:36

pull-request-size bot added the size/L label Oct 3, 2024

dlwogus0128 self-assigned this Oct 3, 2024

dlwogus0128 added Fix 문제 해결, 코드 수정 재현✦ size/L and removed size/L labels Oct 3, 2024

[Feat] #396 - pull develop

df7a1d2

yungu0010 approved these changes Oct 5, 2024

View reviewed changes

[Fix] #396 - 코드 리뷰 반영

27f2ece

dlwogus0128 merged commit d59ee60 into develop Oct 7, 2024

dlwogus0128 deleted the fix/#396-fix-soptune-result-components branch October 7, 2024 11:34

lsj8706 reviewed Oct 7, 2024

View reviewed changes

dlwogus0128 mentioned this pull request Oct 15, 2024

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

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fix] #396 - 솝마디 결과뷰 컴포넌트 수정 #398

[Fix] #396 - 솝마디 결과뷰 컴포넌트 수정 #398

dlwogus0128 commented Oct 3, 2024

height bot commented Oct 3, 2024

yungu0010 left a comment

yungu0010 Oct 5, 2024

dlwogus0128 Oct 5, 2024

yungu0010 Oct 5, 2024

dlwogus0128 Oct 5, 2024

lsj8706 Oct 7, 2024

dlwogus0128 Oct 7, 2024

yungu0010 Oct 5, 2024

dlwogus0128 Oct 5, 2024

yungu0010 Oct 5, 2024

yungu0010 Oct 5, 2024

dlwogus0128 Oct 7, 2024 •

edited

Loading

lsj8706 left a comment

lsj8706 Oct 7, 2024

dlwogus0128 Oct 7, 2024

lsj8706 Oct 7, 2024

lsj8706 Oct 7, 2024 •

edited

Loading

dlwogus0128 Oct 7, 2024

lsj8706 Oct 7, 2024

dlwogus0128 commented Oct 7, 2024

		.filter { _ in self.user?.isAnonymous == false }
		.map { _ in self.user }.asDriver()

[Fix] #396 - 솝마디 결과뷰 컴포넌트 수정 #398

[Fix] #396 - 솝마디 결과뷰 컴포넌트 수정 #398

Conversation

dlwogus0128 commented Oct 3, 2024

🌴 PR 요약

🌱 PR Point

📌 참고 사항

📸 스크린샷

📮 관련 이슈

height bot commented Oct 3, 2024

yungu0010 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dlwogus0128 Oct 7, 2024 • edited Loading

Choose a reason for hiding this comment

lsj8706 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

lsj8706 Oct 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dlwogus0128 commented Oct 7, 2024

dlwogus0128 Oct 7, 2024 •

edited

Loading

lsj8706 Oct 7, 2024 •

edited

Loading