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

Apply new MealView design #115

Merged
merged 4 commits into from
Aug 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Projects/App/iOS/Source/AppMain.swift
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ struct AppMain: App {
FlowPresenter(flow: flow)
}
.ignoresSafeArea()
.onAppear {
let color = Dodam.color(DodamColor.Primary.normal)
UIRefreshControl.appearance().tintColor = UIColor(color)
}
}
}
}

Choose a reason for hiding this comment

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

코드 패치에 대한 간단한 리뷰를 제공하겠습니다.

버그 위험 및 개선 제안:

  1. UIRefreshControl의 tintColor 설정:

    • UIRefreshControl.appearance().tintColor 설정은 앱의 전체 UI에 영향을 미칠 수 있습니다. 만약 이 색상을 특정 뷰에만 적용하고 싶다면, 해당 뷰에서 직접 UIRefreshControl 인스턴스를 생성하여 사용하세요.
  2. 색상 정의:

    • Dodam.color(DodamColor.Primary.normal)가 올바른 색상 객체를 반환하는지 확인해야 합니다. 색상 정의가 유효하지 않거나 예상과 다르게 동작하면 UI에 문제를 일으킬 수 있습니다.
  3. onAppear 위치:

    • onAppear가 필요 없는 경우 (예: 상태 관리나 다른 생명주기 메서드에서 처리되는 경우), 불필요하게 호출될 수 있으므로 항상 주의가 필요합니다.
  4. 일관성 유지:

    • 코드 스타일과 일관성을 유지하는 것이 좋습니다. let color = ... 구문은 더 직관적으로 작성할 수 있습니다. 예를 들어, 검증 후 바로 설정하는 방식으로 변경할 수 있습니다.

결론:

이 코드 변경은 전반적으로 적절해 보이지만, UI에 미칠 영향을 고려하고, 색상 정의의 안전성을 점검하는 것이 중요합니다. 추가적으로, 색상을 뷰에 국한시키는 방법을 검토하는 것이 좋습니다.

26 changes: 26 additions & 0 deletions Projects/Domain/Source/Entity/MealType.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
//
// MealType.swift
// Domain
//
// Created by hhhello0507 on 8/12/24.
//

import Foundation
import SwiftBok

public enum MealType: Int, RawRepresentable {
case breakfast
case lunch
case dinner

public var label: String {
switch self {
case .breakfast:
"아침"
case .lunch:
"점심"
case .dinner:
"저녁"
}
}
}

Choose a reason for hiding this comment

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

코드 리뷰 결과입니다.

버그 리스크:

  1. label 프로퍼티의 반환형: switch 문에서 모든 케이스에 대해 반환값이 누락되었습니다. 각 케이스에 return 키워드가 필요합니다. 예를 들어, case .breakfast: return "아침"과 같이 수정해야 합니다.

개선 제안:

  1. RawRepresentable 이점 활용: Enum의 RawValue로 String을 사용할 수 있습니다. 이를 통해 언어 변경 시 관리가 쉬워질 수 있습니다.
  2. 주석 추가: Enum에 대한 설명 주석을 추가하면 가독성이 향상됩니다. 예를 들어 각 식사 종류가 무엇을 의미하는지 간단히 설명할 수 있습니다.

수정된 예시 코드:

public enum MealType: String, RawRepresentable {
    case breakfast = "아침"
    case lunch = "점심"
    case dinner = "저녁"
    
    public var label: String {
        switch self {
        case .breakfast:
            return "아침"
        case .lunch:
            return "점심"
        case .dinner:
            return "저녁"
        }
    }
}

이러한 수정을 고려하시면 코드의 안정성과 가독성을 높일 수 있습니다.

4 changes: 4 additions & 0 deletions Projects/Domain/Source/Response/Meal/MealResponse.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,8 @@ public struct MealResponse: ResponseProtocol {
public let breakfast: Meal?
public let lunch: Meal?
public let dinner: Meal?

public var meals: [Meal] {
[self.breakfast, self.lunch, self.dinner].compactMap { $0 }
}
}

Choose a reason for hiding this comment

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

이 코드 패치에 대한 간단한 코드 리뷰는 다음과 같습니다:

버그 리스크

  1. self.breakfast, self.lunch, self.dinner 중 하나라도 nil인 경우, 예상대로 동작하므로 현재 구현은 안전합니다. 그러나, 모든 경우를 커버하지 않고 Meal 타입이 Optional이라는 점을 주의해야 합니다.

개선 제안

  1. 문서화: meals 프로퍼티의 사용 목적을 주석으로 설명하는 것이 좋습니다. 코드의 가독성을 향상시키고 유지보수 시 이해하는 데 도움이 될 수 있습니다.
  2. 정렬: 만약 meals 배열의 순서를 중요시한다면, 현재 구성된 순서가 의도된 것인지 확인해야 합니다. 예를 들어, 브렉퍼스트가 항상 첫 번째로 오는 것이 중요한지 등을 고려할 수 있습니다.
  3. Bundle Method: compactMap 대신 filtermap을 따로 사용할 수도 있지만, 성능 상의 이점은 명확하지 않으므로 현재 방식으로 두는 것도 괜찮습니다.

이상으로, 이 코드는 전반적으로 걱정할 요소가 적으며, 사용이 간편하고 효율적입니다.

51 changes: 51 additions & 0 deletions Projects/Feature/Source/Component/CalendarDateCell.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
//
// CalendarCell.swift
// Feature
//
// Created by hhhello0507 on 8/12/24.
//

import SwiftUI
import DDS

struct CalendarDateCell: View {

private let date: Date?
private let selected: Bool

init(date: Date?, selected: Bool) {
self.date = date
self.selected = selected
}

private var label: String {
guard let date else {
return ""
}
guard let day = date[.day] else {
return ""
}
return "\(day)"
}

var body: some View {
Text(label)
.headline(.medium)
.foreground(
selected
? DodamColor.Static.white
: DodamColor.Label.alternative
)
.padding(.vertical, 8)
.if(selected) { view in
view.background {
Rectangle()
.dodamFill(DodamColor.Primary.normal)
.clipShape(.medium)
.frame(width: 36, height: 36)
}
}
.frame(maxWidth: .infinity)
.opacity(date == nil ? 0 : 1)
}
}

Choose a reason for hiding this comment

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

코드 리뷰를 진행하겠습니다.

버그 위험:

  1. 옵셔널 언랩핑: datenil일 경우, guard let date else { return "" } 구문은 올바르게 처리하고 있지만, guard let day = date[.day]에서 datenil일 때 컴파일 에러를 발생시킬 수 있습니다. 이는 date가 다른 방법으로 안전하게 사용되도록 수정해야 합니다.

개선사항:

  1. 상수 및 리터럴 사용: 텍스트의 패딩과 같은 값 (8, 36)을 상수로 정의하면 코드 유지보수가 용이해집니다.
  2. 날짜 형식화: 라벨에서 날짜를 더 명확하게 표시할 필요가 있을 경우, DateFormatter를 사용하는 것이 좋습니다.
  3. SwiftUI 최적화: .frame(maxWidth: .infinity) 설정은 레이아웃에서 예상치 못한 동작을 일으킬 수 있으므로, 필요에 따라 Spacer() 등을 고려하는 것이 좋습니다.
  4. 읽기 쉬운 코드: 코드의 가독성을 높이기 위해 연산 프로퍼티와 기본 초기자를 간결하게 정리해도 좋습니다.

결론:

전반적으로 구조는 잘 되어 있으나, 옵셔널 처리 및 코드 유지보수를 위한 몇 가지 개선 사항이 있습니다. 이러한 부분을 점검하여 안정성과 가독성을 높이는 것이 좋습니다.

63 changes: 19 additions & 44 deletions Projects/Feature/Source/Home/Component/MealContainer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,72 +8,47 @@
import SwiftUI
import DDS
import Domain
import Shared

struct MealContainer: View {

@State private var pageSize: CGSize?
private let mealData: MealResponse?
@Binding var mealIdx: Int

public init(
data mealData: MealResponse?,
mealIdx: Binding<Int>
) {
self.mealData = mealData
self._mealIdx = mealIdx
self.animatedIdx = mealIdx.wrappedValue
}

@State private var animatedIdx: Int
@State private var heights: [Int: CGFloat] = [:]


var body: some View {
if let data = mealData {
if data.exists {
let meals = [
data.breakfast,
data.lunch,
data.dinner
]
DodamPageView(selection: $mealIdx) {
ForEach(meals.indices, id: \.self) { idx in
GeometryReader { geometryProxy in
let text = meals[idx]?.details.map {
$0.name
}.joined(separator: ", ") ?? "급식이 없어요."
Text(text)
.body1(.medium)
.foreground(DodamColor.Label.normal)
.multilineTextAlignment(.leading)
.fixedSize(horizontal: false, vertical: true)
.onAppear {
withAnimation(.spring) {
heights[idx] = text.boundingRect(
with: CGSize(
width: geometryProxy.size.width,
height: .greatestFiniteMagnitude
),
options: .usesLineFragmentOrigin,
attributes: [
.font: UIFont(
name: Pretendard.Weight.semibold.rawValue,
size: 18
)!
],
context: nil
).height
ForEach(data.meals, id: \.self) { meal in
let splitedArray = splitArray(array: meal.details)
HStack(alignment: .top) {
ForEach(splitedArray, id: \.self) { meals in
VStack(alignment: .leading, spacing: 0) {
ForEach(meals, id: \.self) { meal in
Text(meal.name)
.body1(.medium)
.foreground(DodamColor.Label.normal)
}
}
.frame(maxWidth: .infinity, alignment: .leading)
}
}
.onReadSize { size in
self.pageSize = size
}
.padding(.horizontal, 6)
.page()
}
}
.frame(height: heights[animatedIdx] ?? 44.928)
.onChange(of: mealIdx) { newValue in
withAnimation(.spring(duration: 0.2)) {
animatedIdx = newValue
}
}
.frame(height: pageSize?.height ?? 999)
.onAppear {
let currentTime = Date()
let calendar = Calendar.current

Choose a reason for hiding this comment

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

코드 리뷰를 진행하겠습니다.

개선 제안 및 버그 위험

  1. 명확한 변수 이름:

    • data, meal, meals와 같은 변수명이 조금 모호합니다. 더 구체적인 이름을 주어 가독성을 높이는 것이 좋습니다. 예를 들어, datamealResponse로 변경할 수 있습니다.
  2. 사이즈 초기화:

    • @State private var pageSize: CGSize?가 선언되어 있지만, 사용되기 전에 초기값이 설정되지 않습니다. 필요한 경우 기본값을 설정하거나 nil 체크를 해야 할 필요가 있습니다.
  3. 스프링 애니메이션:

    • 주석 처리된 코드를 보니 애니메이션 로직이 제거되었습니다. 이는 사용자 경험에 영향을 줄 수 있습니다. onAppear에서의 스프링 애니메이션을 적절히 다시 추가하여 부드러운 전환을 제공하는 것이 추천됩니다.
  4. 성능 최적화:

    • ForEach 안에서 중첩된 ForEach가 사용되고 있는데, 이는 성능에 영향을 미칠 수 있습니다. 가능한 경우 데이터를 더 간단하게 구조화하거나 뷰를 단순화하는 것을 고려해보세요.
  5. UI 요소 정렬:

    • HStack이라고 명시하고 있으나, 내부에 VStack이 존재합니다. UI 요소들의 정렬 방식이 원하는 대로 이루어지는지 확인하고 필요한 경우 정렬 속성을 명확히 정의하세요.
  6. 사용되지 않는 코드 삭제:

    • 주석 처리된 오래된 코드 부분은 실제 사용하지 않는 코드 임에도 남아있습니다. 필요 없는 코드는 삭제하여 코드의 직관성을 높이는 것이 유리합니다.
  7. 옵셔널 처리:

    • meal.details와 같은 옵셔널 값 접근 시 guard let이나 if let을 활용하면 안전성을 높일 수 있습니다.

이러한 점들을 고려하면 더욱 안정적이고 유지보수하기 쉬운 코드를 작성할 수 있을 것입니다.

Expand Down
40 changes: 40 additions & 0 deletions Projects/Feature/Source/Meal/Component/MealCell.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
//
// MealCell.swift
// Feature
//
// Created by hhhello0507 on 8/12/24.
//

import SwiftUI
import DDS
import Domain

struct MealCell: View {

private let type: MealType
private let meal: Meal

init(type: MealType, meal: Meal) {
self.type = type
self.meal = meal
}

var body: some View {
VStack(spacing: 12) {
HStack {
DodamTag(type.label, type: .primary)
Spacer()
Text("\(Int(meal.calorie))Kcal")
.label(.medium)
.foreground(DodamColor.Label.alternative)
}
Text(meal.details.map { $0.name }.joined(separator: "\n"))
.body1(.medium)
.foreground(DodamColor.Label.normal)
.frame(maxWidth: .infinity, alignment: .leading)
}
.padding(16)
.background(DodamColor.Background.normal)
.clipShape(.large)
}
}

Choose a reason for hiding this comment

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

코드 리뷰를 진행하겠습니다.

코드 리뷰

  1. 가독성: 각 요소에 대해 적절한 줄바꿈과 공백을 사용하고 있어 가독성이 좋습니다.

  2. 모델 데이터 사용:

    • meal.details가 빈 배열일 경우, joined(separator: "\n")는 빈 문자열을 반환합니다. UI에서 이를 잘 처리하는지 확인이 필요합니다. 예를 들어, 텍스트가 없어질 수 있습니다.
  3. 타입 안전성:

    • meal.calorieInt로 강제 변환하고 있습니다. 만약 meal.calorie가 nil일 가능성이 있다면, 옵셔널 바인딩이나 기본값을 설정해야 할 것입니다.
  4. UI 최적화:

    • DodamTagText 타입의 요소는 각각 자신만의 공간을 가지므로, HStack 내에서 Spacer()를 사용하여 불필요한 여백을 만들지 않도록 주의해야 합니다.
  5. SwiftUI 사용:

    • SwiftUI의 새로운 기능을 활용할 수 있는지 고려해보세요. 예를 들어, .foregroundColor를 사용할 때 경우에 따라 @Environment를 통해 색상을 관리할 수 있습니다.
  6. 주석 추가:

    • 코드에 대한 설명이 부족합니다. 특히 주요 메서드나 복잡한 로직에는 주석을 추가하여 향후 유지보수 시 도움이 되도록 해주세요.

결론

전반적으로 코드가 깔끔하게 작성되었으며 좋은 구조를 가지고 있습니다. 위에서 언급한 개선점을 참고하여 코드의 안정성과 가독성을 더욱 높일 수 있을 것입니다.

Loading