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

[ADD] 홈뷰, 하단 테이블 뷰 개발 #88

Merged
merged 26 commits into from
Feb 21, 2023

Conversation

hongjunehuke
Copy link
Collaborator

@hongjunehuke hongjunehuke commented Feb 19, 2023

📣 Related Issue

👩‍💻 Contents & Screenshot

스크린샷 2023-02-19 오후 5 12 53스크린샷 2023-02-19 오후 5 11 25스크린샷 2023-02-20 오전 12 14 25스크린샷 2023-02-20 오전 12 05 22

  • 기존 컬렉션 뷰이었던 하단 집안일 뷰를 테이블 뷰로 전환했습니다.
  • 끝낸 집안일에 대한 헤더 '끝낸 집안일'을 넣어 두었습니다.
  • 기존에 스크롤 이벤트는 스크롤 시작과 함께 주간 달력이 가려졌지만, 해당 부분 수정하여 하단 컬렉션 뷰를 스크롤 시 주간 캘린더는 따로 스크롤 되어 숨겨지지 않습니다.
  • 집안일 멤버 아이콘을 클릭했을 때 selected 이미지로 변경됩니다.
  • 집안일 멤버 아이콘의 selected 버전은 하나의 아이콘에 해당되는 asset만 일단 넣어두었습니다.

📌 Review Point

  • 테이블 뷰의 layoutSubviews()를 호출해도 셀끼리의 간격이 떨어지지 않아, 테이블 뷰 섹션에 하나의 셀을 넣어 처리했습니다.
  • 테이블 셀의 수평 스크롤이 시뮬에서 했을 때는 가끔 제스처 인식을 못하는것 같은데, 혹시 실제 디바이스에서도 그러니지 모르겠네요. 실제 디바이스에서도 제스처 인식이 잘 안된다면, 따로 제스처를 넣을 생각입니다.

🧐 Question

  • 지금은 하단 테이블 뷰가 각 셀이 아닌, 각 섹션으로 개발되어 있습니다.
  • 섹션으로 개발되었지만, 서버 데이터를 바인딩 하는 등 셀로 개발했을 때와의 차이는 크게 없습니다. indexPath.section 를 활용하면 되더라고요! 하지만, 섹션과 셀의 성격을 생각했을 때 섹션보다는 셀로 개발되는게 더 맞는걸까요?
  • 섹션으로 개발되었을 때, 셀의 간격을 떨어뜨리는 부분이 해결되고 수평 제스처 이후 셀의 layer가 고정된다는 부분에서 이점이 있습니다. 다른 분들 생각이 궁금합니다!

@hongjunehuke hongjunehuke changed the title Feature/80 home table view logic [ADD] 홈뷰, 하단 테이블 뷰 개발 Feb 19, 2023
@hongjunehuke hongjunehuke self-assigned this Feb 19, 2023
Comment on lines 16 to 19
if isSelected {
self.titleImage.image = ImageLiterals.profilelightblue1Selected
}else {
self.titleImage.image = ImageLiterals.profileLightBlue1
Copy link
Contributor

Choose a reason for hiding this comment

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

p1;
이미지 종류가 16개라서 isSelected 된 이미지 16개를 더 넣는 것보다,
isSelected 인 경우 radius 가 24인 파란색 원의 hidden 처리를 없애주는 게 더 좋지 않을까 싶어요!

OnboardingProfileGroupCollectionViewCell 파일 참고하시면 imageView 위에 checkCircleView를 올려 선택 여부에 따라 원을 올리는/제거하는 로직을 구현해주면 될 것 같아요!

}
}
}

// MARK: - property

let titleImage: UIImageView = {
Copy link
Contributor

Choose a reason for hiding this comment

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

p1;
이건 let titleImage = UIImageView()로 간단하게 표현할 수 있을 것 같아요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

// MARK: - property

let titleImage: UIImageView = {
let imageView = UIImageView()
imageView.image = ImageLiterals.profileLightBlue1
return imageView
}()
let titleLabel: UILabel = {
Copy link
Contributor

Choose a reason for hiding this comment

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

p1;
여기 폰트 등록된 이후로 수정 필요해서 caption1으로 수정 부탁드립니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@@ -42,4 +51,12 @@ final class HomeGroupCollectionViewCell: BaseCollectionViewCell {
$0.leading.trailing.equalToSuperview()
Copy link
Contributor

Choose a reason for hiding this comment

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

p1;
위에 top 부분 수정이 필요해요!
offset을 4로 수정 부탁드립니다!!!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Comment on lines +84 to +85
if cell.isSelected == true { cell.onSelected() }
else { cell.onDeselected() }
Copy link
Contributor

Choose a reason for hiding this comment

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

여기서 cell.titleLabel 지정해준 것처럼 cell.titleImage이 나중에 지정되면 이 로직은 필요없을 것 같아요!
지금은 titleImage가 property 생성할 때 지정되지 않아서 위 로직이 없으면 첫 cell을 제외하고 다른 이미지들을 보이지 않는데
나중에는 titleLabel과 동일하게 titleImage도 userList에서 불러올 거라서 이미지가 처음에 보이지 않는 문제가 없을 겁니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

어... 근데 그럼 아래 willDisplay 함수로 처음 셀 포커스와 이후 스크롤로 가려졌다가 보였을 때 포커스되는 셀을 지정하는 로직인데, 위에 유나님이 말씀하신 코드 없이는 마지막 인덱스 포커스했을 때 가려졌다가 보여지면 포커스가 해제되는 버그가 있어 넣어두었습니다!

func collectionView(_ collectionView: UICollectionView, willDisplay cell: UICollectionViewCell, forItemAt indexPath: IndexPath) {
        let selectedIndexPath = IndexPath(row: self.selectedIndex, section: 0)
        collectionView.selectItem(at: selectedIndexPath, animated: false, scrollPosition: .init())
    }

@Guel-git
Copy link
Contributor

p1;
지금 피알에 해당되지 않지만 Home 마무리 피알이라 수정이 필요해서 요청드려요!
피그마와는 다른 크기들이 좀 있어서 수정해봤어요! 둘다 HomeVC에 있는 내용이고 아래 캡처에 수정된 부분만 고쳐주시면 감사하겠습니다!
스크린샷 2023-02-20 오후 3 55 38
스크린샷 2023-02-20 오후 3 56 08

$0.top.equalToSuperview().offset(SizeLiteral.componentPadding)
$0.leading.equalToSuperview().offset(SizeLiteral.componentPadding)
}

workerStackView.snp.makeConstraints {
Copy link
Contributor

Choose a reason for hiding this comment

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

p3;
지금은 worker를 세명만 넣어서 stackView에 넣어주셨는데
api 연결할 때는 table이나 collection view로 교체해주셔야 할 것 같아요!!!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

넵! 확인했습니다! 해당 부분 collection view로 교체해 둘게요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@Guel-git
Copy link
Contributor

Guel-git commented Feb 20, 2023

p1;
또 놓친 부분이 있네요,, 전에 리뷰를 제대로 했어야 하는데,,,
selected 된 cell의 dateLabel & dayLabel은 .title2가
deselected 된 cell 의 dateLabel & dayLabel은 .body2입니다!

스크린샷 2023-02-20 오후 4 27 17

이렇게 변경하게 되면 homeWeekCalendarView와 calendarDailyTableView 사이의 간격이 너무 떨어져서
homeWeekCalendarView의 height를 $0.height.equalTo(80) 이 정도로 줄여도 좋을 것 같아요!

@Guel-git
Copy link
Contributor

p1;
놀랍게도 CalendarDailyTableViewCell에 shadow가 있습니다..!
tableviewcell은 아마 바로 shadow 적용이 안될거에요! (maskToBounds 관련)

그래서 뒤에 shadow가 적용된 shadow view를 하나 만들어서 그 위에 나머지 component들을 올려주시면 잘 동작할 겁니다!
스크린샷 2023-02-20 오후 4 58 37

@hongjunehuke
Copy link
Collaborator Author

코드 리뷰에 따른 반영해서 올려두었습니다!

let titleImage = UIImageView()
let checkCircleView: UIView = {
let view = UIView()
view.layer.borderWidth = 1.5
Copy link
Contributor

Choose a reason for hiding this comment

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

p1;

        view.layer.borderWidth = 1

피그마 상으로는 1인 것 같아요!!

}
checkCircleView.snp.makeConstraints {
$0.top.equalToSuperview()
$0.centerX.equalToSuperview()
Copy link
Contributor

Choose a reason for hiding this comment

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

p4;

수정 전 수정 후

요렇게 하면 조금 더 중앙정렬,,, 이 됩니다...!

            $0.center.equalToSuperview()

Comment on lines 89 to 92
func collectionView(_ collectionView: UICollectionView, willDisplay cell: UICollectionViewCell, forItemAt indexPath: IndexPath) {
let selectedIndexPath = IndexPath(row: self.selectedIndex, section: 0)
collectionView.selectItem(at: selectedIndexPath, animated: false, scrollPosition: .init())
}
Copy link
Contributor

Choose a reason for hiding this comment

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

p2;
앗 이렇게 하기보다는
cellForItemAt 부분에

if selectedIndex == indexPath.item {
    cell.isSelected = true
    collectionView.selectItem(at: indexPath, animated: false, scrollPosition: .init())
}

여기에 이렇게 추가해주시면 될 것 같아요!
Cell에서 circleView 선언할 때 isHidden = true로 해주시구요!

Comment on lines 17 to 18
checkCircleView.isHidden = false
self.bringSubviewToFront(titleImage)
Copy link
Contributor

Choose a reason for hiding this comment

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

p2;
요기는 밑에서 만들어 두신
onSelectedonDeselected 사용해주시면 될 것 같습니다!

@hongjunehuke hongjunehuke merged commit 452d441 into develop Feb 21, 2023
@hongjunehuke hongjunehuke deleted the feature/80-Home-TableView-Logic branch February 21, 2023 13:28
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.

[ADD] 홈뷰, 하단 컬렉션 뷰 개발
2 participants