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] #21 - 공통 버튼 구현 #27

Merged
merged 18 commits into from
Jul 11, 2023
Merged

Conversation

jeongdung-eo
Copy link
Contributor

@jeongdung-eo jeongdung-eo commented Jul 8, 2023

🌁 Background

  • 공통 버튼 구현을 위한 PR 입니다.

📱 Screenshot

👩‍💻 Contents

스크린샷 2023-07-09 오전 2 11 31

✅ Testing

뷰컨트롤러에서 작업하시는 경우 아래와 같이 레이아웃을 잡으시면 됩니다!

button.do {
            $0.getButtonTitle(.start)
            // border 있는 경우 - 초기값 설정
            $0.getButtonUI(.gbbBackground2!, .gbbGray400!)
           // border 없는 경우 - 초기값 설정
            $0.getButtonUI(.gbbBackground2!)
            $0.updateUI(
                { self.button.getButtonUI(.gbbMain3!)
                },
                { self.button.getButtonUI(.gbbBackground2!, .gbbGray400!) }
            )   
        }
        
        button.snp.makeConstraints {
            $0.center.equalToSuperview()
            $0.directionalHorizontalEdges.equalToSuperview().inset(25)
            $0.height.equalTo(convertByHeightRatio(56))
        } 

📝 Review Note

  • 버튼의 타이틀을 enum으로 넣어뒀습니다.
  • backgroundColor과 border 컬러를 입력 받을 수 있게 함수로 선언해두었고, border 컬러 값이 있는 경우에는 border 컬러 값이 font color 될 수 있도록 구현했습니다.
  • button을 누를 때마다 UI 색상 변경: updateUI 함수 안에 첫번째 파라미터에 selected되었을 때의 컬러값을 넣고, 두번째 파라미터에 unselected되었을 때의 컬러값을 넣어주시면됩니다.

📣 Related Issue

📬 Reference

case signIn = "회원가입"
case next = "다음"
case confirm = "확인"
case duplicate = "중복 확인"
Copy link
Contributor

Choose a reason for hiding this comment

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

P1:
"중복확인" 으로 변경 부탁드려요 !
띄어쓰기가 없어야 하는 것 같숩니다 !

Comment on lines 42 to 45
private func setUI() {
makeCornerRound(radius: 11)
titleLabel?.font = .pretendardMedium(18)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Q:
훔 제가 then 을 처음 써봐서 어느 상황에서는 property.do { ~~ } 를 쓸지,
어느 상황에서 property.font 이런식으로 쓸지 모르겠어서 그냥 다 전자로 통일시키긴 했거든요 ... ??
혹시 이에 대한 기준이 있을까여 ..??

Comment on lines 47 to 74
func setButtonTitle(_ title: ButtonTitle) {
setTitle(title.rawValue, for: .normal)
}

func setButtonUI(_ color: UIColor, _ border: UIColor? = .clear) {
self.backgroundColor = color

switch color {
case .gbbMain3!, .gbbGray700!: setTitleColor(.gbbGray100, for: .normal)
case .gbbMain2!: setTitleColor(.white, for: .normal)
default:
setTitleColor(.gbbGray400, for: .normal)
}

if let border = border {
makeBorder(width: 1, color: border)
if border != .clear {
setTitleColor(border, for: .normal)
}
}
}

func setAction(completion: ((UIAction) -> Void)? = nil) {
let action = UIAction { action in
completion?(action)
}
addAction(action, for: .touchUpInside)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Q:
훔... 요건 논의해보면 좋을 것 같은 부분인데,
(정말 개인적인 코드 스타일) 저는 init 또는 viewDidLoad 에 들어가는 함수들만 set 을 앞에 붙였거든요 !
그 외에 설정하는 함수들은 configure 를 앞에 붙이고 쓰고 있습니다 !

오늘부터 합숙 시작하니까, 논의해보면 좋을 것 같아요 !!

Comment on lines 13 to 23
enum ButtonTitle: String, CaseIterable {

case login = "로그인"
case signIn = "회원가입"
case next = "다음"
case confirm = "확인"
case duplicate = "중복 확인"
case write = "작성하기"
case start = "시작하기"

}
Copy link
Contributor

Choose a reason for hiding this comment

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

P2:
훔 근데 Strings.swift 파일에 지정해 두었는데 여기서 또 텍스트로 작성하신 이유가 있을까요 ??
없다면 Strings.swift 파일의 값들 삭제 부탁드립니다 !

Copy link
Contributor

@seongmin221 seongmin221 left a comment

Choose a reason for hiding this comment

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

고생하셨어요 !!
P2 한 가지 남겨서 고 부분만 확인해주시면 어프루브 해드리겠습니다 !

Copy link
Contributor

@ckkim817 ckkim817 left a comment

Choose a reason for hiding this comment

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

어푸룹~ 고생하셨습니다

@seongmin221 seongmin221 self-requested a review July 10, 2023 19:06
Copy link
Contributor

@seongmin221 seongmin221 left a comment

Choose a reason for hiding this comment

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

굳굳 !!
빠른 머지 부탁 !

Copy link
Contributor

@ckkim817 ckkim817 left a comment

Choose a reason for hiding this comment

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

어뿌룹

private func setUI() {
self.do {
$0.makeCornerRound(radius: 11)
$0.titleLabel?.font = .pretendardMedium(18)
Copy link
Contributor

Choose a reason for hiding this comment

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

P1:
이거 텍스트 스타일 headLine으로 바꿔주세요~ (headLine 서체 pretendard medium으로 바꿔놓음)

@jeongdung-eo jeongdung-eo merged commit 435b3c3 into develop Jul 11, 2023
2 checks passed
@jeongdung-eo jeongdung-eo deleted the feat/#21-button-Components branch July 11, 2023 19:01
jeongdung-eo added a commit that referenced this pull request Jan 17, 2024
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] 공통 버튼 구현
3 participants