diff --git a/app/src/main/java/com/ubisam/ubgitea/feature/branchdetail/BranchDetailScreen.kt b/app/src/main/java/com/ubisam/ubgitea/feature/branchdetail/BranchDetailScreen.kt index 0b647e1..9f097db 100644 --- a/app/src/main/java/com/ubisam/ubgitea/feature/branchdetail/BranchDetailScreen.kt +++ b/app/src/main/java/com/ubisam/ubgitea/feature/branchdetail/BranchDetailScreen.kt @@ -19,6 +19,7 @@ import com.ubisam.ubgitea.domain.model.TreeEntry import com.ubisam.ubgitea.domain.model.CommitSummary import com.ubisam.ubgitea.feature.codebrowser.CodeBrowserScreen import com.ubisam.ubgitea.feature.codebrowser.CodeBrowserUiState +import com.ubisam.ubgitea.feature.codebrowser.content.ContentClassification import com.ubisam.ubgitea.feature.history.HistoryScreen import com.ubisam.ubgitea.feature.history.HistoryUiState import kotlinx.coroutines.launch @@ -53,6 +54,17 @@ fun BranchDetailScreen( ) { val pagerState = rememberPagerState(pageCount = { 2 }) val scope = rememberCoroutineScope() + // A page showing horizontally-scrollable source text (a plain file, or a diff's before/after + // panels) has its own left/right drag handling that directly conflicts with the pager's own — + // both are horizontal, so a drag meant to scroll code was instead partly consumed by the pager, + // which is what made scrolling code feel jittery and occasionally scroll text out of view + // entirely. Swiping between tabs only makes sense one level up from that anyway (Files' folder + // listing, History's changed-file list), so the pager's own gesture is disabled while either + // tab is showing scrollable code, and re-enabled once back at that list. + val currentPageShowsScrollableCode = when (pagerState.currentPage) { + 0 -> codeBrowserState is CodeBrowserUiState.File && codeBrowserState.content is ContentClassification.Text + else -> historyState is HistoryUiState.CommitDetailContent && historyState.selectedFile != null + } Column(modifier.fillMaxSize().statusBarsPadding()) { TabRow(selectedTabIndex = pagerState.currentPage) { Tab( @@ -66,7 +78,11 @@ fun BranchDetailScreen( text = { Text(stringResource(R.string.branchdetail_tab_history)) }, ) } - HorizontalPager(state = pagerState, modifier = Modifier.weight(1f)) { page -> + HorizontalPager( + state = pagerState, + userScrollEnabled = !currentPageShowsScrollableCode, + modifier = Modifier.weight(1f), + ) { page -> when (page) { 0 -> CodeBrowserScreen( state = codeBrowserState, diff --git a/docs/development-log.md b/docs/development-log.md index 97e42e4..cce5b77 100644 --- a/docs/development-log.md +++ b/docs/development-log.md @@ -413,3 +413,16 @@ This is the append-only implementation and handoff record shared by Codex and Cl - Known limitations: While investigating, also confirmed that partial characters get visually clipped at the viewport edge when scrolled to a non-character-aligned pixel position — this is expected behavior for any horizontally-scrolled monospace text (the same happens in GitHub's and VS Code's own diff/code viewers) and was not treated as a defect, though it was more noticeable while the direction bug was still live and scroll landed at unusual positions. - Next recommended task: Get the human product owner's live confirmation of the diff view (jitter-free and now correctly directional) alongside the rest of the TASK-025 batch and the TASK-026 Custom Tabs login flow, then continue to the two remaining queued items (image viewer, Issues/PR numbered pagination). - Next owner: Claude, continuing per the standing direction. + +## 2026-08-27 — TASK-025 follow-up: fix tab-swipe vs. code-scroll gesture conflict + +- Agent and role: Claude, live device testing continued directly from the direction-fix follow-up above; this time the human product owner watched the same swipe testing and reported "이거 왤케 떨리냐 화면이...?" (why is the screen trembling so much), then, after Claude's own scripted-swipe screenshots didn't reproduce anything conclusive, correctly root-caused it themselves: "저거 히스토리랑 파일 좌우 탭전환 기능이랑 코드 좌우 스크롤 기능이 섞이면서 그런 것 같은데" (the History/Files tab-swipe and the code's own left/right scroll are colliding). +- Commit/diff: continued on `claude/task-015-notifications`; `feature/branchdetail/BranchDetailScreen.kt` only. +- Bug: `BranchDetailScreen`'s `HorizontalPager` (Files/History tab-swipe) and the diff view's own horizontal code scroll (from the two prior follow-ups) are both horizontal gestures layered on the same screen. Nested, they both tried to consume the same drag — visible as jittery/trembling scrolling, and separately (per a second report in the same message) as source text sometimes scrolling out of view entirely while viewing code. +- Fix: the human product owner specified the exact intended boundary directly — disable tab-swiping only while actual source code is on screen (a file's contents on the Files tab, or a selected file's diff on the History tab), keep it enabled everywhere else (folder listings, the commit list, a commit's changed-file list). Implemented as `HorizontalPager(userScrollEnabled = !currentPageShowsScrollableCode, ...)`, where `currentPageShowsScrollableCode` reads the already-available `codeBrowserState`/`historyState` (both were already being collected in `MainActivity` and passed into this screen, so no new state plumbing was needed) depending on which page is current. Tapping a tab directly still always works, since that path is `animateScrollToPage`, not a gesture. +- Files changed: `feature/branchdetail/BranchDetailScreen.kt`, `tasks/025-branch-detail-tabs.md` (evidence updated in place, same pattern as the two prior diff-scroll follow-ups). +- Checks and results: `./gradlew assembleDebug testDebugUnitTest lintDebug` BUILD SUCCESSFUL, all unit tests passing, 0 lint errors. Reinstalled on the human product owner's device over wireless `adb`; verified directly via scripted `adb shell input swipe` in both directions on both the code view (tab-swipe correctly disabled, code scrolls cleanly) and the file-list view (tab-swipe correctly still works) — an initial version of this same verification looked like a regression (swiping didn't switch tabs on the list view either) until re-checked: that test had swiped in the direction of a page that doesn't exist (already at the last page), not evidence of a real bug. +- Review findings and resolutions: None from a separate review pass. Notably, the human product owner diagnosed the actual root cause directly from watching the live behavior, ahead of Claude's own investigation — Claude's scripted-swipe screenshot sampling (∼1s intervals) had already been established as too coarse to catch genuine per-frame jitter, so the human's direct observation was the deciding signal here, not device automation. +- Known limitations: Whether the trembling is now fully gone (as opposed to just less likely to occur) has not been separately re-confirmed live by the human product owner as of this commit — the scripted-swipe verification confirms the gesture-ownership conflict is structurally resolved (only one consumer is ever active at a time now), but a live human touch test is the only way to confirm the felt result. +- Next recommended task: Get the human product owner's live confirmation of the whole TASK-025 batch plus this fix and the TASK-026 Custom Tabs login flow, together in one pass, then continue to the two remaining queued items (image viewer, Issues/PR numbered pagination). +- Next owner: Claude, continuing per the standing direction. diff --git a/tasks/025-branch-detail-tabs.md b/tasks/025-branch-detail-tabs.md index 72ece22..003b2b8 100644 --- a/tasks/025-branch-detail-tabs.md +++ b/tasks/025-branch-detail-tabs.md @@ -12,7 +12,7 @@ Three more live requests from the same testing session: 2. Each repository row on the repository list now shows its last-commit time (reusing the existing `updatedAtEpochMillis` field and the shared `formatInstant` helper from TASK-024 — no new API data needed). 3. The repository list's 전체(All)/즐겨찾기(Favorites) tabs are now swipeable (swipe right from All → Favorites, swipe left from Favorites → All), not just tap-to-switch. -Plus six more follow-up fixes/changes found or requested during the same live-testing pass on the same branch, kept in this task rather than split out since they're all small, tightly related touch-ups to screens this task already modifies or reuses: +Plus seven more follow-up fixes/changes found or requested during the same live-testing pass on the same branch, kept in this task rather than split out since they're all small, tightly related touch-ups to screens this task already modifies or reuses: 4. **Overlap bug fix**: the narrow-layout (non-Z-Fold-unfolded) branch of `CommitDetailScreen`'s `BoxWithConstraints` had `Header(...)` and its content as bare siblings instead of children of a `Column`, so `BoxWithConstraints` (which behaves like `Box` — it does not arrange multi-child branches vertically on its own) rendered them stacked on top of each other. Reported twice by the human product owner via screenshot before being root-caused; fixed by wrapping each branch's contents in `Column(Modifier.fillMaxSize()) { ... }`. 5. **Status-bar padding fix**: `BranchDetailScreen`'s `TabRow` was jammed against the status bar because, unlike every other top-level screen in the app, it wasn't wrapped in its own `Scaffold`. Fixed with `Modifier.statusBarsPadding()` on the outer `Column` (not a nested `Scaffold`, to avoid double-padding since `CodeBrowserScreen`/`HistoryScreen` already have their own `Scaffold`s internally). @@ -20,10 +20,11 @@ Plus six more follow-up fixes/changes found or requested during the same live-te 7. **Loading-text simplification**: the references-loading message on the repository screen previously interpolated the repository name into a longer sentence; simplified to a plain "로딩 중" per direct request, and the now-unused `%1$s` placeholder removed from `repositories_references_loading`. 8. **In-app back buttons removed**: every screen-header back button (`CodeBrowserScreen`'s `BrowserHeader`, `HistoryScreen`'s `Header`, `NotificationsScreen`'s `FilterTabs`, `WorkItemsScreen`'s `ModeTabs` and `DetailHeader`, `RepositoryListScreen`'s `ReferencesContent` header) removed, since every screen already has a `BackHandler` wired to the system back gesture and the human product owner asked to rely on that instead. `WorkItemsScreen`'s `DetailHeader` composable (which consisted only of a back button) removed entirely along with all 5 call sites. Now-unused `history_back`/`notifications_back`/`workitems_back`/`codebrowser_back` string resources removed; `repositories_back` kept, since it's a distinct recovery action on the references-error state's secondary button, not a header back button. 9. **Pull-to-refresh replaces manual refresh buttons**: `RepositoryListScreen`'s `RepositoryContent` and `ReferencesContent` each had a manual "새로고침" `OutlinedButton`; both replaced with `PullToRefreshBox` (Material3, `@OptIn(ExperimentalMaterial3Api::class)`) wrapping their `LazyColumn`s, so pulling down from the top refreshes instead. +10. **Tab-swipe vs. code-scroll gesture conflict fixed**: the Files/History `HorizontalPager` and the diff view's horizontal code scroll (item 6) are both horizontal gestures over the same screen; nested, they fought over every drag — reported live as visible "trembling" and, separately, source text intermittently scrolling out of view entirely. Root-caused (by the human product owner, from directly watching it happen) as the pager and the inner scroll both trying to consume the same drag. Fixed by disabling the pager's own swipe gesture (`userScrollEnabled = false`) whenever the currently-visible tab is showing horizontally-scrollable source text — a code file's contents on the Files tab, or a selected diff on the History tab — and re-enabling it everywhere else (folder listings, the commit list, a commit's changed-file list), so tab-swiping still works right up to the point actual code is on screen. Tapping a tab always still works regardless, since that's a direct `animateScrollToPage` call, not a gesture. Verified in both directions live via scripted `adb shell input swipe` (an earlier same-session test of this had looked like a regression, but was actually just a swipe fired in the wrong direction from the last page, which has nowhere further to go — not a real bug). ## Scope -- Included: `ui/navigation/Route.kt`/`RouteStackSaver.kt` (`Route.CodeBrowser`/`Route.History` replaced by one `Route.BranchDetail`), new `feature/branchdetail/BranchDetailScreen.kt`, `MainActivity.kt` wiring, `strings_branchdetail.xml`, plus the follow-up touch-ups (items 4-9 above) to `feature/history/HistoryScreen.kt`, `feature/codebrowser/CodeBrowserScreen.kt`, `feature/notifications/NotificationsScreen.kt`, `feature/workitems/WorkItemsScreen.kt`, `feature/repositories/RepositoryListScreen.kt`, and their string resources. +- Included: `ui/navigation/Route.kt`/`RouteStackSaver.kt` (`Route.CodeBrowser`/`Route.History` replaced by one `Route.BranchDetail`), new `feature/branchdetail/BranchDetailScreen.kt`, `MainActivity.kt` wiring, `strings_branchdetail.xml`, plus the follow-up touch-ups (items 4-10 above) to `feature/branchdetail/BranchDetailScreen.kt` (item 10), `feature/history/HistoryScreen.kt`, `feature/codebrowser/CodeBrowserScreen.kt`, `feature/notifications/NotificationsScreen.kt`, `feature/workitems/WorkItemsScreen.kt`, `feature/repositories/RepositoryListScreen.kt`, and their string resources. - Excluded: No change to `CodeBrowserViewModel`/`HistoryViewModel` internals — both are reused exactly as they were, including their own internal drill-down (subfolder navigation, commit detail) and their own per-state `BackHandler`s. This screen only adds the `TabRow` + `HorizontalPager` chrome around them. No change to actual refresh/pagination/network logic — item 9 only changes the trigger UI, not what `onRefresh` does. ## Design notes @@ -73,7 +74,8 @@ Plus six more follow-up fixes/changes found or requested during the same live-te - `./gradlew compileDebugKotlin`: BUILD SUCCESSFUL, both after the `@OptIn(ExperimentalMaterial3Api::class)` fix for `PullToRefreshBox` and again after the diff-scroll rewrite. - `./gradlew assembleDebug testDebugUnitTest lintDebug` (`--rerun-tasks` on the first full pass, incremental on the diff-scroll-rewrite follow-up): BUILD SUCCESSFUL both times, all unit tests passing, 0 lint errors. - `HistoryScreen.kt`'s diff-scroll `rememberScrollableState` (third pass): verified live (by directly swiping the device over `adb`) that the jitter-free scroll from the second pass ran in the wrong direction — dragging left revealed the start of the line instead of further along it, backwards from every other horizontally-scrollable surface on Android. Root cause: `newOffset = horizontalOffsetPx + delta` had the sign backwards for this gesture; fixed to `newOffset = horizontalOffsetPx - delta`, confirmed correct by the same live-swipe method (drag left now reveals later content, drag right returns toward the start, both panels stay in sync). -- Live device: build installed over wireless `adb` (`adb-RFCYA0RD1CZ-ztOzAA._adb-tls-connect._tcp`, Galaxy Z Fold) four times across this task — initial batch, after the diff-scroll jitter fix, and again after the direction fix, the last verified directly via scripted `adb shell input swipe` plus before/after screenshots rather than waiting on a live report. Human product owner asked to confirm the rest of the batch (tabs, overlap fix, status-bar padding, back-button removal, pull-to-refresh) — result pending as of this commit. +- `BranchDetailScreen.kt`: `HorizontalPager` gained `userScrollEnabled = !currentPageShowsScrollableCode`, where `currentPageShowsScrollableCode` checks `codeBrowserState is CodeBrowserUiState.File && codeBrowserState.content is ContentClassification.Text` on page 0, or `historyState is HistoryUiState.CommitDetailContent && historyState.selectedFile != null` on page 1 — both states were already being collected in `MainActivity` and passed into this screen, so no new state plumbing was needed. +- Live device: build installed over wireless `adb` (`adb-RFCYA0RD1CZ-ztOzAA._adb-tls-connect._tcp`, Galaxy Z Fold) five times across this task — initial batch, after the diff-scroll jitter fix, after the direction fix, and after the tab-swipe/code-scroll conflict fix. The last two were verified directly via scripted `adb shell input swipe` plus before/after screenshots (the human product owner requested this directly — "좌우 스크롤 해서 이상한걸 감지해볼래?" — rather than reporting a symptom first) instead of waiting on a live report; the human product owner then separately diagnosed the tab-swipe/scroll conflict's root cause from watching it happen and directed the fix. Human product owner asked to confirm the rest of the batch (tabs, overlap fix, status-bar padding, back-button removal, pull-to-refresh) together — result pending as of this commit. ## Review findings