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: BBToolTip Action 처리 및 마이그레이션 작업 해요 #693

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from

Conversation

Do-hyun-Kim
Copy link
Collaborator

@Do-hyun-Kim Do-hyun-Kim commented Oct 27, 2024

🔵PR을 올리기 전 아래 사항을 확인해주세요.

  • 구현한 로직과 기능이 올바르게 작동되는지 충분히 테스트해주세요.
  • 코드의 성능이나 메모리 효율성이 적절하게 고려되었는지, 불필요한 코드가 없는지 검토해주세요.
  • 이번 PR에서 구현된 주요 기능이나 해결된 문제에 대해 자세히 서술해주세요.
    (위 내용은 지워주세요)

😽개요

  • 기존 ToolTipView를 text, Thumbnail로 모듈 분리하여 기능구현 하였습니다.

🛠️작업 내용

  • BBBaseToolTipView는 ToolTipView에 뼈대가 되는 ShapeLayer를 그리는 역할을 수행하도록 내부 구현을 했습니다.
  • BBToolTip을 통해서 BBThumbnailToolTipView, BBTextToolTipView를 superView에 띄우도록 구현하였습니다.
  • BBThumbnailToolTipViewBBBaseToolTipView를 상속받고 있기 때문에 오직 Thumbnail에 필요한 Layout과 기능 구현만 수행하도록 로직을 구현했습니다.
  • BBTextToolTipView도 위와 동일
  • BBToolTipsuperView Property를 통해 자동으로 Layout을 잡도록 구현했습니다. 때문에 AutoLayout을 선언하지 않고 Tooltip을 넣어줘야하는 view를 넣어주시기만 하면 됩니다.

BBToolTip 사용 방법

//Property 선언
private let toolTipView: BBToolTip = BBToolTip(.monthlyCalendar)
        
toolTipView.do {
    // tooltipView에 기준이될 superView 주입
     $0.superview = tipButton
}
  // toggle 형식으로  tooltip show, hide 처리
reactor.pulse(\.$hiddenTooltipView)
    .bind(with: self) {
        $1 ? $0.toolTipView.hide()
        : $0.toolTipView.show()
     }
     .disposed(by: disposeBag)
  • (작업한 코드에 대한 자세한 설명해주세요. 필요하다면 이미지나 표를 첨부해주세요)
이미지①

✅테스트 케이스

  • ToolTip이 해당 superView 기준으로 정확히 배치되었는지 확인해요
  • TooTip의 Type이 변경될 경우 Layout이 변경되는지 확인해요

🙏🏻아래와 같이 PR을 리뷰해주세요.

  • PR 내용이 부족하다면 보충 요청해주세요.
  • 코드 스타일이 팀의 규칙에 맞게 작성되었는지, 일관성을 유지하고 있는지 확인해주세요.
  • 코드에 대한 문서화나 주석이 필요한 부분에 적절하게 작성되어 있는지 확인해주세요.
  • 구현된 로직이 효율적이고 올바르게 작성되었는지, 아키텍처를 잘 준수하고 있는지 검토해주세요.
  • 네이밍, 포매팅, 주석 등 코드의 일관성이 유지되고 있는지 확인해주세요.

- BBAnimatable 로직 수정
- BBToolTipType xPosition, yPosition 분리
- BBToolTipAction Nested Type 추가
- BBBaseToolTIpView 내부 drawable Method 추가
- BBToolTip Class 추가
- BBDrawable protocol, extension 제거
@Do-hyun-Kim Do-hyun-Kim self-assigned this Oct 27, 2024
@Do-hyun-Kim Do-hyun-Kim marked this pull request as draft October 27, 2024 13:36
- BBToolTip Layout을 frame 기반으로 로직 수정
…하기 위해 intrinsicContentSize 재정의

- BBTooltip contentView ContentView intrinsicContentSize 기반으로 로직 수정
- BBToolTipConfiguration maxWidth, maxHeight Properties 제거
- BBToolTip property, Intializer 수정
- BBTextToolTipView TouchControl 클릭시 Tooltip 사라지는 애니메이션 로직 제거
@Do-hyun-Kim Do-hyun-Kim marked this pull request as ready for review October 30, 2024 06:13
@@ -48,8 +48,8 @@ final public class MemoriesCalendarPageTitleView: BaseView<MemoriesCalendarTitle
private func bindOutput(reactor: Reactor) {
reactor.pulse(\.$hiddenTooltipView)
.bind(with: self) {
$1 ? $0.toolTipView.hidePopover()
: $0.toolTipView.showPopover()
$1 ? $0.toolTipView.hide()
Copy link
Collaborator

Choose a reason for hiding this comment

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

이거 toolTipView에 rx 뚫어주는거 어때용 ?ㅎㅎ 바로 bind(to: tollTipView.rx.isHidden 이렇게 쓰게요

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

확실히 Binder 따로 만들어 놓는게 가독성이 좋겠네요!!

  • 따로 Binder 구현해 놓을게요


UIView.animate(withDuration: duration, delay: 0, options: options) { [weak self] in
guard let self else { return }
self.transform = CGAffineTransform.identity
self.alpha = 1
self.contentView?.transform = CGAffineTransform.identity
Copy link
Collaborator

Choose a reason for hiding this comment

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

..UIView.animate는 weak self를 쓸 필요가 없다고 합니당...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

아하 UIView.animate에는 쓸필요가 없네요 감사합니다 :)

  • 추가로 GCD에도 쓸 필요가 없었네요 기존 GCD에 적용해 놨던 weak self 코드도 따로 이슈 파서 수정해 놓을 게요

Copy link
Collaborator

Choose a reason for hiding this comment

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

고거는 왜 쓸 필요 없나여?! 저도 알려주세영

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

제가 이해한 바로는 GCD 같은 경우 타입 프로퍼티, 타입 메서드로 구성이 되어 있기때문에 weak self를 사용하지 않아도 되요

  • 다만 기존에 있던 작업을 다른 스레드에 보낸다면 순환 참조 문제가 발생 할 수 있다고 하더라고여

//

import UIKit

/// **UIBezierPath**, ** CALayer**, **CGMutablePath**을 활용한 draw 메서드를 정의하는 Protocol입니다.
protocol BBDrawable {
Copy link
Collaborator

Choose a reason for hiding this comment

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

요거 없앤 이유가 있나영? 결국 메서드들은 쓰는 . 것같은뎅..?!

super.init(toolTipType: toolTipType)
setupToolTipUI()
setupToolTipContent()
setupAutoLayount()
Copy link
Collaborator

Choose a reason for hiding this comment

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

오타나써영!!

Copy link
Collaborator

Choose a reason for hiding this comment

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

근데 여기는 왜 setupUI(), setupAttributes() 이렇ㄱ ㅔ안하구.. 따로 네이밍하셧나용?!

Copy link
Collaborator

Choose a reason for hiding this comment

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

썸네일도 그러네영!! 확인해주세요옹

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

오타, 네이밍 같이 수정해놓겠습니다 👍 👍

contentText: "모두가 참여한 날과 업로드한 사진 수로\n이 달의 친밀도를 측정합니다"
)
case let .waitingSurvivalImage(contentText, profile):
return .init(
foregroundColor: .bibbiBlack,
backgroundColor: .mainYellow,
position: .bottom,
yPosition: .bottom,
Copy link
Collaborator

Choose a reason for hiding this comment

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

좋네영 ~! LGTM

Copy link
Collaborator

Choose a reason for hiding this comment

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

PR에 적어주신 사용법 주석으로 tooltipview에도 적어주시면 안되나영?!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

네넵 주석도 적용해놓을게요 💯

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.

2 participants