Skip to content

[REF/#638] Feed ViewModel 내의 UI 스크롤 로직 분리 #699

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

Merged
nahy-512 merged 3 commits into from
Feb 23, 2026
Merged

[REF/#638] Feed ViewModel 내의 UI 스크롤 로직 분리 #699

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
e1b0d61
refactor: move scroll direction logic from ViewModel to UI layer
nahy-512 Feb 2, 2026
239f53d
Merge remote-tracking branch 'origin/develop' into ref/#638-feedviewm…
nahy-512 Feb 20, 2026
9d1f026
refactor: extract FeedScrollState for scroll direction logic
nahy-512 Feb 23, 2026
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
17 changes: 8 additions & 9 deletions presentation/feed/src/main/java/com/hilingual/presentation/feed/FeedScreen.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,6 @@ internal fun FeedRoute(
onUnpublishClick = viewModel::diaryUnpublish,
onReportClick = { context.launchCustomTabs(UrlConstant.FEEDBACK_REPORT) },
readAllFeed = viewModel::readAllFeed,
isScrollingDown = viewModel::isScrollingDown
)
}
}
Expand All @@ -185,7 +184,6 @@ private fun FeedScreen(
onReportClick: () -> Unit,
readAllFeed: () -> Unit,
onFeedRefresh: (FeedTab) -> Unit,
isScrollingDown: (FeedScrollState?, FeedScrollState) -> Boolean,
modifier: Modifier = Modifier
) {
val coroutineScope = rememberCoroutineScope()
Expand Down Expand Up @@ -238,15 +236,17 @@ private fun FeedScreen(

LaunchedEffect(pagerState.currentPage) {
snapshotFlow {
FeedScrollState(
firstVisibleItemIndex = currentListState.firstVisibleItemIndex,
firstVisibleItemScrollOffset = currentListState.firstVisibleItemScrollOffset
)
Pair(currentListState.firstVisibleItemIndex, currentListState.firstVisibleItemScrollOffset)
}
.pairwise()
.collect { (previous, current) ->
val isScrollingDown = previous != null && (
Copy link
Collaborator

@Hyobeen-Park Hyobeen-Park Feb 20, 2026

Choose a reason for hiding this comment

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

null 처리는 ?.let { } 로도 처리할 수 있을 것 같은데 어떤가요??

current.first > previous.first ||
(current.first == previous.first && current.second > previous.second)
Copy link
Collaborator

@Daljyeong Daljyeong Feb 21, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

저도 이 부분을 보면서 기존의 FeedScrollState와 같은 data class를 유지하는 게 좋을 것 같다고 생각했어요. Pair를 사용하면 first, second로 접근해야 하기에 의미를 바로 파악하기 조금 어려운 것 같아 이전처럼 네이밍을 활용해주면 가독성과 의도 전달이 더 좋아질 것 같습니다.

)

if (currentListState.isScrollInProgress &&
isScrollingDown(previous, current) &&
isScrollingDown &&
Copy link
Member

@angryPodo angryPodo Feb 20, 2026

Choose a reason for hiding this comment

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

View의 관심사로 이동했네요!
다만 스크롤 방향을 판단하는 로직이 LaunchedEffect 내부에 직접 구현되어 있어서 가독성이 조금 떨어진다고 생각해요.
이게 인덱스인지 오프셋인지 Pair만 보고도 헷갈릴 수도 있다고 생각해요. 기존에 State클래스를 사용해서 필드로 접근했던 방식이 가독성 면에서는 더 좋았던 것 같아요. 해당 데이터 클래스를 Screen 내부나 최하단에 private로 정의하고 판단 로직을 내부함수로 분리해서 사용하는건 어떨까요? 그러면 LaunchedEffect내부 코드가 더 깔끔해지고 의도가 명확해질 것 같습니다.👍🏻

Copy link
Member Author

@nahy-512 nahy-512 Feb 22, 2026

Choose a reason for hiding this comment

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

넵 말씀주신 방향으로 수정해보겠습니다!

isAtBottom
) {
latestReadAllFeed()
Expand Down Expand Up @@ -349,8 +349,7 @@ private fun FeedScreenPreview() {
hasFollowing = false,
recommendRefreshing = false,
followingRefreshing = false,
onFeedRefresh = {},
isScrollingDown = { _, _ -> false }
onFeedRefresh = {}
)
}
}
15 changes: 0 additions & 15 deletions presentation/feed/src/main/java/com/hilingual/presentation/feed/FeedViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,16 +188,6 @@ internal class FeedViewModel @Inject constructor(
}
}

fun isScrollingDown(previous: FeedScrollState?, current: FeedScrollState): Boolean {
if (previous == null) return false

return current.firstVisibleItemIndex > previous.firstVisibleItemIndex ||
(
current.firstVisibleItemIndex == previous.firstVisibleItemIndex &&
current.firstVisibleItemScrollOffset > previous.firstVisibleItemScrollOffset
)
}

private fun UiState<ImmutableList<FeedItemUiModel>>.updateIfSuccess(
transform: (ImmutableList<FeedItemUiModel>) -> ImmutableList<FeedItemUiModel>
): UiState<ImmutableList<FeedItemUiModel>> {
Expand Down Expand Up @@ -246,11 +236,6 @@ internal class FeedViewModel @Inject constructor(
}
}

internal data class FeedScrollState(
val firstVisibleItemIndex: Int,
val firstVisibleItemScrollOffset: Int
)

sealed interface FeedSideEffect {
data class ShowErrorDialog(val onRetry: () -> Unit) : FeedSideEffect

Expand Down