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

[Refactor] MyPageView 수정사항 반영 #162

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

Conversation

0gonge
Copy link
Collaborator

@0gonge 0gonge commented Oct 18, 2024

🔍 PR Content

  • 기존의 취향부분을 삭제하고, MyPageView를 수정된 UI에 맞게 구현하였습니다.
  • 화면에 보이는 영상 둘러보기와 같은 버튼들이 여러곳에서 사용되어, 이를 컴포넌트화 시켰습니다. (ViskitYellowButton)

📸 Screenshot

2024-10-19.03.11.26.mov

📍 PR Point

  • 데이터가 없을 경우 EmptyView를 기존에는 View에 모든 뷰가 다 존재했는데, 따로 EmptyView를 만들어주어 분리시켰습니다.
  • 영상 둘러보기 시, 일단 영상으로 가는 로직은 아직 작성하지 않았고, 눌림 확인용 print만 진행했습니다. (추 후 영상 구현 완료 시 연결예정)
  • 아직 구현중이라, 북마크를 할 수 있는 영상이 없어 컬렉션 뷰 연결이 잘 되었는지는 아직 모르는 상태지만, 추 후 확인 후 수정예정입니다!

✏️ Notice

ViskitYellowButton 컴포넌트
image

goAroundButton.do { $0.setTitle("영상 둘러보기", for: .normal) }
와 같이 타이틀만 정의해서 사용해주시면 됩니다.

🙏 To Reviewers

  • 다음 작업을 위해 고민하던 중에, 현재 profileViewController에 데이터 바인딩 로직이 전부 존재하는데, 이를 저번에 viewModel은 가급적 사용하지 말라고 하셨던 기억이 있어서 어떻게 분리를 해주면 좋을지 조언 부탁드립니다!

  • resolved [Refactor] MyPageView 구현 #159

@0gonge 0gonge added 🎥 Feature UI & 기능 개발 ⚒️ Fix 기능상의 변경을 이유로 코드가 수정될 때 여경 labels Oct 18, 2024
@0gonge 0gonge self-assigned this Oct 18, 2024
Copy link
Collaborator

@Chandrarla Chandrarla left a comment

Choose a reason for hiding this comment

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

비즈니스 로직이 많아서 분리의 필요성을 느끼고, 구현 할 자신이 있다면 VIewModel을 사용할 수 있지 않을까요?? (개인적인 생각)
고생하셨습닌다 화이팅


public override func layoutSubviews() {
super.layoutSubviews()
layer.cornerRadius = bounds.height / 2
Copy link
Collaborator

Choose a reason for hiding this comment

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

p2
height 쓰는 부분에 .adaptiveHeight 필요할 것 같습니다!

Copy link
Collaborator

Choose a reason for hiding this comment

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

  • 다른 height 부분에도 수정 필요해요

Comment on lines +46 to +48
let paragraphStyle = NSMutableParagraphStyle()
paragraphStyle.lineSpacing = 30 - $0.font.lineHeight
paragraphStyle.alignment = .center
Copy link
Collaborator

Choose a reason for hiding this comment

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

p3
UILabel+에 선언돼있는 lineSpacing 설정 함수를 사용하지 않은 이유가 있나요 ??

Comment on lines +68 to +72
[
imageView,
titleLabel,
goAroundButton
].forEach { addSubview($0) }
Copy link
Collaborator

Choose a reason for hiding this comment

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

p3
addSubviews 써도 좋을 것 같아요!

Copy link
Collaborator

Choose a reason for hiding this comment

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

다른 View / VIewController에도 수정 필요해요

Comment on lines +97 to +104
func setActionButtonHandler(_ handler: @escaping () -> Void) {
goAroundButton.addTarget(self, action: #selector(actionButtonTapped), for: .touchUpInside)
self.actionButtonHandler = handler
}

@objc private func actionButtonTapped() {
actionButtonHandler?()
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

p3
actionButtonTapped()애서 동작을 정의하지 않고, setActionButtonHandler()로 클로저를 통해 동작을 전달하는 이유가 있나요 ??

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎥 Feature UI & 기능 개발 ⚒️ Fix 기능상의 변경을 이유로 코드가 수정될 때 여경
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

[Refactor] MyPageView 구현
2 participants