Skip to content

Conversation

wjdrjs00
Copy link
Member

@wjdrjs00 wjdrjs00 commented Sep 18, 2025

[ PR Content ]

Related issue

Screenshot 📸

  • N/A

Work Description

  • HomRoute간 중복 네비게이션 방지를 위한 로직 추가

To Reviewers 📢

  • 간단한 작업이지만 작업간 "두번 클릭 자체를 막기" vs "중복 생성을 막기" 에 대한 고민이 있었는데, 문제의 원인이 스택이 중복으로 생기는것이기 때문에 후자가 더 적합하다고 판단하여 "중복 생성" 자체를 방지하는 방향으로 구현해봤습니다~!

Summary by CodeRabbit

  • 버그 수정
    • 하단 탭의 선택 상태가 네비게이션 상태와 안정적으로 동기화되도록 개선
    • 이미 선택된 탭을 눌러도 중복 탐색이 발생하지 않도록 방지
    • 탭 전환 시 이전 화면 상태를 복원해 끊김 없는 탐색 경험 제공
    • 중복 스택 생성을 억제해 뒤로 가기 동작을 일관되게 조정

@wjdrjs00 wjdrjs00 requested a review from l5x5l September 18, 2025 10:35
@wjdrjs00 wjdrjs00 self-assigned this Sep 18, 2025
Copy link

coderabbitai bot commented Sep 18, 2025

Walkthrough

HomeBottomNavigationBar가 현재 백스택 엔트리의 라우트(currentRoute)로 선택 상태를 계산하도록 변경되었고, 탭 클릭 시 대상 라우트가 현재 라우트와 다를 때만 네비게이션을 수행하도록 변경되었습니다. 네비게이션 호출은 기존의 popUpTo(0) { inclusive = true }를 유지합니다.

Changes

Cohort / File(s) Summary
Bottom navigation logic
app/src/main/java/com/threegap/bitnagil/navigation/home/HomeBottomNavigationBar.kt
현재 백스택 엔트리에서 currentRoute를 추출해 선택 상태 비교에 사용하도록 변경. 탭 클릭 시 대상 라우트가 currentRoute와 다를 경우에만 navigate 호출하도록 조건 추가. navigate 호출은 기존의 popUpTo(0) { inclusive = true } 옵션을 유지. 공개 API 변경 없음.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    actor User as 사용자
    participant UI as HomeBottomNavigationBar
    participant Nav as NavController

    User->>UI: 탭 클릭
    UI->>Nav: currentBackStackEntry 조회
    Nav-->>UI: currentRoute 반환
    alt targetRoute != currentRoute
        UI->>Nav: navigate(targetRoute, options)
        Note right of Nav: options: popUpTo(0) { inclusive = true }
        Nav-->>UI: 이동 완료
    else 동일 라우트
        UI-->>User: No-op (이동 없음)
    end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

폴짝! 탭을 톡톡 건너뛰니
길은 중복 없이 한 줄로 서네 🐇
현재 길만 바라보고 한 번만 가며,
스택은 깔끔히, 발걸음은 경쾌—홉!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Linked Issues Check ❓ Inconclusive PR은 currentRoute 비교로 동일 목적지로의 중복 네비게이션을 차단하도록 구현하여 일부 중복 케이스를 해결하지만, 네비게이션이 완료되기 전에 발생하는 빠른 연타(레이스 컨디션) 상황에서는 여전히 중복 호출이 발생할 가능성이 있어 링크된 이슈의 목표(어떤 상황에서도 중복 없이 한 번만 생성)를 완전히 만족하는지는 추가 검증이 필요합니다. 권장: 빠른 연타 시나리오에 대한 재현 테스트를 수행하고, 필요하면 navigate 옵션(예: launchSingleTop), 클릭 디바운스 또는 네비게이션 중 버튼 비활성화 같은 보완책을 적용하여 모든 연타 케이스에서 중복 생성이 완전히 차단되는지 확인해 주세요.
✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed 제목 "[Fix/#127] 네비게이션 스택이 중복으로 생성되는 문제를 해결합니다."은 PR의 핵심 변경점인 '네비게이션 스택 중복 방지'를 직접적으로 요약하고 있어 변경사항(홈 탭 네비게이션 중복 방지 로직 추가)과 일치합니다. 간결하고 스캔하기에 적합합니다.
Out of Scope Changes Check ✅ Passed 변경은 app/src/main/java/.../HomeBottomNavigationBar.kt에 국한되어 있고 목적(네비게이션 로직 수정)과 직결되므로 링크된 이슈 범위를 벗어나는 코드 변경은 발견되지 않습니다.
Description Check ✅ Passed PR 설명은 저장소 템플릿의 주요 섹션(관련 이슈, 스크린샷 필드, 작업 설명, 리뷰어 안내)을 포함하고 있으며 closed #127 및 작업 요약과 구현 방향을 명확히 기재하고 있어 전반적으로 템플릿 요건을 충족합니다.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/#127-navigation-logic

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8a024c5 and 79c5ad2.

📒 Files selected for processing (1)
  • app/src/main/java/com/threegap/bitnagil/navigation/home/HomeBottomNavigationBar.kt (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • app/src/main/java/com/threegap/bitnagil/navigation/home/HomeBottomNavigationBar.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@wjdrjs00 wjdrjs00 changed the title [Fix/#127] 중복 네비게이션 방지를 위한 로직 추가 [Fix/#127] 네비게이션 스택이 중복으로 생성되는 문제를 해결합니다. Sep 18, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (4)
app/src/main/java/com/threegap/bitnagil/navigation/home/HomeBottomNavigationBar.kt (4)

33-33: route 직접 비교 대신 hierarchy 사용 권장

동적 파라미터/중첩 그래프에서 동일 탭임에도 불필요한 네비게이션/선택 표시 오류가 날 수 있습니다. destination.hierarchy 기반으로 판단하도록 전환을 제안합니다.

-    val currentRoute = navBackStackEntry?.destination?.route
+    val currentDestination = navBackStackEntry?.destination

(아래 selected 및 클릭 가드에서도 currentDestination를 사용)


56-63: 현재 탭 재선택(reselect) UX 정의 필요

동일 탭을 다시 누르면 아무 동작도 하지 않는데, 보편적으로는 루트로 pop하거나 리스트를 맨 위로 스크롤합니다. 제품 의도에 맞게 결정하고 반영하는 것을 권장합니다.

예시:

val inSameTab = currentDestination?.hierarchy?.any { it.route == homeRoute.route } == true
if (!inSameTab) {
    // navigate (save/restore 포함)
} else {
    // 예: 해당 탭 루트까지 pop
    navController.popBackStack(homeRoute.route, inclusive = false)
}

64-64: selected 계산도 hierarchy 기반으로 통일

탭 내부의 하위 화면에 있을 때도 탭이 선택 상태로 보이도록 해야 합니다.

-                    selected = currentRoute == homeRoute.route,
+                    selected = currentDestination?.hierarchy
+                        ?.any { it.route == homeRoute.route } == true,

99-109: 아이콘/텍스트 접근성 개선

스크린리더 대응을 위해 contentDescription(혹은 semantics { contentDescription = title })을 제공하세요. 현재는 indication = null로 리플도 비활성화되어 있어 시각적/보조기술 피드백이 부족합니다.

예시:

BitnagilIcon(
    id = icon,
    modifier = Modifier.size(24.dp),
    tint = contentTintColor,
    contentDescription = title, // BitnagilIcon이 지원한다면
)

또는

Column(
  modifier = modifier
    .semantics { contentDescription = title }
    .clickable( ... )
    .padding(4.dp),
  ...
)
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e4d499e and 8a024c5.

📒 Files selected for processing (1)
  • app/src/main/java/com/threegap/bitnagil/navigation/home/HomeBottomNavigationBar.kt (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build

Copy link
Contributor

@l5x5l l5x5l left a comment

Choose a reason for hiding this comment

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

고생하셨습니다! 🚀

@wjdrjs00 wjdrjs00 merged commit 1765425 into develop Sep 18, 2025
2 checks passed
@wjdrjs00 wjdrjs00 deleted the fix/#127-navigation-logic branch September 18, 2025 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FIX] 네비게이션 스택이 중복으로 생성되는 문제를 해결합니다.

2 participants