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

Feat [#28] 홈 게시글 뷰에 있는 버튼 action 구현 #36

Merged
merged 8 commits into from
Jan 11, 2024

Conversation

yeonsu0-0
Copy link
Collaborator

👻 PULL REQUEST

💻 작업한 내용

  • 게시글에 있는 각 버튼들에 액션을 구현했습니다.

케밥 버튼 누를 때 나오는 바텀 시트 구현
좋아요 버튼 하트 이미지 토글
투명도 버튼 팝업 추가

💡 참고사항

  • 바텀 시트 구현하면서 탭바 위로 위치하게 할 때 window를 사용했는데, iOS 13 이슈 관련 경고를 없애기 위해 extension을 추가했습니다.
    extension UIApplication {
    /**
    # keyWindowInConnectedScenes
    - Note: iOS 13 keyWindow 경고 해결
    */
    var keyWindowInConnectedScenes: UIWindow? {
    if #available(iOS 13.0, *) {
    return UIApplication.shared.windows.filter { $0.isKeyWindow }.first
    } else {
    return UIApplication.shared.keyWindow
    }
    }
    }

    @objc func handleDismiss() {
    UIView.animate(withDuration: 0.3, delay: 0, usingSpringWithDamping: 1, initialSpringVelocity: 1, options: .curveEaseOut, animations: {
    self.dimView.alpha = 0
    if let window = UIApplication.shared.keyWindowInConnectedScenes {
    self.bottomsheetView.frame = CGRect(x: 0, y: window.frame.height, width: self.bottomsheetView.frame.width, height: self.bottomsheetView.frame.height)
    }
    })
    }

  • 팝업 뷰에서 이미지가 있는 경우를 위해 init() 함수를 새로 추가했습니다.

init(popupImage: UIImage?, popupTitle: String, popupContent: String, leftButtonTitle: String, rightButtonTitle: String) {
super.init(frame: .zero)
popupTitleLabel.text = popupTitle // 팝업 타이틀
popupContentLabel.text = popupContent // 팝업 내용
cancleButton.setTitle(leftButtonTitle, for: .normal) // 팝업 왼쪽 버튼 타이틀
confirmButton.setTitle(rightButtonTitle, for: .normal) // 팝업 오른쪽 버튼 타이틀
if let image = popupImage {
popupImageView.image = image
}
setUI()
setHierarchy()
setLayout()
setAddTarget()
}

📸 스크린샷

기능 스크린샷
GIF

📟 관련 이슈

@@ -686,6 +693,7 @@
3CF184CB2B4EEC0B00816D5F /* MyPageViewController.swift in Sources */,
3C35565B2B494F0A0016BA49 /* UIColor+.swift in Sources */,
2AC9FB1B2B4DE77400D31071 /* AgreementListCustomView.swift in Sources */,
2F17418A2B500CC20089FC4D /* HomeBottomsheetView.swift in Sources */,
3C01692A2B4DC82D0075334B /* DontBePopupView.swift in Sources */,
2A2671FF2B4C3AF0009D214F /* Publisher+UIControl.swift in Sources */,
3C4993672B4F2644002A99CF /* MyPageContentViewController.swift in Sources */,

Choose a reason for hiding this comment

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

이 코드 패치를 간단히 검토해보겠습니다.
버그 위험 또는 개선 제안 사항을 언급할 것입니다.

  1. 파일 참조 추가:

    • UIApplication+.swift 파일의 참조가 추가되었습니다.
    • HomeBottomsheetView.swift 파일의 참조가 추가되었습니다.
  2. 확장(extension) 경로 추가:

    • "UIApplication+.swift" 파일이 Extension 그룹에 추가되었습니다.
  3. View 경로 추가:

    • "HomeBottomsheetView.swift" 파일이 Views 그룹에 추가되었습니다.
  4. 소스 파일 경로 추가:

    • "UIApplication+.swift" 파일이 Sources 그룹에 추가되었습니다.
    • "HomeBottomsheetView.swift" 파일이 Sources 그룹에 추가되었습니다.

코드 패치에서 발견된 주요 변경 사항은 위와 같으며, 버그 위험이나 개선 제안은 없습니다.

return UIApplication.shared.keyWindow
}
}
}

Choose a reason for hiding this comment

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

코드는 주어진 확장(extension)인 UIApplication+.swift 파일에 대한 것입니다. 이 코드의 목표는 iOS 13 이상에서도 keyWindow에 접근할 수 있도록 해주는 keyWindowInConnectedScenes 프로퍼티를 추가하는 것입니다.

이 코드 패치는 핵심적인 문제가 없어 보입니다. 그러나 다음과 같은 개선 제안이 있을 수 있습니다:

  • 코멘트(주석)에 사용된 언어를 영어로 변경하고, 코멘트의 내용을 좀 더 구체화할 수 있습니다. (예: "iOS 13 이상 버전에서의 keyWindow에 접근하기 위한 솔루션")
  • keyWindowInConnectedScenes 프로퍼티에 대한 설명이 더 자세하면 좋을 것입니다. 어떤 시나리오에서 이 프로퍼티를 사용해야 하는지 등을 추가로 기술하는 것이 도움이 될 수 있습니다.
  • else 절에서 shared를 사용하여 UIApplication.shared.keyWindow에 접근하는 대신, self.keyWindow를 사용하는 것이 더 명확할 수 있습니다.

위의 제안 사항은 코드의 가독성과 유지 보수성을 향상시키는 데 도움이 될 수 있습니다.


enum Popup {
static var transparentButtonImage: UIImage { .load(name: "transparentPopUp") }
}
}

extension UIImage {

Choose a reason for hiding this comment

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

해당 코드 패치는 사진 리소스에 대한 열거형 변수와 이미지 확장 기능을 추가합니다.

  1. Bug Risks:

    • 해당 코드 조각에는 실수 또는 버그의 나타낼 수 있는 명확한 위험이 없어 보입니다.
  2. Improvement Suggestions:

    1. 열거형 변수 이름: ImageLiterals라는 이름은 이미지 리소스를 설명하는 것으로 보입니다. 변수 이름을 더 구체적이고 명확하게 지정하는 것이 좋습니다. 예를 들어, ImageResources와 같은 이름으로 변경할 수 있습니다.
    2. 리소스 관리: 현재의 구조는 모든 이미지 리소스를 단일 파일로 저장하고 있습니다. 개별적인 폴더 구조를 사용하여 관련된 이미지들을 그룹화하는 것이 유지보수 및 이해하기 쉬울 수 있습니다.
    3. 이미지 로딩 방법: .load(name:) 메소드를 호출하는 부분에서 어떻게 이미지가 로드되는지 확인할 필요가 있습니다. 해당 부분이 올바른 이미지 리소스를 반환하는지 확인하고, 필요한 경우 에러 처리 또는 대안 로직을 추가하는 것이 좋습니다.
    4. Popup 열거형 내용: 현재 Popup 열거형에는 transparentButtonImage만 있습니다. Popup 열거형 자체에 더 많은 속성을 추가하여 팝업창 관련 리소스를 그룹화하는 것이 더 명확할 수 있습니다.

이러한 개선점을 고려하면 코드의 가독성, 유지보수 가능성 및 확장성이 향상될 수 있을 것입니다.

static let transparentPopupContentLabel = "지금 누르신 투명도 기능이 Don’t be를 더 온화한 커뮤니티로 만들기 위한 일이겠죠?"
static let transparentPopupLefteftButtonTitle = "조금 더 고민하기"
static let transparentPopupRightButtonTitle = "네, 맞아요"
}
}

Choose a reason for hiding this comment

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

위의 코드 패치에 대해 간단한 코드 리뷰를 도와드리겠습니다. 버그 위험 및 개선 제안들이 환영됩니다.

  1. 이 코드 패치는 기존의 enum StringLiteralsenum Home을 추가하는 것으로 보입니다.
  2. 새로운 enum Home은 각각 transparentPopupTitleLabel, transparentPopupContentLabel, transparentPopupLefteftButtonTitle, transparentPopupRightButtonTitle을 선언하여 문자열 값을 가지고 있는 것으로 보입니다.
  3. 입력된 한글 문자열 값들은 코드에서 잘 보여지고 이해할 수 있도록 작성되어 있습니다.
  4. 현재 코드에는 버그나 오류가 없는 것으로 보이며, 적절하게 작동할 것입니다.

개선 제안:

  • 이 코드 패치에서 변경할 부분은 약간의 코드 스타일과 가독성 관련된 내용입니다.
  1. 팀의 코드 스타일 가이드라인을 따르도록 들여쓰기를 조정합니다. 현재 첫 번째 MyPage 열거형 윗줄과 다른 부분의 들여쓰기가 일치하지 않습니다.
  2. 여러 줄의 문자열은 필요한 경우 """ 또는 #"""#을 사용하여 나타낼 수 있습니다. 예를 들어,
    static let transparentPopupContentLabel = """
    지금 누르신 투명도 기능이 Don’t be를 더 온화한 커뮤니티로 만들기 위한 일이겠죠?
    """
    
    이렇게 하면 여러 줄의 문자열을 사용할 때 코드가 더 읽기 쉬워집니다.

위의 제안 사항은 주관적인 것이며, 실제 사용되는 코드 스타일과 요구사항에 따라 다를 수 있습니다. 견고하고 가독성 있는 코드 작성을 위해 팀 내에서 코드 스타일과 가이드라인을 확인하는 것이 좋습니다.

"author" : "xcode",
"version" : 1
}
}

Choose a reason for hiding this comment

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

이 코드는 이미지 리소스 파일에 대한 정보를 담고 있는 JSON 파일처럼 보입니다. 아래는 간단한 코드 검토와 개선 제안입니다:

개선점:

  1. 코드에 오류는 없어 보입니다. 하지만, 이 코드 조각은 단독으로 사용하기보다는 다른 코드와 함께 사용되어야 하는 경우가 많습니다. 해당 코드를 어디에 사용하려는지에 따라서 더 구체적인 피드백을 할 수 있습니다.
  2. 파일 이름과 관련하여, 항상 컨벤션을 따르는 것이 좋습니다. 보통 파일 이름은 소문자와 언더스코어(_)를 사용합니다. "transparent_pop_up.png", "transparent_pop_up@2x.png", "transparent_pop_up@3x.png"와 같이 변경하는 것이 더 일반적입니다.

위 개선 제안 사항을 고려하여 코드를 수정할 수 있습니다.

buttonStackView.snp.makeConstraints {
$0.leading.trailing.bottom.equalToSuperview().inset(20.adjusted)
$0.height.equalTo(44.adjusted)
}
}
}
}

Choose a reason for hiding this comment

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

해당 코드는 DontBePopupView 클래스에 대한 코드 패치입니다. 요청하신 코드 리뷰를 진행하겠습니다.

  1. 버그 위험 사항:

    • 컨테이너 뷰(container)의 하위 뷰를 설정하는 부분에서 팝업 이미지(popupImageView), 팝업 타이틀 레이블(popupTitleLabel), 팝업 내용 레이블(popupContentLabel), 버튼 스택 뷰(buttonStackView) 등을 모두 추가하고 있습니다. 그러나, 팝업 이미지가 없는 경우에도 앞서 소개한 체크 없이 해당 뷰들을 추가하려 하고 있습니다. 이로 인해 레이아웃이 올바르게 설정되지 않을 수 있습니다. 따라서, 팝업 이미지의 유무를 확인하고, 팝업 이미지가 있는 경우에만 해당 뷰를 추가하는 조건문이 필요합니다.
  2. 개선 제안:

    • 이미지 뷰(popupImageView) 생성 이후 let popupImage = UIImageView() 문장이 불필요합니다. 해당 문장은 다음과 같이 변경하여 초기화와 할당을 동시에 수행할 수 있습니다: let popupImage = UIImageView().
    • container 뷰에 대한 레이아웃 설정 부분에서 Superview() 대신 superview를 사용해야 합니다.
    • 버튼과 관련된 속성을 중복하여 설정하는 것보다, 나중에 UIButton 확장으로 여러 번 재사용할 수 있도록 별도의 함수로 분리하는 것이 좋습니다.
    • 레이아웃 설정 부분에서 adjusted 값을 사용하고 있는데, 해당 값이 어떤 의미인지 알 수 없어서 이에 대한 확인이 필요합니다.

위와 같은 내용을 고려하여 코드를 수정하면 다음과 같습니다:

--- a/YourOriginalFile.swift
+++ b/UpdatedFile.swift
@@ -15,7 +15,7 @@ protocol DontBePopupDelegate: AnyObject {
 }
 
 final class DontBePopupView: UIView {
-
+    
     // MARK: - Properties
     
     weak var delegate: DontBePopupDelegate?
@@ -25,10 +25,15 @@ final class DontBePopupView: UIView {
     private let container: UIView = {
         let view = UIView()
         view.backgroundColor = .donWhite
-        view.layer.cornerRadius = 10
+        view.layer.cornerRadius = 10.adjusted
         return view
     }()
     
+    private let popupImageView: UIImageView = {
+        let popupImage = UIImageView()
+        return popupImage
+    }()
+
     private let popupTitleLabel: UILabel = {
         let label = UILabel()
         label.textColor = .donBlack
@@ -42,13 +47,14 @@ final class DontBePopupView: UIView {
         label.textColor = .donBlack
         label.textAlignment = .center
         label.font = UIFont.font(.body4)
+        label.numberOfLines = 0
         return label
     }()
     
     private let buttonStackView: UIStackView = {
         let stackView = UIStackView()
         stackView.distribution = .fillEqually
-        stackView.spacing = 12
+        stackView.spacing = 12.adjusted
         return stackView
     }()
     
@@ -57,7 +63,7 @@ final class DontBePopupView: UIView {
         button.setTitleColor(.donBlack, for: .normal)
         button.titleLabel?.font = UIFont.font(.body3)
         button.backgroundColor = .donGray3
-        button.layer.cornerRadius = 4
+        button.layer.cornerRadius = 4.adjusted
         return button
     }()
     
@@ -66,7 +72,7 @@ final class DontBePopupView: UIView {
         button.setTitleColor(.donWhite, for: .normal)
         button.titleLabel?.font = UIFont.font(.body3)
         button.backgroundColor = .donBlack
-        button.layer.cornerRadius = 4
+        button.layer.cornerRadius = 4.adjusted
         return button
     }()
     
@@ -86,6 +92,23 @@ final class DontBePopupView: UIView {
         setAddTarget()
     }
     
+    init(popupImage: UIImage?, popupTitle: String, popupContent: String, leftButtonTitle: String, rightButtonTitle: String) {
+        super.init(frame: .zero)
+        
+        if let image = popupImage {
+            popupImageView.image = image
+        }
+
+        popupTitleLabel.text = popupTitle // 팝업 타이틀
+        popupContentLabel.text = popupContent // 팝업 내용
+        cancleButton.setTitle(leftButtonTitle, for: .normal) // 팝업 왼쪽 버튼 타이틀
+        confirmButton.setTitle(rightButtonTitle, for: .normal) // 팝업 오른쪽 버튼 타이틀
+
+        setUI()
+        setHierarchy()
+        setLayout()
+        setAddTarget()
+    }
+    
     @available(*, unavailable)
     required init?(coder: NSCoder) {
         fatalError("init(coder:) has not been implemented")
@@ -109,12 +132,18 @@ extension DontBePopupView {
             container.addSubviews(popupTitleLabel, popupContentLabel, buttonStackView)
         }
         
+        if popupImageView.image != nil {
+            container.addSubviews(popupImageView, popupTitleLabel, popupContentLabel, buttonStackView)
+        }
+        
         buttonStackView.addArrangedSubviews(cancleButton, confirmButton)
     }
     
     private func setLayout() {
-        container.snp.makeConstraints {
-            $0.leading.trailing.equalToSuperview().inset(24.adjusted)
+        if popupImageView.image != nil {
+            container.snp.makeConstraints {
+                $0.leading.trailing.equalToSuperview().inset(24.adjusted)
+                $0.centerY.equalToSuperview()
+            }
         }
     
         popupTitleLabel.snp.makeConstraints {
@@ -131,13 +160,26 @@ extension DontBePopupView {
             $0.height.equalTo(44.adjusted)
         }
     } else {
-            container.snp.makeConstraints {
-                $0.leading.trailing.equalToSuperview().inset(24.adjusted)
-                $0.centerY.equalToSuperview()
-            }
-            
-            popupTitleLabel.snp.makeConstraints {
-                $0.top.equalToSuperview().inset(24.adjusted)
+        if popupImageView.image != nil {
+            popupImageView.snp.makeConstraints {
+                $0.top.equalToSuperview().inset(38.adjusted)
+                $0.size.equalTo(116.adjusted)
+                $0.centerX.equalToSuperview()
+            }
+        } else {
+            popupTitleLabel.snp.makeConstraints {
+                $0.top.equalToSuperview().inset(24.adjusted)
+                $0.leading.trailing.equalToSuperview().inset(18.adjusted)
+            }
+        }
+            
+        popupContentLabel.snp.makeConstraints {
+            $0.top.equalTo(popupTitleLabel.snp.bottom).offset(12.adjusted)
+            $0.leading.trailing.equalToSuperview().inset(18.adjusted)
+            $0.bottom.equalTo(cancleButton.snp.top).offset(-26.adjusted)
+        }
+        
+        buttonStackView.snp.makeConstraints {
+            $0.leading.trailing.bottom.equalToSuperview().inset(20.adjusted)
+            $0.height.equalTo(44.adjusted)
         }
     }
 }

이러한 수정 사항을 적용하면 코드의 버그 위험 사항이 감소하고 개선사항도 적용되므로 안정성과 유지보수성이 향상됩니다.

Copy link
Member

Choose a reason for hiding this comment

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

P3
이미지 있는 경우, 없는 경우 분기 처리 수고하셨습니다!! 깔끔하네요~!

@objc
func transparentShowPopupButton() {
TransparentButtonAction()
}
}

Choose a reason for hiding this comment

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

이 코드의 주요 변경 사항 및 개선 제안은 다음과 같습니다:

  1. likeButtonprivate으로 선언하지 않았습니다. 이 경우에도 앱 외부에서 이 버튼에 대한 접근이 가능합니다. 따라서 private let likeButton로 수정해야 합니다.

  2. setAddTarget() 메서드를 호출하여 버튼들에 대한 target-action을 설정하는 것은 좋은 방법입니다. 그러나 해당 메서드가 현재 클래스 내에서 호출되지 않으므로 실제로 동작하지 않을 것입니다. init(frame: CGRect) 또는 awakeFromNib()와 같은 초기화 메서드 내에서 setAddTarget()을 호출하여 기능이 작동하도록 해야 합니다.

  3. KebabButtonAction, LikeButtonAction, TransparentButtonAction은 클로저 타입의 프로퍼티로 정의되어 있습니다. 이러한 프로퍼티는 관례적으로 소문자로 시작하는 이름을 사용하며, kebabButtonAction, likeButtonAction, transparentButtonAction와 같이 이름을 변경하는 것이 좋습니다.

  4. isLiked는 사용하지 않는 것으로 보입니다. 이 프로퍼티는 코드에서 어떠한 목적으로 사용되지 않으므로 제거하는 것이 좋습니다.

  5. 코드의 가독성을 높이기 위해 여백을 일관되게 사용하는 것이 좋습니다. 예를 들어, backgroundUIView.addSubviews(...)likeStackView.addArrangedSubviews(...) 사이의 여백을 일정하게 맞추는 것이 가독성을 향상시킬 수 있습니다.

  6. kebabButton, likeButton, ghostButton에 대한 액션 메서드 이름은 현재 명확하지 않습니다. 액션 메서드의 용도와 의미를 잘 나타내는 이름으로 변경하는 것이 좋습니다.

  7. likeToggleButton()transparentShowPopupButton() 메서드에서 액션의 내용이 비어 있는 것으로 보입니다. 이를 적절한 동작으로 구현해야 합니다.

  8. 종속성 라이브러리인 SnapKit을 사용하는데 해당 코드는 주어지지 않아 확인할 수 없습니다. 그러나 설치된 버전과 호환되는 최신 버전을 사용하는지 확인하고 필요한 경우 업그레이드하는 것이 좋습니다.

  9. 코드의 전체적인 레이아웃 및 UI 구조는 제공되지 않아 모든 요소가 상호 작용하는 방식을 완전히 이해하기 어렵습니다. 따라서 코드 리뷰의 완전성을 확보하기 위해서는 더 많은 코드와 UI 정보가 필요합니다.


func confirmButtonTapped() {
transparentButtonPopupView.alpha = 0
// ✅ 투명도 주기 버튼 클릭 시 액션 추가
}
}

Choose a reason for hiding this comment

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

이 코드 패치의 주요 변경 사항은 다음과 같습니다:

  1. bottomsheet라는 새로운 HomeBottomsheetView 인스턴스가 추가되었습니다.
  2. transparentButtonPopupView라는 새로운 DontBePopupView 인스턴스가 추가되었습니다.
  3. setHierarchy() 함수에서 transparentButtonPopupView를 뷰의 하위 뷰로 추가하는 코드가 추가되었습니다.
  4. setDelegate() 함수에서 transparentButtonPopupView의 델리게이트를 설정하는 코드가 추가되었습니다.
  5. collectionView(_:cellForItemAt:) 함수에서 cell의 KebabButtonAction, LikeButtonAction 및 TransparentButtonAction 클로저가 정의되었습니다.
  6. extension HomeViewController: DontBePopupDelegate가 추가되었으며, 투명한 팝업 뷰의 취소 버튼 및 확인 버튼에 대한 액션을 처리합니다.

개선 제안:

  1. 가독성을 향상시키기 위해 스페이싱과 들여쓰기를 일관되게 유지하는 것이 좋습니다.
  2. 변수 및 상수의 이름은 가독성이 좋고 의도를 명확하게 전달할 수 있도록 지어주는 것이 좋습니다. 예를 들어, bottomsheettransparentButtonPopupView는 더 명확한 이름으로 변경될 수 있습니다.

문제나 개선할 점:

  1. 코드 리뷰를 통해 현재 버전의 코드에서는 오류나 버그의 위험이 발견되지 않았습니다. 해당 리뷰는 코드 스타일과 구조에 대한 소소한 개선을 제안한 것입니다.

}
}
}
}

Choose a reason for hiding this comment

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

이 코드에는 몇 가지 개선할 점이 있습니다:

  1. HomeBottomsheetView 클래스의 이름을 약간 변경하여 좀 더 명확하게 만들 수 있습니다. 예를 들어, SettingsBottomSheetView와 같이 설정과 관련된 네임스페이스를 포함하는 이름으로 변경할 수 있습니다.

  2. setUI() 메서드와 setRegisterCell() 메서드가 현재 비어 있으므로, 이러한 메서드의 목적과 기능에 대한 주석을 추가하거나, 필요 없는 메서드라면 제거할 수 있습니다.

  3. setDataBind() 메서드가 현재 구현되지 않은 상태이므로, 해당 메서드를 사용하기 위한 목적과 기능을 파악하고, 필요하다면 내용을 추가해야 합니다.

  4. handleDismiss() 메서드에서 completion 클로저가 비어 있기 때문에, 애니메이션 완료 이후에 어떤 동작을 수행할지 고려해야 합니다. 예를 들어, 애니메이션이 완료되었을 때 하단 시트를 숨기거나, 화면에서 제거하는 작업 등을 수행할 수 있습니다.

  5. handlepanGesture(_:) 내부의 스위치 문은 현재 .began, .changed, .ended 케이스만 처리하고 있습니다. 다른 가능한 케이스도 고려해서 해당하는 로직을 추가해야 합니다. 파라미터 gesture에 대한 옵셔널 체킹도 필요할 수 있습니다.

  6. 코드 상단의 주석 영역은 이 파일의 정보와 작성자, 작성일 등을 설명하는 용도로 사용되고 있는 것 같습니다. 필요하다면 이 주석 부분에 조금 더 자세한 정보를 추가해볼 수 있습니다.

  7. 코드 스타일과 일관성을 유지하기 위해 몇 가지 변경을 제안합니다:

    • 변수나 상수 선언시 타입 어노테이션(: 타입) 추가
    • 클로저 인자들 사이에 공백 추가
    • 들여쓰기에 탭 대신 스페이스 사용

이러한 개선 사항을 참고하여 코드를 검토하고 안전성과 가독성을 향상시킬 수 있습니다.

Copy link
Collaborator

@Heyjooo Heyjooo left a comment

Choose a reason for hiding this comment

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

너무 깔끔해요..! 수고하셨습니다 !!

Comment on lines +16 to +19
var KebabButtonAction: (() -> Void) = {}
var LikeButtonAction: (() -> Void) = {}
var TransparentButtonAction: (() -> Void) = {}
var isLiked: Bool = false
Copy link
Collaborator

Choose a reason for hiding this comment

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

p3
굳굳

Copy link
Member

@boogios boogios left a comment

Choose a reason for hiding this comment

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

P3
바텀 시트 커스텀 구현 미쳤네요 정말~~!! 고생많으셨습니다!!! 팝업뷰 추가로 커스텀도 고생하셨어요!

@@ -86,6 +92,23 @@ final class DontBePopupView: UIView {
setAddTarget()
}

init(popupImage: UIImage?, popupTitle: String, popupContent: String, leftButtonTitle: String, rightButtonTitle: String) {
Copy link
Member

Choose a reason for hiding this comment

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

P3
이미지 있는 경우의 init 좋네요!

buttonStackView.snp.makeConstraints {
$0.leading.trailing.bottom.equalToSuperview().inset(20.adjusted)
$0.height.equalTo(44.adjusted)
}
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

P3
이미지 있는 경우, 없는 경우 분기 처리 수고하셨습니다!! 깔끔하네요~!


// MARK: - UI Components

private let myView = HomeView()
private lazy var homeCollectionView = HomeCollectionView().collectionView
private let uploadToastView = DontBeToastView()

private let transparentButtonPopupView = DontBePopupView(popupImage: UIImage(named: "transparentPopUp"), popupTitle: StringLiterals.Home.transparentPopupTitleLabel, popupContent: StringLiterals.Home.transparentPopupContentLabel, leftButtonTitle: StringLiterals.Home.transparentPopupLefteftButtonTitle, rightButtonTitle: StringLiterals.Home.transparentPopupRightButtonTitle)
Copy link
Member

Choose a reason for hiding this comment

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

P2
PopupView 사용할 때는 enter 한번씩 해주시면 보기 편할거같아요!!! 그리고 ImageLiterals 로 변경해주시면 좋을거같습니다!

transparentButtonPopupView.snp.makeConstraints {
$0.edges.equalToSuperview()
}

Copy link
Member

Choose a reason for hiding this comment

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

P3
공백 체크 바랍니당!


func confirmButtonTapped() {
transparentButtonPopupView.alpha = 0
// ✅ 투명도 주기 버튼 클릭 시 액션 추가
}
}

Choose a reason for hiding this comment

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

이 코드 패치를 간단하게 검토해드리겠습니다.

버그 위험과 개선 제안:

  • loadView() 메서드에서 transparentButtonPopupView의 알파 값을 0으로 설정하고 있으나, setLayout() 메서드 내에서 뷰의 하위에 추가한 후 알파 값을 변경하지 않는다는 점을 확인해야 합니다. 이에 따라 알파 값이 항상 0으로 유지될 수 있습니다. 원하는 동작을 위해 해당 값들을 적절하게 변경해주어야 합니다.
  • HomeBottomsheetView 클래스가 선언되어 있지 않아 코드에서 사용되는 bottomsheet 객체의 초기화와 관련된 부분은 확인할 수 없습니다. 해당 부분도 추가적인 내용을 파악해야 합니다.

개선 제안:

  • homeCollectionView.dataSourcehomeCollectionView.delegate 설정부에 대해 주석으로 설명이 없기 때문에 역할을 명확히 하기 위해 주석을 추가하는 것이 좋습니다.
  • 컬렉션 뷰 셀에서 KebabButtonAction, LikeButtonAction, TransparentButtonAction 클로저를 정의하고 있으며, 클로저 내에서 바깥쪽 환경에 있는 self를 참조하고 있습니다. 메모리 관리와 관련하여 클로저 캡처 리스트를 적절하게 활용하고 강한 순환 참조를 피하기 위해 [weak self]를 적용하는 것이 좋습니다.

일반적으로 이 코드 패치는 버그를 일으킬 가능성은 없어보이지만, 몇 가지 개선 제안을 반영하는 것이 좋을 수 있습니다. 제안된 사항들을 확인하시고 필요한 수정사항을 적용해보세요.

@yeonsu0-0 yeonsu0-0 merged commit aff9457 into develop Jan 11, 2024
1 check passed
@yeonsu0-0 yeonsu0-0 deleted the feat/#28-homeAction branch January 11, 2024 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feat] 홈 뷰 버튼 action 구현
3 participants