Skip to content

centerButton 구성 작업 완료#1

Open
ornwoo96 wants to merge 56 commits intodevelopfrom
feat/home
Open

centerButton 구성 작업 완료#1
ornwoo96 wants to merge 56 commits intodevelopfrom
feat/home

Conversation

@ornwoo96
Copy link
Contributor

  • tabitem 클릭시 해당 viewcontroller 이동작업은 진행중입니다.

Copy link

@jeongjaein jeongjaein left a comment

Choose a reason for hiding this comment

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

예시 리뷰입니다.

Comment on lines +18 to +21




Choose a reason for hiding this comment

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

Suggested change

필요하지 않은 줄바꿈인 것 같습니다


protocol CameraViewProtocol: AnyObject {
var presenter: CameraPresenterProtocol? { get set }

Choose a reason for hiding this comment

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

줄바꿈 전체적으로 한번 정리 해야할 거 같아요

}

func showMemo(for name: String) {
router?.presentMemoScreen(from: view!, forname: name)

Choose a reason for hiding this comment

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

view가 무조건 있을 건 알지만
앱 내 전체적으로 강제언래핑 보단 옵셔널 바인딩을 사용했으면 좋겠습니다.


class HomeViewController: UIViewController {

let items = ["1월","2월","3월","4월","5월","6월","7월","8월","9월","10월","11월","12월"]

Choose a reason for hiding this comment

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

, 뒤에는 화이트스페이스하나 넣어주세요!

Comment on lines +26 to +48
// let segmetedControl = UISegmentedControl(items: items)
//
// let scrollView = UIScrollView()
// scrollView.contentSize = CGSize(width: .zero, height: 50)
//
// scrollView.addSubview(segmetedControl)
// view.addSubview(scrollView)
//
// scrollView.do {
// $0.translatesAutoresizingMaskIntoConstraints = false
// $0.topAnchor.constraint(equalTo: view.safeAreaLayoutGuide.topAnchor).isActive = true
// $0.leadingAnchor.constraint(equalTo: view.leadingAnchor).isActive = true
// $0.heightAnchor.constraint(equalToConstant: 50).isActive = true
// }
// segmetedControl.do {
// $0.translatesAutoresizingMaskIntoConstraints = false
// $0.topAnchor.constraint(equalTo: scrollView.topAnchor).isActive = true
// $0.leadingAnchor.constraint(equalTo: scrollView.leadingAnchor).isActive = true
// $0.trailingAnchor.constraint(equalTo: scrollView.trailingAnchor).isActive = true
// $0.bottomAnchor.constraint(equalTo: scrollView.bottomAnchor).isActive = true
// $0.heightAnchor.constraint(equalToConstant: 45).isActive = true
// }
//

Choose a reason for hiding this comment

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

불필요한 주석은 디벨롭에 들어가지 않았으면 좋겠습니다.

class MemoViewController: UIViewController {
var presenter: MemoViewPresenterProtocol?
var chuImage = UIImageView()
let memoNavBar = UINavigationBar()

Choose a reason for hiding this comment

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

swift는 약어를 지양합니다.
풀네임을 써주시면 좋을거같아요

let centerButton = UIButton()
let cameraButton = UIButton()
let libraryButton = UIButton()
var centerButtonExpanded: Bool = true

Choose a reason for hiding this comment

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

Suggested change
var centerButtonExpanded: Bool = true
var isCenterButtonExpanded: Bool = true

요론 물음표 느낌도 괜찮습니다 이해하기가 편해요

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants