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.

Sign up for GitHub

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

[Bug] 버튼 연타 시 중복 호출 이슈 수정 #243

Merged
merged 3 commits into from
Sep 4, 2023
Merged

[Bug] 버튼 연타 시 중복 호출 이슈 수정 #243

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4bfb6d1
fix: onBackClick 중복 호출을 통한 잘못된 뒤로가기 방지
kisa002 Aug 30, 2023
0e41bf9
fix: session 화면 중복 이동 방지
kisa002 Aug 30, 2023
7d98e2b
fix: 코멘트 반영
kisa002 Sep 1, 2023
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
8 changes: 8 additions & 0 deletions ...ignsystem/src/main/java/com/droidknights/app2023/core/designsystem/component/TopAppBar.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package com.droidknights.app2023.core.designsystem.component

import androidx.annotation.StringRes
import androidx.compose.foundation.background
import androidx.compose.foundation.clickable
import androidx.compose.foundation.interaction.MutableInteractionSource
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.fillMaxWidth
Expand All @@ -16,6 +18,7 @@ import androidx.compose.material3.MaterialTheme
import androidx.compose.material3.Text
import androidx.compose.runtime.Composable
import androidx.compose.runtime.CompositionLocalProvider
import androidx.compose.runtime.remember
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.Color
Expand Down Expand Up @@ -55,6 +58,11 @@ fun KnightsTopAppBar(
modifier = Modifier
.fillMaxWidth()
.background(containerColor)
.clickable(
interactionSource = remember { MutableInteractionSource() },
indication = null,
onClick = {}
)
wisemuji marked this conversation as resolved.
Show resolved Hide resolved
.then(modifier)
) {
if (navigationType == TopAppBarNavigationType.Back) {
Expand Down
10 changes: 10 additions & 0 deletions feature/main/src/main/java/com/droidknights/app2023/feature/main/MainNavigator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import androidx.navigation.compose.rememberNavController
import androidx.navigation.navOptions
import com.droidknights.app2023.feature.bookmark.navigation.navigateBookmark
import com.droidknights.app2023.feature.contributor.navigation.navigateContributor
import com.droidknights.app2023.feature.home.navigation.HomeRoute
import com.droidknights.app2023.feature.home.navigation.navigateHome
import com.droidknights.app2023.feature.session.navigation.navigateSession
import com.droidknights.app2023.feature.session.navigation.navigateSessionDetail
Expand Down Expand Up @@ -61,6 +62,15 @@ internal class MainNavigator(
navController.popBackStack()
}

fun popBackStackIfNotHome() {
if (!isSameCurrentDestination(HomeRoute.route)) {
popBackStack()
}
Comment on lines +66 to +68
Copy link
Contributor

Choose a reason for hiding this comment

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

홈에서도 뒤로가기혹은 네비게이션 동작이 똑같은 상황을 만들어 낼 수 있지 않을까요?
이 부분은 저도 고민해보지 못한 부분이라 비슷한 내용을 담고 있는 이슈 남겨봤습니다.

Copy link
Contributor Author

@kisa002 kisa002 Sep 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

근본적으로 방지한다면, Navigate의 경우 PR에 작성해둔 것 처럼
navigate 시 이동할 destination과 currentDestination이 같지 않은 경우에만 이동

하도록 구현한다면 전체 상황에 대해서 방지할 수 있습니다.

뒤로가기의 경우에도 호출되는 모든 popBackStack 부분들에 때 의도하는 destination을 넘겨 해당 destination이 아닌 경우에만 뒤로가도록하는 방식을 채택한다면 마찬가지로 근본적으로 수정가능할 것 으로 보입니다.

다만 전반적으로 하나하나 연타를 해보았을 때 위의 시나리오 외에 중복 호출 되는 곳은 없어 적용하지 않았습니다. 만약 근본적인 차단을 고려하신다면 navigate, popBackStack하는 모든 부분들을 destination을 넘겨 받아 비교 후 처리하도록 구현하도록 하겠습니다. 😃

Copy link
Contributor

Choose a reason for hiding this comment

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

넵, 아직은 확실하지 않은 기능이고 버그에 대한 이슈이므로 이정도로 괜찮을 것 같습니다.
확인 감사합니다

}

fun isSameCurrentDestination(route: String) =
wisemuji marked this conversation as resolved.
Show resolved Hide resolved
navController.currentDestination?.route == route

@Composable
fun shouldShowBottomBar(): Boolean {
val currentRoute = currentDestination?.route ?: return false
Expand Down
4 changes: 2 additions & 2 deletions feature/main/src/main/java/com/droidknights/app2023/feature/main/MainScreen.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,12 +95,12 @@ internal fun MainScreen(
)

contributorNavGraph(
onBackClick = { navigator.popBackStack() },
onBackClick = navigator::popBackStackIfNotHome,
onShowErrorSnackBar = onShowErrorSnackBar
)

sessionNavGraph(
onBackClick = { navigator.popBackStack() },
onBackClick = navigator::popBackStackIfNotHome,
onSessionClick = { navigator.navigateSessionDetail(it.id) },
onShowErrorSnackBar = onShowErrorSnackBar
)
Expand Down