-
Notifications
You must be signed in to change notification settings - Fork 26
AI 챗봇 앱 [STEP 3] Paul & Kyle #50
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
base: d_Kyle
Are you sure you want to change the base?
Conversation
Mino777
left a comment
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.
안녕하세요!! 카일, 폴!
STEP 2 진행하시느라 정말 수고 많으셨습니다!!🥳
짧은 시간동안 기능 구현 열심히 해주신게 보이네용ㅎㅎ
질문 주신 부분들중에 메세지로 답변이 없는 부분은 코드 코멘트에 달아놓았으니, 참고 하시면 좋을 것 같습니다~!
고민 및 질문
시뮬레이터 테스트 관련 질문
이 부분은 환경에 따라 다를 것 같습니다ㅎㅎ 저 같은 경우 보안상 시뮬레이터에서 빌드가 되지 않도록 하는 솔루션 프레임워크를 사용하다보니 시뮬레이터로는 빌드 자체가 되지 않아서 강제로 실제 디바이스로만 테스트가 가능합니다ㅎㅎ 그런데 결국엔 실제 디바이스 테스트가 가장 중요하기 때문에 실제 디바이스는 필수적이라고 생각하시면 좋을 것 같습니다👍
말풍선 애니메이션 레이아웃 구현 실패
해당 부분은 사실 시간이 부족해서 구현을 못하신걸로 보입니다ㅎㅎ 나중에 시간 되시면 좀 더 학습하셔서 구현 해보시면 간단하게 구현할 수 있으니, 학습 해보시면서 추후에 구현해보시는게 좋을 것 같습니다. 구현 하시다가 막히시면 그때 그때 질문 주세요💪
아 그리고 추가적으로 보통의 경우 이러한 애니메이션의 경우 Lottie라는 라이브러리를 대중적으로 사용합니다. 물론 Lottie 디자인의 경우 디자인 부서에서 담당하구요!
| func configureUI() { | ||
| view.backgroundColor = .white | ||
| view.addSubview(chatCollectionView) | ||
| view.addSubview(chatInputView) | ||
| } | ||
|
|
||
| func setupCollectionView() { | ||
| self.chatCollectionView.dataSource = dataSource | ||
| } | ||
|
|
||
| func setupConstraints() { | ||
| chatInputView.translatesAutoresizingMaskIntoConstraints = false | ||
| chatCollectionView.translatesAutoresizingMaskIntoConstraints = false | ||
|
|
||
| NSLayoutConstraint.activate( | ||
| [ | ||
| chatCollectionView.leadingAnchor.constraint(equalTo: view.leadingAnchor), | ||
| chatCollectionView.topAnchor.constraint(equalTo: view.safeAreaLayoutGuide.topAnchor), | ||
| chatCollectionView.trailingAnchor.constraint(equalTo: view.trailingAnchor), | ||
| chatCollectionView.bottomAnchor.constraint(equalTo: chatInputView.topAnchor, constant: -10), | ||
| chatInputView.leadingAnchor.constraint(equalTo: view.leadingAnchor), | ||
| chatInputView.trailingAnchor.constraint(equalTo: view.trailingAnchor), | ||
| chatInputView.bottomAnchor.constraint(equalTo: view.safeAreaLayoutGuide.bottomAnchor) | ||
| ] | ||
| ) | ||
| } | ||
|
|
||
| func setupChatInputView() { | ||
| chatInputView.setChatSendButton { [weak self] message in | ||
| self?.input.send(.sendButtonTapped(message: message)) | ||
| guard let requestDTO = self?.chatBotViewModel.requestDTO else { return } | ||
| self?.applyChatResponse(response: requestDTO) | ||
| } | ||
| } |
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.
해당 ViewController의 슈퍼뷰까지 별도의 객체로 사용해서
ViewController에는 레이아웃 관련 셋업 로직이 없도록 할 수 도 있습니다ㅎㅎ
private lazy var mainView ~
override func loadView() {
super.~
view = mainView
}
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.
ChattingView로 상위 View를 생성하여 구현했습니다! 확실히 ViewController의 로직이 깔끔해진 것 같습니다.
override func loadView() {
super.loadView()
self.view = chattingView
setupKeyboardNotification()
bind()
setupChatInputView()
}리팩토링하면서 NSDiffableDataSourceSnapshot을 apply하는 책임은 ViewController여야할 지, ChattingView여야할 지 고민했습니다. 저희는 해당 CollectionView를 소유한 부분에서 구현하는 것이 맞지 않을까 판단했는데 마이노의 의견은 어떤 지 궁금합니다!
func applyChatResponse(response: [RequestDTO]) {
var chatCollectionViewSnapshot = ChatCollectionViewSnapshot()
chatCollectionViewSnapshot.appendSections([.messages])
chatCollectionViewSnapshot.appendItems(response)
dataSource.apply(chatCollectionViewSnapshot)
chatCollectionView.scrollToBottom()
}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.
View는 addSubView, autolayout, 각 component들의 속성 변경 등 Layout과 관련된 책임만 가져가시는게 좋습니다.
따라서 Datasource와 관련된 로직은 ViewController가 가져가는게 확실하게 분리되겠죠?!
ViewController에 해당 view.collectionView로 프로퍼티를 하나 정의해놓고,
ViewController에서 해당 Datasource 로직을 수행하시면 좋을 것 같습니다ㅎㅎ
| @objc | ||
| private func keyboardWillShow(notification: NSNotification) { | ||
| guard | ||
| let keyboardSize = (notification.userInfo?[UIResponder.keyboardFrameEndUserInfoKey] as? NSValue)?.cgRectValue | ||
| else { | ||
| return | ||
| } | ||
|
|
||
| self.view.frame.size.height -= keyboardSize.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.
보통 키보드 레이아웃 조절 시 view의 frame 자체를 움직이기 보단, scrollView의 contentInset을 조절해서 레이아웃을 구현하는 것이 좋습니다.
view의 frame 자체 직접 조절하게 되면, autolayout이 깨질 수 있고, 여러분들이 질문주신 것 처럼, 예상치 못한 UI와 관련된 문제들이 발생할 수 있습니다.
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.
현재 ChattingView(UIView) 안에 CollectionView와 ChatInputView가 add 되어 있습니다. contentInset을 활용하기 위해선 UIScrollView를 상속한 View에서 작업이 이루어져야 하는데 그렇다면 ChattingView를 UIView가 아닌 UIScrollView로 구현하거나 CollectionView 안에 ChatInputView를 add 해줘야 할까요??
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.
ChattingView를 UIScrollView로 구현해보시면 좋을 것 같습니다ㅎㅎ
| final class UserBubbleView: ChatBubbleView { | ||
| override func draw(_ rect: CGRect) { | ||
| setRightBubbleView(rect: rect) | ||
| } | ||
| } | ||
|
|
||
| final class SystemBubbleView: ChatBubbleView { | ||
| override func draw(_ rect: CGRect) { | ||
| setLeftBubbleView(rect: rect) | ||
| } | ||
| } |
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.
좌우 정렬만 자식 클래스에서 진행하고, 공통되는 draw 로직은 부모클래스에서 진행하면 중복 코드들을 줄일 수 있을 것 같아요!
그리고 상속이 아닌, protocol로 추상화 해줘도 되겠네요!
개인적으로 상속은 불가피한 상황이 아니면 최대한 유연하고 안전하게 설계 및 사용할 수 있도록 protocol을 사용하는 편이에요.
나중에 프로젝트가 커지고, 상속을 남발하다보면 관리도 힘들고, 디버깅도 힘들어질 수 있는 상황이 오게됩니다ㅎㅎ
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.
아래와 같이 수정했습니다!
protocol ChatBubbleMakable where Self: UIView {
var messageView: MessageView { get }
func configureUI()
func setupConstraints()
func setRightBubbleView(rect: CGRect)
func setLeftBubbleView(rect: CGRect)
func configureMessage(text: String)
}| } | ||
|
|
||
| func setRightBubbleView(rect: CGRect) { | ||
| let bezierPath = UIBezierPath() |
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.
이렇게 메서드 내에만 있고, 많이 타이핑이 되어야 하는 프로퍼티는 꼭 정직하게 정의해줄 필요는 없습니다!
let path = 요렇게만 해도 타이핑이 많이 줄어들 것 같네용ㅎㅎ
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.
수정했습니다!
| self.register(ChatCell.self, forCellWithReuseIdentifier: ChatCell.identifier) | ||
| } | ||
|
|
||
| func srollToBottom() { |
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.
수정했습니다! 감사합니다
| extension ChatCollectionView { | ||
| func setupChatCell() { | ||
| self.register(ChatCell.self, forCellWithReuseIdentifier: ChatCell.identifier) | ||
| } |
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.
class 메인 구현부와 extension 구현부에 메서드를 위치시키는 여러분들의 기준점은 무엇인가요?! 설명해주세요!
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 메서드는 private extension에서 구현하고 internal 메서드는 class 메인 구현부에 위치시키고자 했습니다!
|
|
||
| import UIKit | ||
|
|
||
| final class ChatCell: UICollectionViewListCell { |
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.
말씀해주신 CollectionViewCell 의 height를 dynamic하게 계산하지 못하는 이슈의 경우,
label의 content size가 계산되기 전에 cell이 재사용되서 발생하는 이슈로 보입니다.
힌트를 드리자면, prepareForReuse 메서드를 공부해보시면 좋을 것 같습니다👍
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.
구현 당시 prepareReuse()를 활용하는 방법도 고민해보았는데 dequeReusableCell를 사용했을 때, 재사용될 때마다 초기화 해준다면 재사용 셀을 사용하는 의미가 조금 퇴색될 수 있다고 판단했었습니다.
아래와 같이 구현했을 때는 메세지 뷰의 text만 초기화 시켜주는 로직인데 해당 부분이 재사용 셀을 활용하는데 비효율적인 방법일 지 마이노의 의견이 궁금합니다!
override func prepareForReuse() {
super.prepareForReuse()
configureUser(text: "")
configureSystem(text: "")
}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.
음, 재사용 대상에 대한 개념을 조금 다르게 이해하신 것 같아요!
Cell을 재사용한다는건, 큰 틀의 Cell을 재사용하는것이지, Cell 안의 값들까지 재사용하는건 아니니까요! (물론 동일한 내용을 계속 보여주는게 목적이라면 그럴수 도 있지만, 그런 경우는 없으니까요ㅎㅎ)
셀의 내용을 초기화해주는건 재사용의 의미를 퇴색시키기보다는 오히려 효율적으로 셀을 관리하는 방법이지 않을까요? 초기화 하지 않고 이전 Cell의 내용에 영향을 받아서 잘 못 동작하는게 더 재사용의 의미를 퇴색시키겠죠?!
넵 현재 문제가 되는게, label의 text값이 변경되기 전에 layout이 잡히면서 발생하는 문제이기 때문에 text값만 초기화해주어도 될 것 같습니다👍
|
|
||
| import UIKit | ||
|
|
||
| final class MessageView: UILabel { |
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.
UILabel인데 ~View 네이밍은 혼란은 줄 수 있을 것 같아요!
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.
MessageLabel로 수정했습니다!
Mino777
left a comment
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.
STEP3 도 정말 고생많으셨습니다💪💪
코멘트 드린 부분들 확인 후 노티주시면 머지하도록 하겠습니다!
고생하셨어요!!💪💪
AI 챗봇 앱 [STEP 3] Paul & Kyle
멤버 : @earths-voluble, @Changhyun-Kyle
리뷰어 : @Mino777
AI 챗봇 앱 세 번째 Step입니다!
정리하여 PR 드립니다. 감사합니다!
🤔 고민한 점
UICollectionViewDiffableDataSourceUICollectionViewDiffableDataSource를 활용했습니다.UICollectionViewDiffableDataSource는NSDiffableDataSourceSnapshot을 활용하여 데이터의 변화를 감지합니다.NSDiffableDataSourceSnapshot의identifierType은Hashable을 채택해야하며Int타입의 고유한 식별자를 생성하여 타입의 변화를 인지합니다.RequestDTOUICollectionViewDiffableDataSource를 활용하기 위해선 데이터 모델에Hashable을 채택해야했습니다. 하지만, 기존OpenAI의RequestModel에Hashable을 채택 시 동일한 질문을 보냈을 때 고유한 식별자가 중복되기 때문에 새로운DTO계층이 필요하다고 생각했습니다.따라서,
RequestDTO를 생성 후UUID타입의id프로퍼티를 생성하여 각 메세지가 고유한UUID타입의 난수를 가질 수 있게 구현했습니다.BezierPath를 활용하여ChatBubbleView구현 후 서브클래싱이번
STEP에서 정말 많은 시간을 할애했던말풍선 View를BezierPath를 활용하여 구현했습니다.말풍선 View는 사용자의 요청과 시스템의 응답으로 총 2가지가 필요했습니다.따라서,
ChatBubbleView라는 상위 클래스를 생성 후UserBubbleView와SystemBubbleView에서 상속 받아 코드의 재사용성을 높였습니다.네트워크 응답 대기 시 임의의
말풍선 View생성말풍선 애니메이션을 구현하기 위해선 임의의
말풍선 View가 필요했습니다.요청에 대한 응답은 무조건 가장 마지막에 보여지기 때문에 다음과 같이 로직을 구현했습니다.
사용자가 메세지를 보낼 때
content가 비어있는Message모델을 데이터에 반영하여cell을 생성합니다. 이후, 응답이 왔을 때 가장 마지막에 있던content가 비어있는Message모델을 삭제 후 응답 받은 데이터 모델을 추가하여 해당View를 대체합니다.🧐 궁금한 점
말풍선 재사용 시 레이아웃이 정상적으로 그려지지 않는 이슈
ChatCollectionView의 셀이 재사용되는 과정에서 셀을 재사용 시 완전히 새로 그리지 않고, 재사용 이전의 셀의height를 그대로 가져오는 듯한 이슈가 발생했습니다. 이번 스텝에서 저희를 사실상 처음부터 끝까지 괴롭혔던 이슈인데, 결국은 해결하지 못하고 원인도 파악하지 못했습니다. 이러한 문제의 원인 혹은 적절한 구현 방법이 궁금합니다!시뮬레이터에서 키보드 레이아웃 이슈
키보드 호출 시 레이아웃을 조절하기 위해 구현한
keyboardWillShow및keyboardWillHide메서드가 두 번씩 호출되는 듯한 현상입니다. 이 역시 원인을 파악하기 위해 여러 방법을 시도해 보았는데, 결국 실기기에서는 정상적으로 동작하고, 시뮬레이터에서만 발생하는 이슈임을 확인했습니다.코딩을 하다 보면 기기에서는 발생하지 않고 시뮬레이터에서만 발생하는 이슈가 심심찮게 있다고 느꼈는데요. 저희도 해당 문제로 아까운 시간을 허비하고 나니 확실히 실기기에서의 테스트가 신뢰성이 높다고 느꼈습니다. 현업에서도 시뮬레이터보다는 실기기 테스트를 선호하는지도 궁금해졌습니다!
말풍선 애니메이션 레이아웃 구현 실패
말풍선 뷰 안에 점 세 개가 움직이는
CoreAnimation객체를 넣어 구현하고자 하였으나, 이 역시 제대로 표시되지 않고 레이아웃이 깨져 버리는 현상을 해결하지 못하고 미완성된 말풍선으로 대체했습니다. 이와 같은 애니메이션을 적절하게 구현하기 위한 방법이 궁금합니다!