-
Notifications
You must be signed in to change notification settings - Fork 1
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
HomeViewController 연결 & 네비게이션 바 구현 & CTFontManagerRegisterFontsForURL로 폰트 적용 #38
Changes from 9 commits
5e3e068
0d24d3e
c75037f
5e7542a
04d3570
60d4af8
2035f1b
ae689c4
2bdb0c7
7650d90
d79da1d
9f9abad
2332b3e
787e3f0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
{ | ||
"colors" : [ | ||
{ | ||
"color" : { | ||
"color-space" : "srgb", | ||
"components" : { | ||
"alpha" : "1.000", | ||
"blue" : "0xF3", | ||
"green" : "0xFC", | ||
"red" : "0xFE" | ||
} | ||
}, | ||
"idiom" : "universal" | ||
} | ||
], | ||
"info" : { | ||
"author" : "xcode", | ||
"version" : 1 | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
{ | ||
"info" : { | ||
"author" : "xcode", | ||
"version" : 1 | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
{ | ||
"images" : [ | ||
{ | ||
"filename" : "Setting_line_light.png", | ||
"idiom" : "universal", | ||
"scale" : "1x" | ||
}, | ||
{ | ||
"filename" : "[email protected]", | ||
"idiom" : "universal", | ||
"scale" : "2x" | ||
}, | ||
{ | ||
"filename" : "[email protected]", | ||
"idiom" : "universal", | ||
"scale" : "3x" | ||
} | ||
], | ||
"info" : { | ||
"author" : "xcode", | ||
"version" : 1 | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
import UIKit | ||
|
||
final class MHNavigationBar: UIView { | ||
// MARK: - Property | ||
private let titleLabel = UILabel(style: .header) | ||
private let settingImageView = UIImageView(image: .settingLight) | ||
|
||
// MARK: - Initializer | ||
init(title: String) { | ||
super.init(frame: .zero) | ||
|
||
setup(with: title) | ||
configureConstraints() | ||
} | ||
|
||
required init?(coder: NSCoder) { | ||
super.init(coder: coder) | ||
|
||
setup(with: "") | ||
configureConstraints() | ||
} | ||
|
||
// MARK: - Setup & Configuration | ||
func setup(with title: String) { | ||
titleLabel.text = "\(title) 기록소" | ||
backgroundColor = .baseBackground | ||
addSubview(titleLabel) | ||
addSubview(settingImageView) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: 컨벤션 관련 내용인데 제 이전 PR의 경우 addSubView를 configureConstraints 함수에 위치시켰습니다.. 통일하면 더 좋지 않을까요?? 추가로 저희가 초반에 정한 컨벤션에선 configureAddSubView라는 함수를 따로 분리하는 것으로 정하긴 했었는데 addSubView가 그렇게 많지 않은 상황에서 함수를 분리하는 것은 오히려 가독성을 해칠 것 같다는 생각으로 일단 전 configureConsratints에 위치시켰습니다...ᐟ.ᐟ (setup에 들어가는게 더 적절한 것 같기도 하네용..) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 저는 회의록에 기록된 컨벤션처럼 configureAddSubView에 subview함수를 넣는 것도 좋아보입니다! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 저도 영현님 코드보면서 저 부분 고민되더라구요, |
||
} | ||
|
||
func configureConstraints() { | ||
titleLabel.setTop(anchor: topAnchor) | ||
titleLabel.setLeading(anchor: leadingAnchor) | ||
titleLabel.setBottom(anchor: bottomAnchor) | ||
titleLabel.setCenterY(view: settingImageView) | ||
|
||
settingImageView.setTop(anchor: topAnchor) | ||
settingImageView.setTrailing(anchor: trailingAnchor) | ||
settingImageView.setCenterY(view: self) | ||
settingImageView.setWidth(30) | ||
settingImageView.setHeight(30) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: 현재 코드의 경우 navigationBar 뷰의 양 끝에 딱 붙게 titleLabel과 settingView를 위치시키고 있는데, navigationBar가 view(네비바가 들어가는 view)와 width가 같다고 가정하고 padding을 여기서 넣어주는 방식에 대해선 어떻게 생각하시나요?? |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
import UIKit | ||
|
||
public final class HomeViewController: UIViewController { | ||
// MARK: - Property | ||
private let navigationBar = MHNavigationBar(title: "효준") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: 추후 바뀌어야할 임시적인 String 값이니 TODO를 추가해주면 좋을 것 같습니다 .ᐟ.ᐟ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 좋습니다 ! |
||
|
||
// MARK: - Initializer | ||
public init() { | ||
super.init(nibName: nil, bundle: nil) | ||
} | ||
|
||
required init?(coder: NSCoder) { | ||
super.init(coder: coder) | ||
} | ||
|
||
// MARK: - View LifeCycle | ||
override public func viewDidLoad() { | ||
super.viewDidLoad() | ||
|
||
setup() | ||
configureConstraints() | ||
} | ||
|
||
private func setup() { | ||
view.backgroundColor = .baseBackground | ||
view.addSubview(navigationBar) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3 |
||
} | ||
|
||
private func configureConstraints() { | ||
navigationBar.setTop(anchor: view.safeAreaLayoutGuide.topAnchor, constant: 20) | ||
navigationBar.setLeading(anchor: view.leadingAnchor, constant: 24) | ||
navigationBar.setTrailing(anchor: view.trailingAnchor, constant: 24) | ||
Comment on lines
+35
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P1: 저희 컨벤션에 따라 magic number는 상단에 constant로 만들어주시면 좋을 것 같습니다 .ᐟ.ᐟ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
앗, magic number는 의미상 알아보기 힘들거나 재사용시 변수처리하는 것아니었나요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
엇 다시 확인해보니 그런 것 같네요 ...ᐟ.ᐟ 제가 잘못알고 있었던 것 같습니다 .. 혼동을 드려 죄송합니다 .ᐟ.ᐟ |
||
} | ||
} |
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.
P1: 해당 ImageView는 설정 버튼을 나타내는 것으로 보입니다. 그렇다면 UIButton이 아닌 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.
인정합니다 !
수정해서 반영하겠습니다