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: 로그인 버튼 레이아웃을 수정해요 #664

Merged
merged 2 commits into from
Sep 30, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ public final class AccountSignInViewController: BaseViewController<AccountSignIn
private let titleLabel = BBLabel(.body1Bold, textAlignment: .center, textColor: .gray200)
private let loginImageView = UIImageView()

private let kakaoLoginButton = UIButton()
private let appleLoginButton = UIButton()
private let kakaoLoginButton = BBButton()
private let appleLoginButton = BBButton()
private let loginStack = UIStackView()

override public func viewWillAppear(_ animated: Bool) {
Expand Down Expand Up @@ -69,16 +69,30 @@ public final class AccountSignInViewController: BaseViewController<AccountSignIn
loginStack.do {
$0.axis = .vertical
$0.spacing = Metric.spacing
$0.alignment = .fill
$0.distribution = .fill
$0.distribution = .fillEqually
}

kakaoLoginButton.do {
$0.setImage(DesignSystemAsset.kakaoLogin.image, for: .normal)
$0.setImageWithTitle(
image: DesignSystemAsset.kakao.image,
title: "Kakao로 계속하기"
)
$0.setTitle("Kakao로 계속하기", for: .normal)
$0.backgroundColor = UIColor(red: 254/255, green: 229/255, blue: 0/255, alpha: 1)
$0.layer.cornerRadius = 8
$0.setTitleColor(.bibbiBlack, for: .normal)
$0.setTitleFontStyle(.head2Bold)
}

appleLoginButton.do {
$0.setImage(DesignSystemAsset.appleLogin.image, for: .normal)
$0.setImageWithTitle(
image: DesignSystemAsset.apple.image,
title: "Apple로 계속하기"
)
$0.backgroundColor = UIColor(red: 254/255, green: 254/255, blue: 254/255, alpha: 1)
$0.layer.cornerRadius = 8
$0.setTitleColor(.bibbiBlack, for: .normal)
$0.setTitleFontStyle(.head2Bold)
}
}

Expand All @@ -102,8 +116,9 @@ public final class AccountSignInViewController: BaseViewController<AccountSignIn
}

loginStack.snp.makeConstraints {
$0.horizontalEdges.equalToSuperview().inset(Metric.inset)
$0.bottom.equalTo(view.safeAreaLayoutGuide).offset(Metric.loginOffset)
$0.height.equalTo(124)
$0.horizontalEdges.equalToSuperview().inset(20)
$0.bottom.equalTo(view.safeAreaLayoutGuide).offset(12)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ import SnapKit
public class BBButton: UIButton {

// MARK: - Views

public var mainStackView: UIStackView = UIStackView()
public var mainImageView: UIImageView = UIImageView()
Copy link
Collaborator

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 키워드를 사용하는 게 좋을 것 같은데 어떠신가염??

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

좋아여

public var mainTitleLabel: BBLabel = BBLabel()

// MARK: - Properties
Expand Down Expand Up @@ -65,17 +66,23 @@ public class BBButton: UIButton {
// MARK: - Helpers

private func setupUI() {
addSubview(mainTitleLabel)

addSubviews(mainStackView)
mainStackView.addArrangedSubview(mainTitleLabel)
}

private func setupConstraints() {
mainTitleLabel.snp.makeConstraints {
mainStackView.snp.makeConstraints {
$0.center.equalToSuperview()
}
}

private func setupAttributes() {
mainStackView.do {
$0.spacing = 4
$0.distribution = .fillProportionally
$0.axis = .horizontal
}

mainTitleLabel.do {
$0.numberOfLines = 1
}
Expand All @@ -100,6 +107,14 @@ public class BBButton: UIButton {
self.id = id
}

public func setImageWithTitle(image: UIImage?, title: String?) {
Copy link
Collaborator

@rlarjsdn3 rlarjsdn3 Sep 29, 2024

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() 메서드도 부탁드려요.

Copy link
Collaborator Author

@akrudal akrudal Sep 29, 2024

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하고는 분리하겠습니다!

Copy link
Collaborator

Choose a reason for hiding this comment

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

네 이해했어요~ 문서 주석만 잘 달아주세요

mainStackView.removeAllArrangedSubviews()
mainStackView.addArrangedSubviews(mainImageView, mainTitleLabel)

mainImageView.image = image
mainTitleLabel.text = title
}

/// 버튼의 타이틀을 변경합니다.
public override func setTitle(_ title: String?, for state: UIControl.State) {
titleLabel?.text = title
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"images" : [
{
"filename" : "appleLogin.svg",
"filename" : "Apple.svg",
"idiom" : "universal"
}
],
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"images" : [
{
"filename" : "kakaoLogin.svg",
"filename" : "kakao.svg",
"idiom" : "universal"
}
],
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

This file was deleted.

Loading