-
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
Conversation
GNB로 쓰일 가능성을 고려해 분리하였음
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.
수고많으셨습니다 .ᐟ.ᐟ 👍👍👍
아래 리뷰말고도 PR Description에 나와있는 캡쳐화면을 보면 저희 폰트가 잘 적용되어있지 않은 것 같습니다 ㅠㅠ 해당 부분 수정한 후 머지시켜야할 듯 합니다.. ㅠㅠ
final class MHNavigationBar: UIView { | ||
// MARK: - Property | ||
private let titleLabel = UILabel(style: .header) | ||
private let settingImageView = UIImageView(image: .settingLight) |
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.
인정합니다 !
수정해서 반영하겠습니다
addSubview(titleLabel) | ||
addSubview(settingImageView) |
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.
P2: 컨벤션 관련 내용인데 제 이전 PR의 경우 addSubView를 configureConstraints 함수에 위치시켰습니다.. 통일하면 더 좋지 않을까요??
추가로 저희가 초반에 정한 컨벤션에선 configureAddSubView라는 함수를 따로 분리하는 것으로 정하긴 했었는데 addSubView가 그렇게 많지 않은 상황에서 함수를 분리하는 것은 오히려 가독성을 해칠 것 같다는 생각으로 일단 전 configureConsratints에 위치시켰습니다...ᐟ.ᐟ (setup에 들어가는게 더 적절한 것 같기도 하네용..)
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.
저는 회의록에 기록된 컨벤션처럼 configureAddSubView에 subview함수를 넣는 것도 좋아보입니다!
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.
저도 영현님 코드보면서 저 부분 고민되더라구요,
윤철님 의견대로 기존에 컨벤션 정한 대로 처리하겠습니다 !!
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 comment
The reason will be displayed to describe this comment to others. Learn more.
P3: 현재 코드의 경우 navigationBar 뷰의 양 끝에 딱 붙게 titleLabel과 settingView를 위치시키고 있는데, navigationBar가 view(네비바가 들어가는 view)와 width가 같다고 가정하고 padding을 여기서 넣어주는 방식에 대해선 어떻게 생각하시나요??
|
||
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 comment
The 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 comment
The reason will be displayed to describe this comment to others. Learn more.
좋습니다 !
navigationBar.setTop(anchor: view.safeAreaLayoutGuide.topAnchor, constant: 20) | ||
navigationBar.setLeading(anchor: view.leadingAnchor, constant: 24) | ||
navigationBar.setTrailing(anchor: view.trailingAnchor, constant: 24) |
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: 저희 컨벤션에 따라 magic number는 상단에 constant로 만들어주시면 좋을 것 같습니다 .ᐟ.ᐟ
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: 저희 컨벤션에 따라 magic number는 상단에 constant로 만들어주시면 좋을 것 같습니다 .ᐟ.ᐟ
앗, magic number는 의미상 알아보기 힘들거나 재사용시 변수처리하는 것아니었나요?
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: 저희 컨벤션에 따라 magic number는 상단에 constant로 만들어주시면 좋을 것 같습니다 .ᐟ.ᐟ
앗, magic number는 의미상 알아보기 힘들거나 재사용시 변수처리하는 것아니었나요?
엇 다시 확인해보니 그런 것 같네요 ...ᐟ.ᐟ 제가 잘못알고 있었던 것 같습니다 .. 혼동을 드려 죄송합니다 .ᐟ.ᐟ
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.
navigationBar.setTop(anchor: view.safeAreaLayoutGuide.topAnchor, constant: 20) | ||
navigationBar.setLeading(anchor: view.leadingAnchor, constant: 24) | ||
navigationBar.setTrailing(anchor: view.trailingAnchor, constant: 24) |
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: 저희 컨벤션에 따라 magic number는 상단에 constant로 만들어주시면 좋을 것 같습니다 .ᐟ.ᐟ
앗, magic number는 의미상 알아보기 힘들거나 재사용시 변수처리하는 것아니었나요?
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.
고생 많으셨어요. 열정맨🔥
addSubview(titleLabel) | ||
addSubview(settingImageView) |
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.
저는 회의록에 기록된 컨벤션처럼 configureAddSubView에 subview함수를 넣는 것도 좋아보입니다!
|
||
private func setup() { | ||
view.backgroundColor = .baseBackground | ||
view.addSubview(navigationBar) |
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.
P3
addSubview 관련해서 수정하실 예정이라면 여기도 수정하시면 좋을 것 같아요!
CTFontManagerRegisterFontsForURL를 통해 Presentation 모듈에 있는 Font 가져오기
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.
수고하셨습니당 .ᐟ.ᐟ
#️⃣ 연관된 이슈
📝 작업 내용
⚽️ 트러블 슈팅
문제 상황
우리 팀은 Multiple Project를 통한 모듈화를 진행하였다.
그리고 앱 내에서 사용되는 폰트는 단 한 가지, “OwnglyphBerry”을 사용하고 있다.
Excutable 모듈인
MHApplication
모듈에서 plist에 적용하면 폰트 적용이 잘 되나, 모듈의 역할 상으로 Presentation에서 하는게 맞다.그러나,
MHPresentation
모듈에서 폰트를 Plist에 넣고 관리하면 모듈마다 Bundle이 다르기 때문에 MHApplication에서 MHPresentation의 번들을 읽을 수 없어 Plist를 가져올 수 없다.그렇기 때문에 다른 모듈이 갖고 있는 Font를 어떻게 다른 모듈에서 적용하는지 알아보겠다.
문제 해결
개요
먼저 MHPresentation에 폰트를 넣어서 저장하고, plist에 다음과 같이 추가한다.
MHApplication이 저 폰트를 앱에 등록하여 사용하기 위해 MHPresentation의 Bundle에 찾아가서 폰트가 저장된 URL을 찾아야 한다.
그리고 그 경로를
CTFontManagerRegisterFontsForURL
에 등록해줘야 한다!CTFontManagerRegisterFontsForURL
폰트 매니저로써 폰트를 등록시켜준다..!
위 메소드를 호출하면 url이 유효한 경우, Bundle에 있는 폰트가 등록되고 정상적으로 사용할 수 있게 된다.
1번만 호출해주면 되는데 AppDelegate 혹은 SceneDelegate에서 호출해주면 될듯 하다.
우리는 MHPresentation에 UIFont를 확장하여 static 메소드를 넣어주었고,
해당 메소드는
HomeViewController
가 있는 곳의 번들을 찾아서 OwnglyphBerry.ttf 폰트를 찾아 URL을 갖는다.이후 CTFontManagerRegisterFontsForURL에 넣어주어 등록할 수 있게 한다.
해당 메소드를 AppDelegate에서 위 메소드를 호출하여 폰트가 등록되도록 처리하였다.
깔-끔
배운 점
참조 링크
https://developer.apple.com/documentation/coretext/1499468-ctfontmanagerregisterfontsforurl
https://minsone.github.io/ios/mac/ios-register-custom-font
https://nsios.tistory.com/196
📸 스크린샷