Skip to content

Conversation

@Naknakk
Copy link
Contributor

@Naknakk Naknakk commented Jan 15, 2024

⭐️Issue

#48

🌟Motivation

RegisterNormal 기능 80% 구현 했습니다.. ( 플랫폼 버튼 누를 때 webView 띄우는 것만 남았습니다. )

🌟Key Changes

  • 읽기 상태에 따른 뷰 변화
  • DatePicker를 이용해 읽은 기간 설정

🌟Simulation

Simulator Screen Recording - iPhone 13 mini - 2024-01-16 at 02 37 56


🌟To Reviewer

  • 코드가 많이 더러운데요 ... 일단 구현하는데 초점을 두느라 이렇게 되었습니다. 코드가 이해되지 않는 부분은 리뷰 달아주시면 빠르게 답변 달겠습니다.
  • 추후 반드시! 더러운 부분은 수정할게요. 일단 서버 붙이러 떠나겠습니다..

🌟Reference


@Naknakk Naknakk linked an issue Jan 15, 2024 that may be closed by this pull request
3 tasks
@Naknakk Naknakk self-assigned this Jan 15, 2024
Copy link
Collaborator

@hyowon612 hyowon612 left a comment

Choose a reason for hiding this comment

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

까다로운 부분 많았을텐데 정말 수고 많았어요!!!👼🏻


// MARK: - UI Components

private let background = UIView()
Copy link
Collaborator

Choose a reason for hiding this comment

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

W3
네이밍 통일해주세용 (backgroundView)

setUI()
setHieararchy()
setLayout()
selectedButton = startButton
Copy link
Collaborator

Choose a reason for hiding this comment

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

W2
요건 무슨 코드인가용?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

datePicker를 처음 열었을 때 시작 날짜 버튼이 활성화 되어있어야하므로, 이를 위해 추가한 코드입니다!

Comment on lines 124 to 125


Copy link
Collaborator

Choose a reason for hiding this comment

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

W3
공백 없애주세여

Comment on lines +203 to +209
readingStatusLabel.snp.makeConstraints {
$0.height.equalTo(42)
}

dropStatusLabel.snp.makeConstraints {
$0.height.equalTo(42)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

W2
라벨은 높이 지정 안해줘도 폰트에 맞춰서 저절로 높이 잡히지 않나요?

Copy link
Contributor Author

@Naknakk Naknakk Jan 16, 2024

Choose a reason for hiding this comment

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

말로 설명해써요 피그마 보면서 같이 이야기해야 편할 것 같아서!~!
간단하게 적어두면 라벨 고유 높이보다 더 큰 높이가 필요해서 주었습니다!!

Comment on lines 294 to 303
private func buttonDateStyle(of label: UILabel) {
label.do {
$0.makeAttribute(with: $0.text)?
.lineSpacing(spacingPercentage: 145)
.applyAttribute()
$0.font = .Label1
}
}

private func buttonTitleStyle(of label: UILabel) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

W2
함수 네이밍 컨벤션 맞춰주세용!

if self.isOn {
self.onStateLayout()
func changeState(_ state: Bool) {
UIView.animate(withDuration: self.animationDuration) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

W3
애니메이션도 넣었네용 짱 ~


func onStateLayout() {
circleView.snp.updateConstraints {
$0.trailing.equalTo(barView.snp.trailing).inset(2.76)
Copy link
Collaborator

Choose a reason for hiding this comment

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

W3
이 이쁘지 못한 inset은 디자인이 이렇게 돼있는거죠ㅜ?
정수로 수정하는게 좋을거같네욤

Copy link
Contributor Author

Choose a reason for hiding this comment

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

전에 한번 이야기해봤는데 이렇게 의도한 거라고 하셔서 그대로 할 것 같아요 !!

}()

popDatePicker.subscribe(onNext: { popDatePicker in
view.customDatePicker.isHidden = !popDatePicker
Copy link
Collaborator

Choose a reason for hiding this comment

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

W3
popDatePicker가 Bool값인가여?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

맞습니당 좀더 보기 편하게 showDatePicker로 네이밍 변경할게요!

Copy link
Member

@Guryss Guryss 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 19 to 22

self.window = UIWindow(windowScene: windowScene)
self.window?.rootViewController = navigationController
self.window?.rootViewController = RegisterNormalViewController()
self.window?.makeKeyAndVisible()
Copy link
Member

Choose a reason for hiding this comment

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

W1;
SceneDelegate 복구시켜놓는거 까먹지 않깅

Comment on lines +17 to +21
let dateFormatter: DateFormatter = {
let formatter = DateFormatter()
formatter.dateFormat = "yyyy-MM-dd"
return formatter
}()
Copy link
Member

Choose a reason for hiding this comment

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

W2;
DateForMatter는 생각보다 시간복잡도가 매우 크다는 점 알고 계셨나용?
저도 세미나때 static 키워드를 정리하면서 알게 되었는데, 우리가 해당 속성을 다룬다면
꼭 알아두는게 좋을 것 같아 아티클 가져와봅니당 :)
한번 읽어보시고 추후에 DateForMatter를 어떻게 다뤄볼지 얘기해보아용

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
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 +26 to +38
private let totalStackView = UIStackView()

private let buttonStackView = UIStackView()

private let startButton = UIButton()
private let startButtonStackView = UIStackView()
private let startTitleLabel = UILabel()
private let startDateLabel = UILabel()

private let endButton = UIButton()
private let endButtonStackView = UIStackView()
private let endTitleLabel = UILabel()
private let endDateLabel = UILabel()
Copy link
Member

Choose a reason for hiding this comment

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

뚠뚠 🎵 stackView 사랑은 오늘도 진행중이라네 🎶

Comment on lines 94 to 97
private func bind() {

}

Copy link
Member

Choose a reason for hiding this comment

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

W1;
안사용하는 함수면 지워줘도 될듯합니당

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 216 to 221
buttonStatusSubject
.subscribe(onNext: { status in
view.customDatePicker.bindReadStatus(status: status)
})
.disposed(by: disposeBag)
}
Copy link
Member

Choose a reason for hiding this comment

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

Rx 활용 최고네요 !!!

@Naknakk Naknakk merged commit f01caf1 into main Jan 16, 2024
@Naknakk Naknakk deleted the Feat/#48 branch January 17, 2024 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[Feat] RegisterNormal 기본적인 기능 구현

4 participants