-
Notifications
You must be signed in to change notification settings - Fork 0
[Feat] #563 - 채팅 로그 뷰에서 오브의 추천소 접근 경로 구현 #564
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
Conversation
그 외 기타 사소한 코드 개선
Johyerin
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.
작업 내용 잘 적용된 것 확인했습니다! 고생하셨습니다~~
| extension CharacterChatMessageItem { | ||
|
|
||
| /// 화면에 표시되는 날짜 문자열 | ||
| var formattedDateString: String { | ||
| let formatter = DateFormatter() | ||
| formatter.locale = Locale(identifier: "ko_KR") | ||
| formatter.dateFormat = "M월 d일 EEEE" | ||
| return formatter.string(from: createdDate) | ||
| } | ||
|
|
||
| /// 화면에 표시되는 채팅 아이템의 시간 문자열 | ||
| var formattedTimeString: String { | ||
| let dateFormatter = DateFormatter() | ||
| dateFormatter.locale = Locale(identifier: "ko_KR") | ||
| dateFormatter.dateFormat = "a hh:mm" | ||
| return dateFormatter.string(from: createdDate) | ||
| } | ||
|
|
||
| } |
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.
P2
해당 함수들을 Date 타입의 extension으로 따로 빼서 작성해둬도 좋을 것 같습니다!
formattedTimeString 변수의 경우, CharacterChatItem에서도 동일하게 쓰이고 있기 때문에 유지보수성을 높이고 코드의 중복을 줄이기 위해서
extension Date {
var formattedTimeString: String {
let formatter = DateFormatter()
formatter.locale = Locale(identifier: "ko_KR")
formatter.dateFormat = "a hh:mm"
return formatter.string(from: self)
}
var formattedDateString: String {
let formatter = DateFormatter()
formatter.locale = Locale(identifier: "ko_KR")
formatter.dateFormat = "M월 d일 EEEE"
return formatter.string(from: self)
}
}
이런 식으로 분리해준 뒤에 item.createdDate.formattedTimeString, messageItem.createdDate.formattedDateString와 같이 접근해줘도 좋을 것 같아요!
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.
저도 Date의 extension으로 빼는 방법을 생각해 봤었는데, 이 때에는 Date에서 바로 접근이 가능해서 약간 수정이 필요합니다. 그러다 최종적으로는 현재 코드처럼 구현하는 방법으로 정했습니다.
왜냐하면 formatted(Time|Date)String 을 구현할 때 채팅 화면에서 보일 포맷을 사용하고 있기 때문인데,
ChatItem 외부로 옮기면 함수 이름만 봤을 때 정확히 어떤 포맷으로 포매팅했는지 알기 힘들고, 다른 포맷을 쓰고 싶은 경우에는 또 따로 만들어 주어야 하기 때문입니다. (확장으로 구현한 속성이나 메서드는 해당 타입의 인스턴스에 접근할 수 있으면 전역에서 언제든 사용할 수 있는 속성 혹은 메서드일텐데, 지금과 같은 경우는 채팅 화면에서 쓰일(그 중에서도 채팅 아이템에서 필요한) 특수한 경우의 포매팅이라고 봤습니다.)
즉, 현재는 ChatItem 안의 메서드라 '채팅 화면에 보일 포맷으로 포매팅된 문자열이구나' 하고 유추할 수 있으나, Date의 확장으로 옮기면 그렇지 않습니다. 이를 해결하기 위해 몇 가지 안을 생각해볼 수 있었으나, 다음의 이유들로 인해 저는 현행을 유지하는 게 더 적절하다고 생각합니다!
연산 속성의 이름을 formattedTimeStringInChatLog 와 같은 이름으로 자세하게 변경
함수 이름이 너무 길어지기도 하고, 특정 상황에서만 사용하는 경우에도 확장으로 빼는 게 적절한 지 의문
제시된 코드에서 속성 이름은 변하지 않고 별도의 주석만을 추가
그러나 특정 포맷에 국한된다는 점은 여전히 동일.
또한, 다른 포맷의 문자열을 반환하는 계산 속성을 만들 때, 지금의 이름은 너무 포괄적이라 헷갈릴 것 같다고 생각.
함수로 구현하여 매개변수로 포맷을 주입
포맷을 인자로 넣는 과정에서 잘못된 형식인 경우 런타임 오류 발생 가능.
런타임 오류 막기 위해 에러 처리 등을 구현할 경우, 불필요하게 코드가 길어질 것으로 예상.
또한, 현재로서는 다른 곳에서 비슷한 기능이 쓰일 일이 많이 없는 것 같아, 함수로 구현하는 의미가 적어짐.
추후 필요 시 구현해도 충분할 것이라 생각.
추가적인 의견이나 제안이 있으면 말씀해 주세요!
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.
확인했습니다! 의견 감사합니다 :)
codeJiwon
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.
채팅 배경의 그라데이션이 잘 적용된 것 확인했습니다.
오브의 추천소로 연결되는 동작도 흐름이 자연스럽고 매끄럽습니다!
구현하느라 고생 많으셨습니다.
| final class ChatBubble: UIView, ORBRecommendationGradientStyle { } | ||
| private(set) var chatBubble = ChatBubble() | ||
|
|
||
| private let characternameLabel = 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.
P4
변수명 컨벤션에 따라 characternameLabel은 characterNameLabel로 수정하는 것이 더 적절해 보입니다!
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.
확인 감사합니다! 수정하여 반영하겠습니다!
🌴 작업한 브랜치
✅ 작업한 내용
채팅 로그 뷰에서 오브의 추천소 접근 경로를 구현하였습니다.
isPlaceRecommendation가true인 경우, 오브의 추천소로 유도하도록 구현했습니다.CharacterChatMessageItem에orbRecommendation케이스를 추가하였습니다.이 케이스는 채팅 중 오브가 '오브의 추천소'로 이동을 유도하는 경우에 해당합니다.
CharacterChatItem.swift파일에서CharacterChatMessageItem관련 코드를 별도 파일로 분리했습니다.기타
ORBNavigationController에 변경사항이 있습니다.이는 캐릭터 채팅 로그의 배경이 자연스럽게 보이도록 하는 코드입니다.
채팅 로그는 블러 처리가 되어 반투명하게 보이도록 디자인되어있는데, 실제로 뷰가
push된 후에도 배경이 반투명하게 보이도록 하기 위해서 약간의 눈속임을 주는 중입니다. (push가 완료된 후에도 실제 투명한 게 아니라, 이전 뷰를 캡쳐하여 사용.push,pop동작 시CharacterChatLogView의backgroundView를 적절히 숨기거나 나타내도록 동작.)채팅 로그 뷰에서 오브의 추천소를
push할 때나, 오브의 추천소에서pop하여 채팅 로그 뷰로 돌아갈 때, 이backgroundView가 숨겨지면서 어색해 보이는 문제가 생겼고, 이를 해결한 코드입니다. (하단 영상 참고해주세요)이를 해결하기 위해
CharacterChatLogView의backgroundView를 숨기거나 나타내는 동작을 채팅 로그 뷰컨트롤러가 아닌,ORBNavigationController에서 구현하였습니다.❗️PR Point
그라데이션이 적용된 채팅 말풍선이 원래는 배경도 그라데이션이 적용되는 것이 맞습니다!
PR 에 올린 GIF와 영상은 이 점이 반영되기 전에 찍은 거라, 채팅 말풍선에 배경 그라데이션이 반영되어있지 않으니, 리뷰 시에 참고 바랍니다!
(pull 받아서 실행했을 때 아래와 같이 채팅 말풍선에도 그라데이션이 적용되는 것이 정상입니다!)
📸 스크린샷
Simulator.Screen.Recording.-.iPhone.16.-.2025-06-16.at.14.46.49.mp4