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
[Design] 메인 지도 뷰 UI를 구성합니다. #35
Conversation
…ock/GetARock-iOS into feature/20-design-map-ui
currentLocationButton -> moveToCurrentLocationButton
.github/workflows/BuildTest.yml
Outdated
@@ -0,0 +1,21 @@ | |||
name: GetARock Build Test |
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.
오잉 이 파일이 왜 여기 따라왔을까요..?
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.
오잉...?
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.
흐음...?
private lazy var bottomButtonStackView: UIStackView = { | ||
$0.axis = .horizontal | ||
return $0 | ||
}(UIStackView(arrangedSubviews: [createEventsButton, myBandsButton, myPageButton])) |
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.
q5;
요기 arrangedSubviews부분도 아래처럼 줄바꿈해주시면 좋을것같아용
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.
엇 여기는 120자가 안 넘어서 줄바꿈을 안 했는데 필요할까요..?
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.
엇 제가 Xcode 글자 수 가이드라인을 99자로 설정해 둬서 아래 코드도 잘못 개행한 거였어요..!
120자로 변경하고 보니 아래 코드도 120자를 안 넘기길래 개행 삭제하고 원래대로 돌렸습니다! e07dbdd
let camera: GMSCameraPosition = GMSCameraPosition.camera(withLatitude: -33.86, | ||
longitude: 151.20, | ||
zoom: 11) |
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.
Q;
이 부분은 위치값이 없을때 지도애서 보여주는 초기 위치 값인가요?
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.
네 사용자가 권한 허용을 안 했을 때 보여줄 기본 위치입니다
현재 시드니가 기본값이에요!
|
||
// MARK: - Life Cycle | ||
|
||
override func loadView() { |
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.
Q;
loadView()는 super.loadView()
가 사용되지않아 검색해보니
공식문서에서 super
키워드를 사용하지 말라고 되어있네요
Your custom implementation of this method should not call super.
그래서 좀 찾아보니 스토리보드를 사용한 경우 ,
스토리보드에서 그린 뷰들이 loadView()에서 그려지기 때문에
super.loadView()
를 사용하게되면 코드에서 정의한 뷰로 재정의? 되기때문에 사용하지 말라고 하는 걸까요..?
참고
https://developer.apple.com/documentation/uikit/uiviewcontroller/1621454-loadview
https://mrgamza.tistory.com/279
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.
P5
viewDidLoad에 해도 문제없이 실행되긴하던데 제일 mapView 설정이 제일 기본적인거라서 loadView에 선언하신건가요??
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.
우선 루키 질문에 대해서는
loadView()는 뷰 컨트롤러의 루트뷰(view)를 만들고 로드하는 메서드이고,
코드로 작성한 커스텀 뷰를 루트 뷰로 지정해줄 때 override해서 구현한다고 알고 있습니다.
그래서 여기서는 구글맵인 mapView를 루트 뷰로 사용하기 위해
loadView에서 해당 코드를 작성했습니다.
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.
그리고 노엘의 질문에 대해서는
스택오버플로우를 참고하니 2가지 큰 이유가 있는 것 같습니다.
첫 번째로는 노엘이 말씀해 주신 것처럼 스토리보드를 통해 구현한 경우 뷰컨트롤러 안에 뷰가 이미 구현되어 있어서 super.loadView()를 하는 경우 불필요하게 두번 호출됩니다. 또한 xib가 있는데 loadView를 오버라이드해서 구현하게 되면 기존 xib를 무시하고 loadView의 내용이 반영되는 것 같습니다.
두 번째로는 코드로만 개발하는 경우, 위에서 언급했듯이 코드로 작성한 커스텀 뷰를 루트 뷰로 지정해 줄 때 loadView를 오버라이드해서 구현하므로 굳이 superclass를 필요로 하지 않는 것 같습니다.
결론은, 필요 없으니까 굳이 호출해서 일을 늘리지 말라..? 인 것 같습니다.
참고
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.
고생하셨어요 로라! 지도색상 좋아요!
그리고 상단 요소들 좌우 패딩 값은 25로 맞춰주시면 감사하겠습니다!!🥰
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.
|
||
// MARK: - Life Cycle | ||
|
||
override func loadView() { |
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.
P5
viewDidLoad에 해도 문제없이 실행되긴하던데 제일 mapView 설정이 제일 기본적인거라서 loadView에 선언하신건가요??
private let createEventsButton: UIButton = { | ||
$0.setImage(UIImage(named: "createEventsButton"), for: .normal) | ||
return $0 | ||
}(UIButton()) |
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
프로퍼티 선언 방식도 처음에 맞추고 가는게 좋을 거 같아요 @GODNOEL @compuTasha @ccdkk
참고로 저는 이런식으로 합니다!
private lazy var textViewVstack: UIStackView = {
let stackView = UIStackView(arrangedSubviews: [bandIntroductionLabel, bandIntroTextView])
stackView.axis = .vertical
stackView.spacing = 10
return stackView
}()
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.
엇 저는 위키에 View Controller Convention 참고해서 구현했습니다!
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.
저는 클로저 문법($0)을 사용하는데,
선택한 이유는 프로퍼티 내에서 변수선언 할때의 네이밍 고민할 시간도 단축해주고
개인적으로 코드를 읽는데 더 가독성이 좋아서 그렇게 사용하고 있었습니다!
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.
그렇군요 좋아요 일단 이대로 갑시다!
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.
저도 루키와 같은 방법으로 사용합니다. 어떤 방법이 더 좋을까요? 통일하면 좋을 것 같아요
넵! 지금은 구글맵 뷰만 추가한 상황이라 기본 값으로 설정해둔 시드니가 보이고 |
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.
고생하셨어요 로라!🫶
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.
고생하셨습니다 로라 👍 설 연휴에도 열정 멋지네요🔥 감사합니다
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.
고생하셨습니다 로라! 맵뷰 싱크로율이 장난없네요.
.github/workflows/BuildTest.yml
Outdated
@@ -0,0 +1,21 @@ | |||
name: GetARock Build Test |
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.
흐음...?
private var currentLocationLabel: UILabel = { | ||
$0.text = "포항시 남구 효자동" | ||
$0.textColor = .white | ||
$0.font = UIFont.systemFont(ofSize: 30, weight: .bold) |
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.
p4;
develop에 머지된 font extension 메소드를 사용해보는 것은 어떨까요?
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.
앗 투두에 적어두고 반영없이 머지할뻔 했네요😓
확인 감사합니다! a48da22
[Design] 메인 지도 뷰 UI를 구성합니다.
1. 관련 이슈
2. 배경
메인으로 사용하는 지도 뷰 UI를 구성합니다.
3. 작업 내용
4. 리뷰 노트
맵 스타일을 적용하기 위해
MapsInfo.plist
에GMS_MAP_ID
를 추가해주세요[Design] extension UIFont를 통해 font Type를 추가합니다. #31 이 merge 되면
currentLocationLabel
에 font type을 적용할 예정입니다.지도 스타일 중에서 피그마 디자인과 가장 비슷한 걸 골랐는데 어떠신가요..?!
피그마 디자인 상에서 왼쪽 라벨과 오른쪽 버튼 그룹의 각 leading, trailing 패딩 값이 다른데 확인 한번 부탁드립니다 @GODNOEL
5. 테스트 방법
SceneDelegate
에서 아래와 같이 수정해주세요.6. 스크린샷