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

Second assignment #2

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Second assignment #2

wants to merge 7 commits into from

Conversation

oy6uns
Copy link
Contributor

@oy6uns oy6uns commented Oct 11, 2022

No description provided.

@oy6uns oy6uns requested a review from yungu0010 October 13, 2022 18:46
@oy6uns oy6uns self-assigned this Oct 13, 2022
@oy6uns oy6uns added good first issue Good for newcomers Sungho and removed good first issue Good for newcomers labels Oct 13, 2022
Copy link
Member

@yungu0010 yungu0010 left a comment

Choose a reason for hiding this comment

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

🍀하나 둘 셋~! 우리 금잔디 화이팅🍀


import UIKit

class CheckAccount: UIViewController {
Copy link
Member

Choose a reason for hiding this comment

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

보통 파일명과 클래스명의 끝부분에는 상속받는 클래스의 이름(UI 제외)을 붙여주는 것 같아요~! 이 뷰의 경우에는 CheckAccountViewController가 되겠네요

Comment on lines +34 to +39
view.backgroundColor = .white

let components: [Any] = [helloLabel, acceptButton]
components.forEach{
view.addSubview($0 as! UIView)
}
Copy link
Member

Choose a reason for hiding this comment

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

viewDidLoad와 같은 생명주기 함수 내에서는 보통 메소드 호출을 해주는 것이 가독성에도 더 좋고 코드가 깔끔해진다고 생각합니다❗️개인적인 의견이니 참고만 해주세요 ~!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

넵~😄

/// text field를 만들 때, 텍스트+밑줄을 한꺼번에 만드는 class
import UIKit

class KakaoCustomView: UIView {
Copy link
Member

Choose a reason for hiding this comment

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

커스텀뷰 사용 너무 좋네요👍

// Use this method to optionally configure and attach the UIWindow `window` to the provided UIWindowScene `scene`.
// If using a storyboard, the `window` property will automatically be initialized and attached to the scene.
// This delegate does not imply the connecting scene or session are new (see `application:configurationForConnectingSceneSession` instead).
guard let _ = (scene as? UIWindowScene) else { return }
Copy link
Member

Choose a reason for hiding this comment

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

아래 같은 의미의 코드가 있어서 이 부분은 삭제해도 좋을 것 같습니다❗️

Comment on lines +65 to +66
make.top.equalToSuperview().offset(56.adjusted)
make.leading.equalTo(self.view.safeAreaLayoutGuide.snp.leading).offset(14.adjusted)
Copy link
Member

Choose a reason for hiding this comment

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

make라는 반환타임을 사용하는 것과 $0 형태의 단축인자를 사용하는 것을 혼용하는 것보단 통일해 쓰면 어떨까요?👀

Copy link
Contributor Author

Choose a reason for hiding this comment

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

좋은거 같아요!

Comment on lines +142 to +156
extension Int {
var adjusted: CGFloat {
let ratio: CGFloat = CGFloat(UIScreen.main.bounds.width) / 375
let ratioH: CGFloat = CGFloat(UIScreen.main.bounds.height) / 812
return ratio <= ratioH ? CGFloat(self) * ratio : CGFloat(self) * ratioH
}
}

extension Double {
var adjusted: CGFloat {
let ratio: CGFloat = CGFloat(UIScreen.main.bounds.width) / 375
let ratioH: CGFloat = CGFloat(UIScreen.main.bounds.height) / 812
return ratio <= ratioH ? CGFloat(self) * ratio : CGFloat(self) * ratioH
}
}
Copy link
Member

Choose a reason for hiding this comment

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

이 아이들은 따로 파일을 만들어 관리하면 좋을 것 같슴당😋

// Use this method to optionally configure and attach the UIWindow `window` to the provided UIWindowScene `scene`.
// If using a storyboard, the `window` property will automatically be initialized and attached to the scene.
// This delegate does not imply the connecting scene or session are new (see `application:configurationForConnectingSceneSession` instead).
guard let _ = (scene as? UIWindowScene) else { return }
Copy link
Member

Choose a reason for hiding this comment

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

아래 같은 의미의 코드가 있어서 지워주심 될 것 같아요 ~!

Copy link

@6uohul 6uohul 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

@6uohul 6uohul left a comment

Choose a reason for hiding this comment

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

오~ 과제하느라 수고하셨습니당~
잘 보고가요 ~~ 👍🏻

let components: [Any] = [startLabel, introLabel, emailPhoneTextField, passwordTextField, loginButton, newAccountButton]
components.forEach{
view.addSubview($0 as! UIView)
}
Copy link

Choose a reason for hiding this comment

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

따로 빼셔도 좋을 것 같아용~!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

늦게 봤네용 좋은 의견 감사합니당 ☺️

Copy link

@chaentopia chaentopia left a comment

Choose a reason for hiding this comment

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

CGRect말고 SnapKit 써서 레이아웃 잡아줘도 좋을 것 같아요~!! 고생하셨습니다!!!

@oy6uns
Copy link
Contributor Author

oy6uns commented Nov 14, 2022

CGRect말고 SnapKit 써서 레이아웃 잡아줘도 좋을 것 같아요~!! 고생하셨습니다!!!

오토레이아웃 잡고 수정한다는걸,,,깜박했네요 감사합니당~!~!

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.

4 participants