Feat [#54] 마이페이지 계정 정보 UI 구현, 스크롤 자연스럽게 적용 #59
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -930,6 +936,8 @@ | |||
| 2A84465A2B4EE8B400F98F2A /* JoinProfileViewController.swift in Sources */, | |||
| 2A8868DA2B506D720017513A /* NotificationViewController.swift in Sources */, | |||
| 3C4993652B4F2471002A99CF /* MyPageSegmentedControl.swift in Sources */, | |||
| 3C7741542B531AC80069E694 /* AccountInfoDummy.swift in Sources */, | |||
| 3C7741522B5311750069E694 /* MyPageAccountInfoTableViewCell.swift in Sources */, | |||
| 3CF184CD2B4EEC1900816D5F /* MyPageView.swift in Sources */, | |||
| 2FB818D32B51694F00B7498F /* PostView.swift in Sources */, | |||
| 3C2854FF2B3AA01700369C99 /* ExampleView.swift in Sources */, | |||
There was a problem hiding this comment.
해당 코드 패치를 간단히 검토해보면 다음과 같습니다:
-
버그 위험 점: 코드 패치는 파일 및 그룹 구성 요소를 변경하는 것으로 보입니다. 만약 이러한 파일 또는 그룹의 경로가 중복되거나 잘못된 경우 컴파일 또는 프로젝트 빌드 문제가 발생할 수 있습니다.
-
개선 제안:
- 파일/그룹 경로에 대한 중복 확인 및 수정 방안 검토
- 코드 패치에 대한 주석 추가로 변경 내용을 명확하게 설명
다만, 코드 패치 전체 내용을 보여주지 않아 완전한 검토와 개선사항을 파악하기 어렵습니다. 더 자세한 검토는 패치 내용에 대한 전체 컨텍스트를 알아야 합니다.
| func setAddTarget() { | ||
|
|
||
| } | ||
| } |
There was a problem hiding this comment.
해당 코드는 MyPageAccountInfoTableViewCell이라는 테이블 뷰 셀을 정의하는 것으로 보입니다. UILabel을 두 개 만들어 정보 제목(infoTitle)과 내용(infoContent)을 표시합니다. SnapKit 프레임워크를 사용하여 레이아웃을 설정하고 있습니다.
버그나 위험이 있는지 여부는 코드 전체 구현을 파악해야 확실하게 알 수 있지만, 현재 이 코드 조각에서는 아무런 버그나 위험 요소가 보이지 않습니다.
코드 개선 제안은 아래와 같습니다:
setAddTarget()메서드와 관련된 코드가 비어 있으므로, 해당 메서드의 구현을 추가하거나 필요 없는 경우 삭제합니다.fatalError("")를 사용하여init?(coder:)메서드를 구현하면 좋습니다. 세부 구현에 따라 다른 동작을 수행해야 하는 경우 해당 동작을 구현하도록 수정합니다.setUI()메서드가 현재 비어 있으므로, UI 구성 요소의 속성을 설정하고 관련된 초기화 코드를 추가하면 좋습니다.
| AccountInfoDummy(content: "2024.01.14") | ||
| ] | ||
| } | ||
| } |
There was a problem hiding this comment.
코드 리뷰를 해드리겠습니다. 이 코드 패치는 AccountInfoDummy.swift 파일에 대한 내용이며, AccountInfoDummy라는 구조체와 AccountInfoDummy의 확장(extension)이 포함되어 있습니다.
주어진 코드는 단순한 구조체로, content라는 문자열 프로퍼티를 가지고 있습니다. AccountInfoDummy.dummy() 함수는 AccountInfoDummy 인스턴스들로 이루어진 배열을 반환합니다.
코드의 버그나 위험한 부분은 보이지 않습니다. 그러나 이 코드의 개선 제안은 다음과 같습니다:
- 주석에 작성된 파일 생성 날짜 정보는 필요하지 않을 수 있습니다. 파일 관리 시스템에 의해 추적될 수 있기 때문입니다.
- 예시 데이터가 하드코딩되어 있으므로 실제 애플리케이션에서 사용할 경우 수정 가능하도록 하는 것이 좋습니다. 예를 들어, 외부 데이터 소스에서 데이터를 가져오는 방식으로 변경하는 것이 좋습니다.
이외에는 문제가 없어보이는 것 같습니다.
| // | ||
| //extension ExampleViewController: UICollectionViewFlowLayout { | ||
| // | ||
| //} |
There was a problem hiding this comment.
아래는 코드 패치입니다. 이에 대한 간단한 코드 리뷰를 도와드리겠습니다. 버그 위험과/또는 개선 제안 등을 환영합니다.
- accountInfoTableView 및 seperateView와 같은 UI 구성 요소가 추가되었습니다.
- moreInfoTitle, moreInfoButton, signOutButton과 같은 레이블 및 버튼을 만들어 UI에 추가하였습니다.
- ViewDidLoad() 함수에서 getAPI() 함수와 관련된 설정, UI 설정, Delegates 및 RegisterCell 설정이 추가되었습니다.
- viewWillAppear() 함수에서 navigationController의 속성 변경이 이루어집니다.
- backButtonPressed() 함수가 backButtonTapped()으로 변경되었습니다.
- MyPageAccountInfoViewController 클래스 내에서 확장(extension)이 추가되었습니다.
- setUI(), setHierarchy(), setLayout(), setDelegate(), setAddTarget(), setRegisterCell(), setDataBind() 및 addUnderline(to:)와 같은 메서드가 추가되었습니다.
- backButtonTapped()에서 popViewController(animated:) 함수로 화면을 이동하는 것이 변경되었습니다.
- UITableViewDelegate 및 UITableViewDataSource 프로토콜이 추가되었습니다.
- UITableViewDelegates를 통해 테이블 셀을 선택하는 기능이 추가되었으며, tableView(_:heightForRowAt:)를 통해 셀 높이를 설정할 수 있습니다.
- UITableViewDataSource를 통해 테이블 셀의 개수 및 셀 내용을 설정할 수 있습니다.
- Network에 대한 확장(extension)이 추가되었습니다.
개선 사항 및 버그 위험을 더 정확하게 판단하기 위해서는 AccountInfoDummy.dummy(), getAPI()에서 반환된 데이터 등에 대한 정보가 필요합니다.
| @@ -209,7 +198,6 @@ extension MyPageContentViewController: UICollectionViewDataSource, UICollectionV | |||
| extension MyPageContentViewController: UIScrollViewDelegate { | |||
| func scrollViewDidScroll(_ scrollView: UIScrollView) { | |||
| let yOffset = scrollView.contentOffset.y | |||
| let navigationBarHeight = self.navigationController?.navigationBar.frame.height ?? 0 | |||
| if yOffset > 0 { | |||
| scrollView.isScrollEnabled = true | |||
| } else if yOffset < 0 { | |||
There was a problem hiding this comment.
이 코드 패치는 MyPageContentViewController 클래스에 대한 변경 사항을 포함하고 있습니다. 몇 가지 버그 가능성과 개선 제안을 제시할 수 있습니다.
-
tabBarHeight프로퍼티가 삭제되었습니다. 이전에는viewDidLayoutSubviews()에서 탭 바의 높이를 설정하고 있었는데, 이제 필요하지 않아 보입니다. 따라서 다른 부분에서 이 값을 사용하는 경우 해당 코드를 확인하여 제거해야 합니다. -
homeCollectionView.snp.makeConstraints에서 제약 조건을 설정할 때top,leading,trailing,bottom에 대해 따로 설정하고 있습니다. 이를$0.edges.equalToSuperview()로 변경하여 코드를 줄일 수 있습니다. -
uploadToastView.snp.makeConstraints에서 제약 조건을 설정하는 방식도 개선됐습니다. 기존에는leading,trailing,bottom을 별도로 설정하고 있었는데$0.leading.trailing.bottom.equalToSuperview().inset(16.adjusted)으로 변경하면 됩니다. -
scrollViewDidScroll(_:)에서navigationBarHeight를 사용하고 있는데, 해당 변수가 삭제된 것으로 보입니다. 관련된 코드를 확인하고 필요한 경우 수정해야 합니다.
위의 변경 사항을 고려하여 코드 리뷰와 개선 사항을 확인하였습니다.
| scrollView.isScrollEnabled = true | ||
| } else { | ||
| rootView.myPageContentViewController.homeCollectionView.isScrollEnabled = true | ||
| rootView.myPageContentViewController.homeCollectionView.isUserInteractionEnabled = true | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
아래의 코드 패치에 대해 간단한 코드 리뷰를 도와드리겠습니다. 버그 위험 및 개선 제안 사항을 환영합니다:
— 상단에 SnapKit 라이브러리가 추가되었습니다. 이를 사용하여 코드에서 제안하는 것과 같이 레이아웃을 설정할 수 있습니다.
— tabBarHeight 프로퍼티가 추가되어 초기값은 0으로 설정되었습니다.
— statusBarView 라는 새로운 뷰가 추가되었습니다. 이 뷰는 상태 표시줄에 해당하는 부분입니다.
— loadView() 함수에서 setLayout() 함수가 호출되며, UI 컴포넌트의 레이아웃을 설정합니다.
— viewWillAppear() 함수에서 상태 표시줄의 색상을 변경하고 statusBarView를 추가합니다.
— viewWillDisappear() 함수에서 statusBarView를 제거합니다.
— viewDidLayoutSubviews() 함수에서 rootView.pageViewController.view의 제약 조건을 재설정하고, 해당 ViewController의 최대 높이를 계산하여 할당합니다.
— viewDidLoad() 함수에서 setLayout() 함수 호출 부분이 삭제되었습니다.
— scrollViewDidScroll() 함수에서 제약 조건의 재설정 및 위치 값 조정 부분이 변경되었습니다.
- STATUS_BAR_HEIGHT라는 상수가 선언되지 않았으므로 이전 코드에서 정의된 statusBarHeight를 사용하면 됩니다.
| @@ -142,7 +142,7 @@ extension MyPageView { | |||
| pageViewController.view.snp.makeConstraints { | |||
| $0.top.equalTo(segmentedControl.snp.bottom).offset(2.adjusted) | |||
| $0.leading.trailing.equalToSuperview() | |||
| $0.height.equalTo(2000) | |||
| $0.height.equalTo(UIScreen.main.bounds.height - 150) | |||
| } | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
아래는 코드 수정 사항입니다. 버그 위험 및 개선 제안사항을 요약해서 알려드리겠습니다:
119번 줄:
- $0.top.leading.bottom.equalToSuperview()
- $0.edges.equalToSuperview()
=> 상위 뷰에 대한 모든 가장자리 제약 조건을 설정하는 $0.edges.equalToSuperview()로 수정함.
142번 줄:
- $0.height.equalTo(2000)
- $0.height.equalTo(UIScreen.main.bounds.height - 150)
=> 페이지 뷰 컨트롤러의 높이를 UIScreen.main.bounds.height - 150으로 수정함.
개선 사항:
- 코드 전체적으로 가독성과 일관성이 꽤 좋은 편이며 큰 문제는 없어 보입니다.
- 단, 하드 코딩된 값들 (예: 2000)은 가능하면 변수나 상수로 대체하는 것이 좋습니다.
- 구체적인 기능이나 코드 속성에 대한 추가적인 정보를 주시면 더 자세한 리뷰와 개선 제안을 도와드릴 수 있습니다.
|
|
||
| extension MyPageAccountInfoViewController { | ||
| private func setUI() { | ||
| self.title = "계정 정보" |
There was a problem hiding this comment.
P3
StringLiterals 적용해주세요!
| accountInfoTableView.snp.makeConstraints { | ||
| $0.top.equalTo(self.view.safeAreaLayoutGuide) | ||
| $0.leading.trailing.equalToSuperview() | ||
| $0.height.equalTo((48 * 4).adjusted) |
|
|
||
| extension MyPageAccountInfoViewController: UITableViewDelegate { | ||
| func tableView(_ tableView: UITableView, didSelectRowAt indexPath: IndexPath) { | ||
| print("ddd") |
| $0.bottom.equalTo(tabBarHeight).inset(6.adjusted) | ||
| $0.leading.trailing.bottom.equalToSuperview().inset(16.adjusted) | ||
| $0.height.equalTo(44) | ||
| } |
| super.viewDidLayoutSubviews() | ||
|
|
||
| let safeAreaHeight = view.safeAreaInsets.bottom | ||
| let tabBarHeight: CGFloat = 70.0 |
| @@ -142,7 +142,7 @@ extension MyPageView { | |||
| pageViewController.view.snp.makeConstraints { | |||
| $0.top.equalTo(segmentedControl.snp.bottom).offset(2.adjusted) | |||
| $0.leading.trailing.equalToSuperview() | |||
| $0.height.equalTo(2000) | |||
| $0.height.equalTo(UIScreen.main.bounds.height - 150) | |||
|
|
||
| import SnapKit | ||
|
|
||
| class MyPageAccountInfoTableViewCell: UITableViewCell { |
| rootView.myPageContentViewController.homeCollectionView.isUserInteractionEnabled = false | ||
|
|
||
| yOffset = -(navigationBarHeight + statusBarHeight) | ||
| rootView.segmentedControl.frame.origin.y = yOffset + statusBarHeight + navigationBarHeight | ||
| rootView.segmentedControl.snp.remakeConstraints { | ||
| $0.top.equalTo(rootView.myPageProfileView.snp.bottom) | ||
| $0.leading.trailing.equalToSuperview() | ||
| $0.height.equalTo(54.adjusted) | ||
| } | ||
|
|
||
| rootView.pageViewController.view.snp.remakeConstraints { | ||
| $0.top.equalTo(rootView.segmentedControl.snp.bottom).offset(2.adjusted) | ||
| $0.leading.trailing.equalToSuperview() | ||
| let navigationBarHeight = self.navigationController?.navigationBar.frame.height ?? 0 | ||
| $0.height.equalTo(UIScreen.main.bounds.height - statusBarHeight - navigationBarHeight - self.tabBarHeight) | ||
| } | ||
| } else if yOffset >= (rootView.myPageProfileView.frame.height - statusBarHeight - navigationBarHeight) { | ||
| rootView.segmentedControl.frame.origin.y = yOffset - rootView.myPageProfileView.frame.height + statusBarHeight + navigationBarHeight | ||
| rootView.segmentedControl.snp.remakeConstraints { | ||
| $0.top.equalTo(rootView.segmentedControl.frame.origin.y) | ||
| $0.top.equalTo(rootView.myPageProfileView.snp.bottom) | ||
| $0.leading.trailing.equalToSuperview() | ||
| $0.height.equalTo(54.adjusted) | ||
| } | ||
|
|
||
| rootView.pageViewController.view.frame.origin.y = yOffset - rootView.myPageProfileView.frame.height + statusBarHeight + navigationBarHeight + rootView.segmentedControl.frame.height | ||
|
|
| // | ||
| //extension ExampleViewController: UICollectionViewFlowLayout { | ||
| // | ||
| //} |
There was a problem hiding this comment.
코드 리뷰를 해드리겠습니다:
SnapKit라이브러리를 임포트하였는데, 해당 라이브러리가 프로젝트 설정에 추가되어 있는지 확인해야 합니다.titleData와infoData의 초기값이AccountInfoDummy.dummy()로 설정되어 있는데, 이 메서드나 클래스가 정의되어 있는지 확인해야 합니다.accountInfoTableView가isUserInteractionEnabled = false로 설정되어 있어 터치나 스크롤 기능이 비활성화됩니다. 필요에 따라 수정해야 합니다.moreInfoButton과signOutButton에 대한 액션 메서드가 구현되어 있지 않아서 버튼을 눌러도 아무 동작이 없습니다. 해당 버튼들에 대한 액션 메서드를 추가해야 합니다.addUnderline(to:)함수 내에서button.currentTitle속성을 사용하여 현재 버튼의 타이틀을 가져오고 있습니다. 이때 버튼의 타이틀이 nil일 경우를 처리하는 로직이 필요합니다.backButtonTapped()메서드 이름이 적절하지 않습니다. "Back" 버튼을 탭할 때 실행되므로backButtonPressed()와 같은 이름으로 변경하는 것이 좋습니다.
이상입니다! 코드 리뷰 내용에 대한 이해 및 수정이 필요한 부분을 확인하시고 코드를 개선해 나가시기 바랍니다.
| scrollView.isScrollEnabled = true | ||
| } else { | ||
| rootView.myPageContentViewController.homeCollectionView.isScrollEnabled = true | ||
| rootView.myPageContentViewController.homeCollectionView.isUserInteractionEnabled = true | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
이 코드 패치에는 몇 가지 결함과 개선 사항이 있습니다.
-
setUI메서드의 맨 처음에 있는self.view.backgroundColor = .donBlack은 이미 MyPageView에서 설정하고 있으므로 중복입니다. 이 줄을 제거해도 됩니다. -
setUI메서드에서self.navigationItem.title = StringLiterals.MyPage.MyPageNavigationTitle과self.navigationController?.navigationBar.titleTextAttributes = [.foregroundColor: UIColor.donWhite]코드를 사용하여 타이틀을 설정하는데, 다른 ViewController가 하나의 네비게이션 컨트롤러와 함께 사용될 경우 문제가 발생할 수 있습니다. 대신self.title = StringLiterals.MyPage.MyPageNavigationTitle을 사용하는 것이 좋습니다. -
viewWill/WillDisappear및viewDidLayoutSubviews에서 statusBarView를 추가하고 제거하는 부분이 있는데, 이렇게 하는 대신 한 번만 초기화하고 속성으로 유지하는 것이 좋습니다. 예를 들어,statusBarView를 클래스의 프로퍼티로 만들고loadView에서 초기화하고viewWillAppear에서 addSubview 및viewWillDisappear에서 removeFromSuperview를 호출하는 방식으로 변경할 수 있습니다. -
viewDidLayoutSubviews의let safeAreaHeight = view.safeAreaInsets.bottom와let tabBarHeight: CGFloat = 70.0은 사용되지 않으며 삭제해야 합니다. -
scrollViewDidScroll에서rootView.segmentedControl.snp.remakeConstraints내부의top제약 조건 설정은 이미 한 번 수행되었으므로 다시 설정할 필요가 없습니다. 이 부분을 삭제해도 됩니다. -
scrollViewDidScroll에서rootView.pageViewController.view.snp.reMakeConstraints가 중복되어 있습니다. 이 제약 조건이setLayout에 이미 있기 때문에 삭제하거나,setLayout을 호출하여 좌표 계산을 모두 그곳으로 이동시킬 수 있습니다.
…-tBe/Don-tBe-iOS into feat/#54-accountInfoView
👻 PULL REQUEST
💻 작업한 내용
💡 참고사항
📸 스크린샷
📟 관련 이슈