-
Notifications
You must be signed in to change notification settings - Fork 5
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: 로그인 버튼 레이아웃을 수정해요 #664
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨어여
@@ -100,6 +107,14 @@ public class BBButton: UIButton { | |||
self.id = id | |||
} | |||
|
|||
public func setImageWithTitle(image: UIImage?, title: String?) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BBButton을 상속받아 BBImageButton 클래스를 하나 따로 만들어서 구현하시는 게 어때요? BBButton 하나에 너무 많은 일을 하는 느낌이 들어서 그런데 한번 고려해주세요.
메서드 이름을 setTitle(image: UIImage?, title: String?, state: UIControl.State)
로 오버로딩해주실 수 있을까요? 메서드 이름이 너무 길어보이기도 하고 의미 전달이 충분하리라 생각이 들어요.
문서 주석도 한 줄 정도 달아주실 수 있나요? 예전에 구현하신 setBackground() 메서드도 부탁드려요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
음 저도 개인적인 생각으로 답변 달아볼게여..!!
버튼 하나에 너무 많은 일을 한다기에는
버튼은 원래 이미지 버튼, 이미지 텍스트 버튼, 텍스트 버튼이 있습니다.
이거를 저희는 UIButton 하나로 쓰고요
제 생각에는 BBButton이 너무 많은 일을 하고 있다 생각하지 않습니다.
결국 하는 일은 버튼이라는건데, 이 버튼 하는 역할을 모아둔게 클래스라고 생각해요
이거를 단순히 UI상의 문제로 이미지와 이미지 없는 버전을 나눠야한다 생각하지 않습니다!
예를 들어서, 버튼에 텍스트만 있다가, 선택하면 이미지 텍스트로 바뀌어야한다 이러면 BBButton이 두 개 수행해야하지 않을까욤?! 이런 이유로 저는 UI의 자유는 줬으면 좋겠습니다!
만약 image에는 뭐 클릭이 안되게하거나, 하이라이트가 안먹게 하거나 이런 기능상의 차이가 있으면 상속 받는 클래스를 만드는게 맞다 생각해요!
메서드 이름같은 경우에도 정확한게 좋다고 생각하는데,
setImage로 해서 title하고는 분리하겠습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네 이해했어요~ 문서 주석만 잘 달아주세요
@@ -14,7 +14,8 @@ import SnapKit | |||
public class BBButton: UIButton { | |||
|
|||
// MARK: - Views | |||
|
|||
public var mainStackView: UIStackView = UIStackView() | |||
public var mainImageView: UIImageView = UIImageView() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mainStackView
, mainStackView
같은 경우에는 setImageWithTitle
메서드로 setter 작업을 할 수 있어서 private 키워드를 사용하는 게 좋을 것 같은데 어떠신가염??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좋아여
😽개요
🛠️작업 내용
BBButton에 ImageView 추가
버튼 레이아웃 수정
✅테스트 케이스