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

Fix [#168] 로딩뷰 QA 이슈 수정 #169

Merged
merged 2 commits into from
Mar 22, 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
1 change: 0 additions & 1 deletion DontBe-iOS/DontBe-iOS/Global/Literals/StringLiterals.swift
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,6 @@ enum StringLiterals {

enum Loading {
static let loadingMessageTitle = "알고 계셨나요?"
static let loadingMessage1 = "온라인 상에서도 비속어를\n사용하지 않은 사람들이\n실제 행복지수가 더 높다고 해요."
static let loadingMessage2 = "Don’t be는 그저 온화한 커뮤니티가 아닌\n온화하면서도 재밌을 수 있는 커뮤니티를 지향해요."
static let loadingMessage3 = "Don’t be의 온화함을 해치는 글이 보인다면\n직접 투명도 기능을 눌러보세요!"
static let loadingMessage4 = "Don’t be 팀은 온화한 커뮤니티를 만들기 위해\n저희부터 온화한 팀이 되고자 노력하고 있어요."

Choose a reason for hiding this comment

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

이 코드 리뷰에서는 다음과 같은 사항을 고려할 수 있습니다:

  1. "loadingMessage1" 상수가 삭제되었으며 이에 관련된 사용 또는 의존성이 있는지 확인해야 합니다.
  2. 문자열의 길이나 내용 변경으로 인한 UI 또는 다른 기능에 영향을 줄 수 있으므로, 설계 또는 문구적인 변화에 대한 유사한 팀 간 검토 요청이 필요할 수 있습니다.

이외의 디테일한 부분은 여러 컨텍스트와 팀 업무 환경에 따라 달라질 수 있습니다.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@ import Lottie
final class DontBeLoadingView: UIView {

private var loadingText: String = ""
private var loadingTexts = [StringLiterals.Loading.loadingMessage1,
StringLiterals.Loading.loadingMessage2,
private var loadingTexts = [StringLiterals.Loading.loadingMessage2,
StringLiterals.Loading.loadingMessage3,
StringLiterals.Loading.loadingMessage4,
StringLiterals.Loading.loadingMessage5,

Choose a reason for hiding this comment

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

이 코드는 "loadingTexts" 배열에서 "loadingMessage1" 아이템을 제거하고 "loadingMessage2" 아이템을 첫 번째 요소로 추가했습니다. 이 변경은 의도된 것인지 확인해야 합니다. 이에 대한 주석이나 변경 로그가 필요할 수 있습니다. 또한, "loadingTexts" 배열 초기화 중에 콤마(,)를 유지하는 일관성이 있어야 합니다.

개선 제안:

  1. 코드에서 "loadingText" 변수는 현재 사용되지 않는 것 같습니다. 사용하지 않는 변수는 정리하도록 권장합니다.
  2. 각 메시지가 언제 표시되는지 설명하는 주석을 추가하여 코드의 가독성을 높일 수 있습니다.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,20 +148,27 @@ final class DontBeTabBarController: UITabBarController {
extension DontBeTabBarController: UITabBarControllerDelegate {

func tabBarController(_ tabBarController: UITabBarController, didSelect viewController: UIViewController) {
if selectedIndex == 1 {
self.selectedIndex = 0
}

if selectedIndex == 2 {
self.tabBar.items?[2].image = ImageLiterals.TabBar.icnNotificationRead
}

if beforeIndex == 2 && self.selectedIndex == 0 {
showLoadingView()
if beforeIndex == 2 {
if self.selectedIndex == 0 {
showLoadingView()
print("여기는 \(self.selectedIndex)")
}
}

if beforeIndex == 3 {
if self.selectedIndex == 0 {
showLoadingView()
print("여기는 \(self.selectedIndex)")
}
}

if beforeIndex == 3 && self.selectedIndex == 0 {
showLoadingView()
if selectedIndex == 1 {
self.selectedIndex = 0
hideLoadingView()
}

if let selectedViewController = tabBarController.selectedViewController {
Expand Down Expand Up @@ -215,8 +222,14 @@ extension DontBeTabBarController {
loadingView.show()

DispatchQueue.main.asyncAfter(deadline: .now() + 1.4) {
loadingView.hide {
}
loadingView.hide()
}
}

private func hideLoadingView() {
let loadingView = DontBeLoadingView()
DispatchQueue.main.async {
loadingView.hide()
}
}
}

Choose a reason for hiding this comment

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

코드 리뷰:

  1. func tabBarController(_ tabBarController: UITabBarController, didSelect viewController: UIViewController) 메서드 내에서 selectedIndexbeforeIndex의 사용은 명확하지 않다.
  2. showLoadingView()가 여러 번 호출될 수 있으며 이는 비효율적이다. 이미 loading view가 표시 중이라면 새로운 호출을 무시하는 방법을 고려해야 한다.
  3. loadingView.hide { } 대신에 loadingView.hide()를 사용하여 코드를 더 간단하게 만들 수 있다.
  4. hideLoadingView() 메서드가 호출되는 시점과 이유를 좀 더 명확히 설명하면 유지보수가 용이해진다.
  5. loadingView 객체를 필요할 때마다 새로 만드는 대신에 재사용하여 성능을 개선할 수 있다.

개선 제안:

  1. selectedIndexbeforeIndex 값의 확실한 정의와 사용 목적을 추가한다.
  2. showLoadingView()에서 이미 loading view가 보여지고 있는지 체크하여 중복 호출을 방지한다.
  3. loadingView.hide { }loadingView.hide()로 단순화한다.
  4. hideLoadingView() 메서드의 호출 시점 및 목적을 주석으로 상세히 기록한다.
  5. loadingView 객체를 재사용하여 성능을 향상시킨다.

이러한 변경 사항을 적용하면 코드의 가독성과 효율성을 향상시킬 수 있습니다.