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

Feature/onborading #96

Merged
merged 15 commits into from
Aug 1, 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
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
4 changes: 4 additions & 0 deletions Projects/App/Sources/Application/AlimoApp.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ struct AlimoApp: App {

@UIApplicationDelegateAdaptor(AppDelegate.self) var delegate

init() {
Pretendard.register()
}

var body: some Scene {
WindowGroup {
ContentView()
Copy link

Choose a reason for hiding this comment

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

이 코드 패치를 간단히 검토해 보겠습니다.

  1. 위험 요소:

    • Pretendard.register()init() 내에서 호출됩니다. 이는 앱 초기화 시간에 작업을 수행할 수 있지만, 그 기능 및 side effects에 따라 잠재적인 문제가 발생할 수 있습니다.
  2. 개선 제안:

    • Pretendard.register()를 호출하는 위치가 적절한지 다시 고려하십시오. 필요한 경우 다른 곳에서 더 적절한 시점에 호출하는 것이 좋을 수 있습니다.
    • 불필요한 줄바꿈과 띄어쓰기 개선하여 가독성 향상을 고려할 수 있습니다.

개선 사항은 아직 오류나 위험이 없다고 말할 수 있지만, 코드의 유지보수성과 확장 가능성을 고려하여 주세요.

Expand Down
47 changes: 0 additions & 47 deletions Projects/App/Sources/UI/Component/Button/AlimoButton.swift

This file was deleted.

5 changes: 3 additions & 2 deletions Projects/App/Sources/UI/Component/Logo/AlimoLogo.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
//

import SwiftUI
import ADS

struct AlimoLogo: View {

Expand All @@ -15,11 +16,11 @@ struct AlimoLogo: View {
var body: some View {
HStack(alignment: .bottom, spacing: 0) {
Text(appName.uppercased())
.foregroundStyle(type.textColor)
.alimoColor(AlimoColor.Label.normal)
.font(Font(AppFontFamily.Pretendard.bold.font(size: type.fontSize)))
if type.hasDot {
Circle()
.foregroundStyle(type.dotColor)
.alimoColor(AlimoColor.Color.primary60)
.frame(width: 8, height: 8)
.padding(.bottom, 13)
}
Copy link

Choose a reason for hiding this comment

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

이 코드 패치에서 몇 가지 주요 사항을 강조할 수 있습니다:

  1. 새로운 ADS 라이브러리가 추가되었습니다. 이 라이브러리가 어떤 용도로 사용되는지 확인해야 합니다.

  2. .foregroundStyle()과 같은 스타일 메서드를 사용하는 대신, 커스텀한 .alimoColor()와 같은 메서드를 사용하는 것으로 보입니다. 해당 메서드의 구현 및 효과를 검토해야 할 필요가 있습니다.

  3. 뷰 구성이 변경된 부분이 많지 않아 실질적인 동작면에서 큰 문제는 없어 보입니다. 그러나 AppName, AlimoColor.Label.normal, AppFontFamily.Pretendard.bold, 그리고 type 등에 대한 세부 내용을 잘 파악하고 있는지 검토해야 합니다.

  4. 개선 제안: 코드 읽기의 일관성을 유지하기 위해 사전 정의된 색상 이름 대신 RGB 값 또는 시스템 색상을 사용하는 방법을 고려할 수 있습니다. 이는 유지보수와 테스트가 더욱 쉬워집니다.

  5. 코드 주석이 충분한지 여부를 검토하여 이해하기 쉬운 코드로 유지할 수 있는지 확인하십시오.

  6. 코드베이스의 다른 부분과 일관성을 유지하는 것이 중요합니다. 기존 코드 스타일과 일관성을 유지하며 변경사항을 통합해야 합니다.

  7. 마지막으로, 변경을 테스트하고 예기치 않은 동작이 발생하지 않는지 확인하는 것이 중요합니다.

Expand Down
13 changes: 7 additions & 6 deletions Projects/App/Sources/UI/Component/Logo/AlimoLogoType.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
//

import SwiftUI
import ADS

enum AlimoLogoType {
case white
Expand All @@ -15,17 +16,17 @@ enum AlimoLogoType {

var textColor: Color {
return switch self {
case .white: .black
case .yellow: .black
case .gray: .gray300
case .white: AlimoPallete.shared.Neutral00
case .yellow: AlimoPallete.shared.Neutral100
case .gray: AlimoPallete.shared.Neutral30
}
}

var dotColor: Color {
return switch self {
case .white: .white
case .yellow: .main500
case .gray: .gray300
case .white: AlimoPallete.shared.Neutral00
case .yellow: AlimoPallete.shared.Neutral100
case .gray: AlimoPallete.shared.Neutral30
}
}

Copy link

Choose a reason for hiding this comment

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

코드 리뷰:

  1. import ADS는 추가되었지만, 해당 모듈이 어떤 것을 포함하고 있는지에 대한 정보가 없습니다. 외부 종속성의 활용에 대한 문서화가 필요합니다.
  2. AlimoLogoType의 case들에 대해 각각의 textColordotColor를 설정하는데, 이 경우 switch 구문을 사용하면 간결하게 표현될 수 있습니다.
  3. 색상 값을 지정할 때 매직 넘버 대신 상수나 열거형을 사용하여 가독성과 유지보수성을 향상시킬 수 있습니다.
  4. 오탈자: "return switch" 대신 "switch self"가 옳은 표현입니다.

개선 제안:

  1. ADS 모듈에 대한 설명 주석 추가.
  2. switch self 대신 switch self로 수정.
  3. 색상 값을 간결한 이름으로 지정하고, 매직 넘버 대신 상수 또는 열거형 사용 권장.

전반적으로 코드는 깔끔하며 개별 case별 색상 지정을 명확하게 해주는 enum 구조체를 사용하여 관리가 용이합니다. 만약 ADS 모듈이 외부 종속성이라면 이에 대한 처리 및 의존성 관리도 필요합니다.

Expand Down
23 changes: 0 additions & 23 deletions Projects/App/Sources/UI/Component/Test/TextFieldTest.swift

This file was deleted.

36 changes: 0 additions & 36 deletions Projects/App/Sources/UI/Component/Test/TextTestView.swift

This file was deleted.

4 changes: 2 additions & 2 deletions Projects/App/Sources/UI/Component/Test/UITestView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ struct UITestView: View {
Text("Color Test")
}
NavigationLink {
TextTestView()
// TextTestView()
} label: {
Text("Text Test")
}
Expand All @@ -33,7 +33,7 @@ struct UITestView: View {
Text("Logo Test")
}
NavigationLink {
TextFieldTest()
// TextFieldTest()
} label: {
Text("Text Field Test")
}
Copy link

Choose a reason for hiding this comment

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

이 코드 패치를 간단한 코드 리뷰해 드리겠습니다.

  1. 주석 처리된 NavigationLink 부분: 코드에 있는 TextTestView()TextFieldTest() 가 실제로 사용되지 않고 주석 처리되어 있습니다. 필요 없는 코드로 불필요한 혼동을 줄일 수 있습니다. 만약에 임시로 주석처리했다면 적절한 주석을 추가하여 목적을 명확히 하실 수 있습니다.

  2. 코드가 충분히 단순하므로 큰 버그 위험이 나타나지는 않지만, 미래에 더 복잡한 코드로 확장될 경우 유용한 주석과 문서화를 통해 코드의 의도를 더 잘 전달할 수 있습니다.

  3. SwiftUI의 View 생성 및 관리 방식을 고려하여 필요한 경우 모델 분리 및 재사용성 고려할 수 있습니다.

  4. 코드 스타일 일관성을 유지하는 것이 좋습니다. 들여쓰기, 공백 등을 일관되게 유지해야 코드의 가독성이 향상됩니다.

Expand Down
121 changes: 0 additions & 121 deletions Projects/App/Sources/UI/Component/TextField/AlimoTextField.swift

This file was deleted.

This file was deleted.

Loading