-
Notifications
You must be signed in to change notification settings - Fork 5
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
[FEAT] #46 - 명함 그룹화면 구현 #84
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.
기다렸습니다 이준쌤⭐️
고생하셨습니다 ^___^
DBBB91E1639641F40C5B4416 /* Pods_NADA_iOS_forRelease.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 4A2183AE0E469153221624A0 /* Pods_NADA_iOS_forRelease.framework */; }; | ||
F811720027383097002742CF /* ChangeGroupRequest.swift in Sources */ = {isa = PBXBuildFile; fileRef = F81171FF27383097002742CF /* ChangeGroupRequest.swift */; }; | ||
F81A6833274F49A700B80A4F /* UITextField+Extension.swift in Sources */ = {isa = PBXBuildFile; fileRef = F81A6832274F49A700B80A4F /* UITextField+Extension.swift */; }; |
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.
어라라라ㅏ 혹쉬 이게 빠져있는거는 위치가 변경되서 그런걸까요...?
아니면 pull을 안받아서일까요..?
이준교수님 xcode에서는 잘 되는건가욥..?
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.
UITextField+Extension 파일을 민재랑 내가 동시에 만들어서 내가 삭제해줘서 그런거 같네요1
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.
아하... 글면 괜찮을거 같슴다..!
@@ -17,6 +17,7 @@ extension Const { | |||
static let frontViewController = "FrontViewController" | |||
static let groupViewController = "GroupViewController" | |||
static let groupEditViewController = "GroupEditViewController" | |||
static let qrScanViewController = "QRScanViewController" |
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.
바텀시트도 Constants에 같이 추가해두면 좋을 것 같습니다 선배님!
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.
오우 굿포인트 지금 바로 추가할게여!!!
@@ -72,6 +72,7 @@ class CommonBottomSheetViewController: UIViewController { | |||
let label = UILabel() | |||
label.font = .systemFont(ofSize: 20, weight: .bold) | |||
label.textAlignment = .center | |||
label.sizeToFit() |
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.
민재가 너무 잘만들어놔서 코드 짜기 너무 편했심다^__^
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.
일단 46번을 닫기 위함이지만 혹시나 놓치신부분이 있을까봐! 챙겨봤습니다...
주석은 나중에 개발하시면서 필요없어지면 지워주세요!
수고하셨어용 이준선배!
return UINib(nibName: "GroupCollectionViewCell", bundle: nil) | ||
} |
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.
요것도 Const Xib 에서 만들어서 쓰시면 좋을거 같아요!
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.
아하 xib에 추가를 해놓긴했는데 여기선 const로 안썼네요... 알겠슴돠!!
if indexPath.row == 0 { | ||
// groupCell.groupName.textColor = .background | ||
// groupCell.groupBackground.backgroundColor = .primary | ||
groupCell.isSelected = true | ||
} else { | ||
groupCell.isSelected = false | ||
} | ||
groupCollectionView.layoutIfNeeded() |
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.
처음 뷰를 봤을 때 미분류의 경우 selected 된 듯한 효과가 안나오더라구요
요거 collectionView 에 selectItem 에 넣어주지 않아서 그런거라
collectionView.selectItem(at: indexPath, animated: true, scrollPosition: .init())
isSelected 코드 없이 0일 경우에만 위의 코드를 추가해주시면 작동합니당 아 그리고 257 번줄은 지금은 필요는 없을거 같아여
cell 에서 didset 으로 반영해주기 때문이죵
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.
헐 안그래도 이거 이슈파서 고치려고 했는데 감삼돠 교수님!!
func collectionView(_ collectionView: UICollectionView, layout collectionViewLayout: UICollectionViewLayout, minimumInteritemSpacingForSectionAt section: Int) -> CGFloat { | ||
return 0 | ||
} | ||
func collectionView(_ collectionView: UICollectionView, layout collectionViewLayout: UICollectionViewLayout, minimumLineSpacingForSectionAt section: Int) -> CGFloat { | ||
switch collectionView { | ||
case groupCollectionView: | ||
return 5 | ||
case cardsCollectionView: | ||
return 14 | ||
default: | ||
return 0 | ||
} | ||
} |
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.
groupCollectionView 의 경우는 linespacing 이 필요없으니깐 0 으로도 해도될거같구 interitemspacing 이 필요해보이네요!
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.
요게 말입니다... 저도 원래 옆간격이 interitemspacing이고 위아래 간격이 linespacing이라고 알고 있는데,
groupCollectionview가 scrolling 방향이 horizontal이라 그런지 옆간격을 설정해주려면 linespacing을 해야 먹더라구요...?
저도 싱기했습니다ㅏㅏㅏ.....
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.
흠 저도 이 기억이 있는데 왜 따닥따닥.. 선배가 되었을까요 혹시 간격 제대로 적용되나요?
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.
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.
헐 교수님 이거 collectionview estimated size를 none으로 하니까 옆간격이 interitemspacing이 옆간격인가봐요 이거뭐죠? ㅋㅋㅋ.ㅋㅋ.ㅋ.ㅋ.ㅋ...ㅋ..ㅋ....ㅋ..ㅋ....ㅋ.이게뭐지..... 이상허다...
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.
어라ㅏㄹ...? 이상허다 저는 line 으로 잡아야 넓어지던데..?
extension GroupViewController: UICollectionViewDelegateFlowLayout { | ||
func collectionView(_ collectionView: UICollectionView, layout collectionViewLayout: UICollectionViewLayout, sizeForItemAt indexPath: IndexPath) -> CGSize { | ||
|
||
var width: CGFloat | ||
var height: CGFloat | ||
|
||
switch collectionView { | ||
case groupCollectionView: | ||
if groups[indexPath.row].count > 4 { | ||
width = CGFloat(groups[indexPath.row].count) * 16 | ||
} else { | ||
width = 62 | ||
} | ||
height = collectionView.frame.size.height |
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.
지금 인하대학교가 끝까지 안보이는 이슈가 있는데요ㅜ
컬렉션뷰의 estimate size 를 none
으로 하지 않아서 발생하는거 같아요!
인퍼테이스빌더로 해줘도 되고 코드로
let cardCreationCollectionViewlayout = cardCreationCollectionView.collectionViewLayout as? UICollectionViewFlowLayout
cardCreationCollectionViewlayout?.estimatedItemSize = .zero
이렇게 설정해주어도 된답니다!
설정해주면 interitemspacing
를 0으로 했기 때문에
따닥붙고 인하대학교까지 보일거에요! 기존에는 automatic 이었기 때문에 간격도 자동으로 벌어졌는데 끝에가 안보였던거 같네용
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.
4글자까지는 정적인 사이즈로 조절했는데 혹시 이유가 있을까요?!
그리고 글자수대로 width 를 잡아도 좋긴한대 다음과 같이 짜도 좋을거 같아요! 아래는 1글자에 대한 크기도 대응을 해주었습니당!
// ✅ 20(여백), 폰트사이즈 system 14기준
let cellSize = CGSize(width: topicList[indexPath.item].size(withAttributes: [NSAttributedString.Key.font : UIFont.systemFont(ofSize: 14)]).width + 20, height: 30)
return cellSize
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.
어머어머 estimated size까지 신경써주시는 세심함... 감삼다 덕분에 해결됐어요!!!
아 그리고 4글자까지 정적인 사이즈로 조절한 이유는 1-4글자까지 글자수로 대응하면 너무 터치되는 버튼 사이즈가 작아서 불편하더라구요.. 그래서 제플린을 보니 지금 미분류, SOPT 버튼의 크기를 반영해서 4글자까지는 정적으로 대응했습니다!!
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.
아아 좋네요! 이유가 있었군요 저도 적용해보겠습니다 담번에 터치영역까지 고려한 ㄷ ㄷ
🌴 PR 요약
🌱 작업한 브랜치
🌱 작업한 내용
📸 스크린샷
📮 관련 이슈