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] #148 - 공용 컴포넌트 생성 #149

Merged
merged 4 commits into from
Aug 6, 2024
Merged

[Fix] #148 - 공용 컴포넌트 생성 #149

merged 4 commits into from
Aug 6, 2024

Conversation

mcrkgus
Copy link
Member

@mcrkgus mcrkgus commented Aug 5, 2024

🔥 Pull requests

👷 작업한 내용

  • 공용 컴포넌트를 생성함
  • DTO 타입추론 통일

🖥️ 주요 코드 설명

TotalListCollectionViewCell

  • HankkiCategoryTag 공용 컴포넌트 사용 예시입니다.
  • 꼭 width값을 제거해주세요.
    private let menutag: UILabel = HankkiCategoryTag()
menutag.do {
            $0.setNeedsLayout()
            $0.layoutIfNeeded()
        }

menutag.snp.makeConstraints {
            $0.height.equalTo(20)
        }

✅ Check List

  • Merge 대상 브랜치가 올바른가?
  • 최종 코드가 에러 없이 잘 동작하는가?
  • 전체 변경사항이 500줄을 넘지 않는가?

📟 관련 이슈

@mcrkgus mcrkgus added 🛠️ Fix 버그, 오류 해결 💞 가현 가현 공주 작업 labels Aug 5, 2024
@mcrkgus mcrkgus self-assigned this Aug 5, 2024
Copy link
Contributor

@shimseohyun shimseohyun left a comment

Choose a reason for hiding this comment

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

고생하였소
width 값은 꼭 지우겠소

Comment on lines 37 to 39
let width = max(size.width + 20, minimumWidth)
let height = size.height + 10
return CGSize(width: width, height: height)
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
Member

Choose a reason for hiding this comment

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

LGTM

Comment on lines 34 to 41
// 글자 수에 따라 tag 사이즈 조정
override var intrinsicContentSize: CGSize {
let size = super.intrinsicContentSize
let width = max(size.width + 20, minimumWidth)
let height = size.height + 10
return CGSize(width: width, height: height)
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

p2:
코드를 깊게 탐구하다 문득 든 생각이오.
카테고리 태그 이외에도 이러한 둥근 태그 형태는 이곳저곳에서 많이 사용되는 것으로 아는데, 이것을 extesion으로 빼면 조금 효과적으로 사용할 수 있지 않을까라는 생각이 들고 있소
그대 생각은 어떠하오?

Copy link
Member Author

Choose a reason for hiding this comment

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

음 공용 컴포넌트로 관리하는게 좋을 것 같아유
소녀 말에 의하면 자주 사용되는 컴포넌트(버튼, 토스트메세지, 얼럿)들도 extension으로 빼는 방법일텐데... 기준이 모호해질것 같소
기본적인 속성들만 확장시키는 것이 더 좋을 듯 하옵니다

Comment on lines 25 to 32
extension HankkiAPIServiceProtocol {
typealias GetCategoryFilterResponseDTO = BaseDTO<GetCategoryFilterResponseData>
typealias GetPriceFilterResponseDTO = BaseDTO<GetPriceFilterResponseData>
typealias GetSortOptionFilterResponseDTO = BaseDTO<GetSortOptionFilterResponseData>
typealias GetHankkiListResponseDTO = BaseDTO<GetHankkiListResponseData>
typealias GetHankkiPinResponseDTO = BaseDTO<GetHankkiPinResponseData>
typealias GetHankkiThumbnailResponseDTO = BaseDTO<GetHankkiThumbnailResponseData>
typealias GetHankkiDetailResponseDTO = BaseDTO<GetHankkiDetailResponseData>
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
Member

Choose a reason for hiding this comment

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

소인 LEGEND 뿌듯하구려.

Copy link
Member

@EunsuSeo01 EunsuSeo01 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 37 to 39
let width = max(size.width + 20, minimumWidth)
let height = size.height + 10
return CGSize(width: width, height: height)
Copy link
Member

Choose a reason for hiding this comment

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

LGTM

Comment on lines 9 to 10

class HankkiCategoryTag: UILabel {
Copy link
Member

Choose a reason for hiding this comment

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

p3: Components 폴더에 있는 다른 파일들처럼 이 파일을 눌러보지 않아도 Label인지 View인지 알 수 있게 HankkiCategoryTagLabel로 네이밍을 더 명확하게 하는 것은 어떤지 묻고 싶소이다.

Copy link
Member Author

Choose a reason for hiding this comment

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

좋은 의견이오 고맙소 적극 반영할게유

Comment on lines 25 to 32
extension HankkiAPIServiceProtocol {
typealias GetCategoryFilterResponseDTO = BaseDTO<GetCategoryFilterResponseData>
typealias GetPriceFilterResponseDTO = BaseDTO<GetPriceFilterResponseData>
typealias GetSortOptionFilterResponseDTO = BaseDTO<GetSortOptionFilterResponseData>
typealias GetHankkiListResponseDTO = BaseDTO<GetHankkiListResponseData>
typealias GetHankkiPinResponseDTO = BaseDTO<GetHankkiPinResponseData>
typealias GetHankkiThumbnailResponseDTO = BaseDTO<GetHankkiThumbnailResponseData>
typealias GetHankkiDetailResponseDTO = BaseDTO<GetHankkiDetailResponseData>
Copy link
Member

Choose a reason for hiding this comment

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

소인 LEGEND 뿌듯하구려.

@@ -12,7 +12,7 @@ final class TotalListCollectionViewCell: BaseCollectionViewCell {
// MARK: - UI Components

let thumbnailImageView: UIImageView = UIImageView()
private let menutag: UILabel = UILabel()
private let menutag: UILabel = HankkiCategoryTag()
Copy link
Member

Choose a reason for hiding this comment

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

p3: 얘도 menuTagLabel로 하면 좋을 듯하오.

@mcrkgus mcrkgus merged commit b498a73 into develop Aug 6, 2024
@mcrkgus mcrkgus deleted the fix/#148 branch August 6, 2024 01:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💞 가현 가현 공주 작업 🛠️ Fix 버그, 오류 해결
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Fix] 공용 컴포넌트 추가 및 코드 일부 수정
3 participants