-
Notifications
You must be signed in to change notification settings - Fork 0
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
Feature/week3 과제 구현 중 .. #6
base: dev
Are you sure you want to change the base?
Conversation
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.
코드 잘 봤습니다 1!!! 고생많으셨습니다 1!!!!
protocol DetailDelegate: AnyObject { | ||
func dataBind(nickname: String) // 다음화면에서 이전화면으로 Data | ||
} | ||
class DetailViewController_Closer: UIViewController { |
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.
네이밍을 할 때 camel case로 하면 좋을 것 같아요 !!
private func setUpButton() { | ||
versionInfoView.updateRecordButton.addTarget(self, action: #selector(updateRecordButtonTapped), for: .touchUpInside) | ||
|
||
appScoreView.allReviewButton.addTarget(self, action: #selector(allReviewButtonTapped), for: .touchUpInside) |
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.
delegate를 사용하면 좋을 것 같아요 !
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.
.DS_Store 파일은 당장은 문제가 되지 않더라도 추후에 깃에서 충돌을 유발하게 될 수도 있기 때문에 올라가지 않게 주의해주시는 게 좋습니다!
scrollView.snp.makeConstraints { | ||
$0.top.equalToSuperview() | ||
$0.bottom.equalToSuperview() | ||
$0.leading.equalToSuperview() | ||
$0.trailing.equalToSuperview() | ||
} |
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.edges.equalToSuperview()
로 적어줄 수 있을 것 같아용
} | ||
contentView.snp.makeConstraints { | ||
$0.edges.equalToSuperview() | ||
$0.width.equalTo(scrollView.snp.width) |
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.
contentView
는 scrollView
에 속하기 때문에 equalToSuperview()
로 적어줘도 좋을 것 같습니닷~!
} | ||
titleView.snp.makeConstraints { | ||
$0.height.equalTo(150) | ||
$0.leading.trailing.equalToSuperview() |
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.horizontalEdge.equalToSuperview()
로도 적어줄 수 있을 것 같아욥!
import SnapKit | ||
import UIKit | ||
import Then |
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.
퍼스트 파티 프레임워크인 UIKit을 가장 먼저 임포트해주면 더 보기에 좋을 것 같다는 의견 남깁니당
$0.top.equalToSuperview().offset(20) | ||
$0.leading.equalToSuperview().offset(20) |
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.
superview를 기준으로 잡을 때에는 inset 사용을 추천드립니다!
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.
너무 고생하셨습니당!!
setLayout() | ||
} | ||
|
||
private func setUI() { |
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로 하신 이유가 무엇인가용
$0.delegate = self | ||
$0.dataSource = self |
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.
delegate나 dataSource를 채택하는 부분은 따로 메소드화 시켜도 좋을 것 같아요 !
지금은 tableview 하나만 채택하면 되지만, 나중에 데이터 전달이 많아지게 될 경우에는 한 메소드에서 관리하는게 훨씬 편합니당
|
||
private func setLayout() { | ||
chartTableView.snp.makeConstraints { | ||
$0.leading.trailing.top.bottom.equalToSuperview() |
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.edges.equalToSuperview()
로도 쓸 수 있습니다
|
||
class ChartCell: UITableViewCell { | ||
|
||
private lazy var iconImageView = 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.
lazy 키워드를 사용하신 이유가 무엇인가용
func tableView(_ tableView: UITableView, heightForHeaderInSection section: Int) -> CGFloat { | ||
let sectionData = sectionList[section] | ||
let headerHeight: CGFloat = sectionData.title.isEmpty ? 0 : | ||
sectionData.subtitle.isEmpty ? 60 : 75 | ||
return headerHeight | ||
} |
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.
수고하셨습니다! 가현님이 이미 코드리뷰를 잘 남겨주셔서..
또한, 해당 과제와 상관없는 커밋이 같이 껴서 들어오는 이유에 대해서 알아보세요!
구현 완료 항목
Simulator.Screen.Recording.-.iPhone.16.Pro.-.2024-11-01.at.20.51.23.mp4