[#65] 홈 UI 변경 #276
[#65] 홈 UI 변경 #276
Conversation
- 상단 그림자 height값 조절 - 선택 시 보여지는 선택 UI 수정 - 리스트에서 메뉴 선택 시 무조건 클릭되는 버그 수정
궁금한 사항이 있어, 코드리뷰 전 코멘트 남깁니다!
|
|
아하! 알겠습니다. 참고해서 리뷰달께요. |
| import EATSSUDesign | ||
|
|
||
| import Moya | ||
| import SnapKit | ||
| import Then |
There was a problem hiding this comment.
EATSSUDesign와 같은 내부 사용자 정의 라이브러리는 외부 라이브러리 하단에 작성해주세요:)
There was a problem hiding this comment.
무슨 말씀인지 이해를 못했습니다.
import UIKit
import Moya
import SnapKit
import Then
import EATSSUDesign
이렇게 작성을 해야한다는 말씀이실까요?
There was a problem hiding this comment.
넵. 보통 아래 순서로 작성하더라구요!
- Foundation / UIKit / Swift 표준 라이브러리
- 외부 라이브러리 (CocoaPods, SPM 등)
- 내부 모듈 / 사용자 정의 프레임워크
There was a problem hiding this comment.
앗 그럼 파일 전부 제가 통일 시켜보겠습니다.
import UIKit
import EATSSUDesign
import SnapKit
import Then
거의 다 이런식으로 작성되어있어서.. 저도 그대로 따라했습니다..
하단에 메뉴 선택 이펙트 확인하신거면 이것도 작업 후에 머지 하겠습니다.
| $0.top.equalToSuperview() | ||
| $0.leading.bottom.trailing.equalToSuperview() |
There was a problem hiding this comment.
| $0.top.equalToSuperview() | |
| $0.leading.bottom.trailing.equalToSuperview() | |
| $0.edges.equalToSuperview() |
| private let menuStackView = UIStackView() | ||
|
|
||
| // 메뉴가 없을 때 표시할 안내 텍스트 | ||
| private let emptyLabel = UILabel().then { | ||
| $0.text = "영업 시간이 아니에요." | ||
| $0.font = .semiBold(size: 10) |
There was a problem hiding this comment.
폰트는 EATSSUDesignFontFamily 디자인 시스템 사용해주시면 됩니다.
아래와 같이 UIFont는 레거시파일입니다. 해당 부분 중복 정의되어있어서 혼동을 줄 수 있겠네요.!
제가 작업하면서 기존 파일 제거해놓을께요.
extension UIFont {
/*
설명
1.x.x 버전에서 사용 중이던 AppleSD 폰트 사용입니다.
이제 더 이상 사용하지는 않지만, 개발단계에 있어서 빌드에러가 발생하지 않도록 남겨두고, 추후 제거하겠습니다.
*/
...
| private let emptyLabel = UILabel().then { | ||
| $0.text = "영업 시간이 아니에요." | ||
| $0.font = .semiBold(size: 10) | ||
| $0.textColor = .black | ||
| $0.textAlignment = .center | ||
| $0.numberOfLines = 0 | ||
| $0.isHidden = true | ||
| $0.isHidden = true // 기본적으로 숨겨둠 |
There was a problem hiding this comment.
// 메뉴가 없을 때 표시할 안내 텍스트 라는 주석으로 해당 컴포넌트의 의미는 명확히 전달된 것 같아 해당 주석은 없어도 될 것 같아요. 어디까지나 제 의견이니 모두 반영하실 필요는 없습니다!
| @@ -37,11 +51,11 @@ final class RestaurantMenuGroupCell: BaseTableViewCell { | |||
| wrapperView.backgroundColor = .white | |||
|
|
|||
| menuStackView.axis = .vertical | |||
There was a problem hiding this comment.
컴포넌트 속성 지정은 선언 시에 해주는 건 어떨까요? 해당 함수의 역할이 한 눈에 들어왔음합니다.
| override func configureUI() { | ||
| self.selectionStyle = .none | ||
| self.selectedBackgroundView = UIView() |
There was a problem hiding this comment.
selectedBackgroundView를 직접 생성했다고 생각했는데, UITableViewCell이 직접 제공하는 속성이군요!
배워갑니다!!
| /// 셀이 재사용되기 전에 호출됨 | ||
| /// menuStackView에 남아있는 메뉴 뷰들을 제거하고, 재사용 풀에 저장 | ||
| override func prepareForReuse() { | ||
| super.prepareForReuse() | ||
| for view in menuStackView.arrangedSubviews { | ||
| view.removeFromSuperview() | ||
| if let itemView = view as? RestaurantMenuItemView { | ||
| itemViewPool.append(itemView) // 뷰 재사용 풀에 넣기 | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// 재사용 가능한 뷰가 있다면 꺼내고, 없으면 새로 생성 | ||
| private func dequeueReusableItemView() -> RestaurantMenuItemView { | ||
| if let reused = itemViewPool.popLast() { | ||
| return reused | ||
| } else { | ||
| return RestaurantMenuItemView() | ||
| } | ||
| } |
There was a problem hiding this comment.
재사용 풀을 선언하여 구현하는 방식은 처음 봅니다! 재사용 풀을 사용한 이유도 궁금해요.
예외사항없다면 모든 뷰가 아래 로직을 통해 재사용이 되나요? 아님 재사용 안되는 경우도 다수 존재하나요?
if let reused = itemViewPool.popLast() {
return reused
}
There was a problem hiding this comment.
넵 재사용 됩니다! 원래 사용했던 방식은 UITableViewCell에서 prepareForReuse을 사용했었습니다. 제가 구조를 UITableViewCell이 아닌 UIView로 구현하게 되면서 날짜가 변경될 때마다 RestaurantMenuItemView를 새로 만들었고 성능이 떨어졌었습니다. 그래서 커스텀 재사용 스택이라고 보시면 될 것 같습니다!
간단하게 말하면 18일날 메뉴가 총 20개고 19일이 19개라고 할 때, 20->19일로 가면 총 19개가 재사용되는.. 그렇습니다!
There was a problem hiding this comment.
아하! 그럼 날짜에 따라 메뉴 개수가 20->4->10 순서로 변경되면, 4->10 메뉴로 갈 때에는 6개가 새로 만들어져야겠군요?!
There was a problem hiding this comment.
처음에 20개를 만들었다면 남은 것들은 전부 pool에 저장됩니다! 그래서 4 -> 10개로 갈때는 pool안에 있던 걸 그냥 꺼내쓰게 됩니다. 만약 21개나 22개를 띄워야한다면 1,2개만 새로 생성해서 사용하는거라고 생각해주시면 될 것 같아요.
즉, 처음 생성되는 갯수 기준으로 재사용 & 부족하면 생성되어 사용됩니다!
| import SnapKit | ||
|
|
||
| /// 식당 메뉴 아이템 하나를 나타내는 커스텀 뷰 | ||
| /// 구성: 메뉴명(name), 가격(price), 평점(rating) |
| // 메뉴명 라벨 | ||
| private let nameLabel = UILabel().then { | ||
| $0.numberOfLines = 0 | ||
| $0.font = .body3 | ||
| $0.font = .body3 // 커스텀 폰트 스타일 | ||
| } | ||
|
|
||
| // 가격 라벨 | ||
| private let priceLabel = UILabel().then { | ||
| $0.font = .body3 | ||
| $0.textAlignment = .center | ||
| } | ||
|
|
||
| // 평점 라벨 | ||
| private let ratingLabel = UILabel().then { | ||
| $0.font = .body3 | ||
| $0.textAlignment = .center | ||
| } | ||
|
|
||
| // 콘텐츠 수평 스택 뷰 (이름, 가격, 평점 수평 정렬) | ||
| private let contentStackView = UIStackView().then { | ||
| $0.axis = .horizontal | ||
| $0.alignment = .center | ||
| $0.spacing = 24 | ||
| } |
There was a problem hiding this comment.
#277pr에도 리뷰달아놓았는데요.(#277 (comment))
같은 파일 내에서도 then 사용여부가 통일안되어 있는 경우도 있어, 이번 기회에 프로젝트 내에서 then 사용여부를 통일하여도 좋을 것 같아요.
There was a problem hiding this comment.
안그래도 then을 음.. 빼도 되지 않을까 하는 생각이 점점 들긴합니다! slack에서 얘기해보죠!
There was a problem hiding this comment.
요건...! then 제거하는 걸로 통일해도 괜찮을까요?
실기기로 빌드해서 사용해봤는데, 일반적으로 사용자의 터치 유지 기간이 0.1초가 안되나봅니다. 제 입장에선 길게 눌러야지만 선택 이펙트가 나타나는 느낌이었어요. 상환님도 실기기로 테스트 한 번 해보시면 좋을 것 같아요. 저는 해당부분 이슈로 처리했으면 하는데 의견남겨주세요:) (해당 pr 병합후, 이슈로 진행해도 좋을 것 같아요) |
아뇨! 잘 반영된 것 같아요! |
위에서 말한 코멘트들은 다 반영을 했고, 이펙트에 대한 부분을 좀 생각해봤습니다. 이것저것 테스트를 해보다가 이펙트 인식이 안되는게 아니라 그 시간이 너무 짧은 나머지 뷰에 표시되지 않는다는 걸 알아냈는데, 생각해보니까 메뉴를 클릭하면 무조건 리뷰페이지가 뜨는 구조고 그 뷰가 뜨게 될 때, 약간의 애니메이션? 딜레이가 있었어요. 그래서 그 구간을 활용하는 방식으로 구현을 해봤습니다. |
|
|
||
| // 터치 시작: 배경 강조 효과 | ||
| override func touchesBegan(_ touches: Set<UITouch>, with event: UIEvent?) { | ||
| super.touchesBegan(touches, with: event) | ||
| backgroundWrapper.backgroundColor = UIColor.gray300 | ||
| } | ||
|
|
||
| // 터치 취소: 배경 색상 복원 | ||
| override func touchesCancelled(_ touches: Set<UITouch>, with event: UIEvent?) { | ||
| super.touchesCancelled(touches, with: event) | ||
| backgroundWrapper.backgroundColor = .clear | ||
| } | ||
|
|
||
| // 터치 종료 시 실행: 영역 안에 있으면 onTap 클로저 실행 | ||
| override func touchesEnded(_ touches: Set<UITouch>, with event: UIEvent?) { | ||
| super.touchesEnded(touches, with: event) | ||
|
|
||
| guard let touch = touches.first else { return } | ||
| let location = touch.location(in: self) | ||
|
|
||
| if bounds.contains(location), let indexPath = indexPath { | ||
| onTap?(indexPath, menuIndex) | ||
| } |
There was a problem hiding this comment.
.clear설정이 딜레이를 갖도록 구현했군요!
저도 성능상에는 문제가 없을 듯 해서 우선 이 방법도 괜찮을 것 같아요.
아님 UIView의 animate 속성 이용해 볼 수 있을 것 같아요!
| // 터치 시작: 배경 강조 효과 | |
| override func touchesBegan(_ touches: Set<UITouch>, with event: UIEvent?) { | |
| super.touchesBegan(touches, with: event) | |
| backgroundWrapper.backgroundColor = UIColor.gray300 | |
| } | |
| // 터치 취소: 배경 색상 복원 | |
| override func touchesCancelled(_ touches: Set<UITouch>, with event: UIEvent?) { | |
| super.touchesCancelled(touches, with: event) | |
| backgroundWrapper.backgroundColor = .clear | |
| } | |
| // 터치 종료 시 실행: 영역 안에 있으면 onTap 클로저 실행 | |
| override func touchesEnded(_ touches: Set<UITouch>, with event: UIEvent?) { | |
| super.touchesEnded(touches, with: event) | |
| guard let touch = touches.first else { return } | |
| let location = touch.location(in: self) | |
| if bounds.contains(location), let indexPath = indexPath { | |
| onTap?(indexPath, menuIndex) | |
| } | |
| // 터치 시작: 배경 강조 효과 | |
| override func touchesBegan(_ touches: Set<UITouch>, with event: UIEvent?) { | |
| super.touchesBegan(touches, with: event) | |
| UIView.animate(withDuration: 0.1) { | |
| self.backgroundWrapper.backgroundColor = UIColor.gray300 | |
| } | |
| } | |
| // 터치 취소: 배경 색상 복원 | |
| override func touchesCancelled(_ touches: Set<UITouch>, with event: UIEvent?) { | |
| super.touchesCancelled(touches, with: event) | |
| UIView.animate(withDuration: 0.1) { | |
| self.backgroundWrapper.backgroundColor = .clear | |
| } | |
| } | |
| // 터치 종료 시 실행: 영역 안에 있으면 onTap 클로저 실행 | |
| override func touchesEnded(_ touches: Set<UITouch>, with event: UIEvent?) { | |
| super.touchesEnded(touches, with: event) | |
| guard let touch = touches.first else { return } | |
| let location = touch.location(in: self) | |
| UIView.animate(withDuration: 0.1) { | |
| self.backgroundWrapper.backgroundColor = .clear | |
| } | |
| if bounds.contains(location), let indexPath = indexPath { | |
| onTap?(indexPath, menuIndex) | |
| } | |
| } | |
#️⃣ 관련 이슈
Resolved #65
💡작업 내용
💬리뷰 요구사항(선택)