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 31d4f74..a0d771e 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 @@ -26,7 +26,7 @@ import kotlinx.coroutines.launch /** * One branch/tag/commit ref, shown as two swipeable tabs — Files (the code browser, default) and - * History — over the same ref. Each tab keeps its own existing ViewModel and internal drill-down + * Commit — over the same ref. Each tab keeps its own existing ViewModel and internal drill-down * (a subfolder, a commit's detail) entirely as before; this screen only adds the tab chrome and * the pager, and only intercepts "back" once a tab's own internal back-handling has nothing left * to unwind (see `onCodeBrowserBack`/`onHistoryBack`). @@ -97,7 +97,6 @@ fun BranchDetailScreen( onRetry = onRetryCodeBrowser, onBack = onCodeBrowserBack, onAuthenticationExpired = onAuthenticationExpired, - onViewHistory = { scope.launch { pagerState.animateScrollToPage(1) } }, onNavigateImage = onNavigateImage, ) else -> HistoryScreen( diff --git a/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/CodeBrowserScreen.kt b/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/CodeBrowserScreen.kt index bbe186a..7a78e81 100644 --- a/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/CodeBrowserScreen.kt +++ b/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/CodeBrowserScreen.kt @@ -96,7 +96,6 @@ fun CodeBrowserScreen( onRetry: () -> Unit, onBack: () -> Unit, onAuthenticationExpired: () -> Unit, - onViewHistory: (CodeBrowserTarget) -> Unit, onNavigateImage: (Int) -> Unit, modifier: Modifier = Modifier, ) { @@ -106,11 +105,11 @@ fun CodeBrowserScreen( when (state) { CodeBrowserUiState.Idle -> Unit is CodeBrowserUiState.Loading -> { - BrowserHeader(state.target, state.path, state.isFile, onBreadcrumbSelected, onViewHistory) + BrowserHeader(state.target, state.path, state.isFile, onBreadcrumbSelected) CenterMessage { CircularProgressIndicator() } } is CodeBrowserUiState.Directory -> { - BrowserHeader(state.target, state.path, false, onBreadcrumbSelected, onViewHistory) + BrowserHeader(state.target, state.path, false, onBreadcrumbSelected) if (state.entries.isEmpty()) { CenterMessage { Text(stringResource(R.string.codebrowser_directory_empty)) } } else { @@ -122,11 +121,11 @@ fun CodeBrowserScreen( } } is CodeBrowserUiState.File -> { - BrowserHeader(state.target, state.path, true, onBreadcrumbSelected, onViewHistory) + BrowserHeader(state.target, state.path, true, onBreadcrumbSelected) FileContent(state.content, state.path, state.siblingImagePaths, onNavigateImage) } is CodeBrowserUiState.UnsupportedEntry -> { - BrowserHeader(state.target, state.path, true, onBreadcrumbSelected, onViewHistory) + BrowserHeader(state.target, state.path, true, onBreadcrumbSelected) CenterMessage { Text( stringResource( @@ -137,7 +136,7 @@ fun CodeBrowserScreen( } } is CodeBrowserUiState.Error -> { - BrowserHeader(state.target, state.path, state.isFile, onBreadcrumbSelected, onViewHistory) + BrowserHeader(state.target, state.path, state.isFile, onBreadcrumbSelected) CenterMessage { Text(stringResource(state.type.messageResource)) if (state.type == CodeBrowserError.AUTHENTICATION_EXPIRED) { @@ -160,21 +159,15 @@ private fun BrowserHeader( path: RepositoryPath, leafIsFile: Boolean, onBreadcrumbSelected: (RepositoryPath) -> Unit, - onViewHistory: (CodeBrowserTarget) -> Unit, ) { Column(Modifier.fillMaxWidth().padding(16.dp)) { - Row(verticalAlignment = Alignment.CenterVertically) { - Column(Modifier.weight(1f)) { - Text("${target.repository.owner}/${target.repository.name}", style = MaterialTheme.typography.titleLarge) - val refKind = when (target.kind) { - CodeReferenceKind.BRANCH -> stringResource(R.string.codebrowser_ref_branch) - CodeReferenceKind.TAG -> stringResource(R.string.codebrowser_ref_tag) - CodeReferenceKind.COMMIT -> stringResource(R.string.codebrowser_ref_commit) - } - Text(stringResource(R.string.codebrowser_ref, refKind, target.ref.value)) - } - OutlinedButton(onClick = { onViewHistory(target) }) { Text(stringResource(R.string.codebrowser_history)) } + Text("${target.repository.owner}/${target.repository.name}", style = MaterialTheme.typography.titleLarge) + val refKind = when (target.kind) { + CodeReferenceKind.BRANCH -> stringResource(R.string.codebrowser_ref_branch) + CodeReferenceKind.TAG -> stringResource(R.string.codebrowser_ref_tag) + CodeReferenceKind.COMMIT -> stringResource(R.string.codebrowser_ref_commit) } + Text(stringResource(R.string.codebrowser_ref, refKind, target.ref.value)) Row(Modifier.horizontalScroll(rememberScrollState()).padding(top = 12.dp)) { val breadcrumbs = CodeBrowserNavigation.breadcrumbs(path) breadcrumbs.forEachIndexed { index, crumb -> diff --git a/app/src/main/res/values/strings_codebrowser.xml b/app/src/main/res/values/strings_codebrowser.xml index 29c361c..0b03594 100644 --- a/app/src/main/res/values/strings_codebrowser.xml +++ b/app/src/main/res/values/strings_codebrowser.xml @@ -25,7 +25,6 @@ 서버 응답을 처리하지 못했습니다. 다시 시도 다시 로그인 - 커밋 %1$d / %2$d 이미지를 표시할 수 없습니다. 이 CSV 파일은 비어 있습니다. diff --git a/docs/development-log.md b/docs/development-log.md index 9634784..da819f2 100644 --- a/docs/development-log.md +++ b/docs/development-log.md @@ -605,3 +605,16 @@ This is the append-only implementation and handoff record shared by Codex and Cl - Known limitations: unchanged from earlier in this task — still optimistic rather than server-authoritative in both directions (accepted tradeoff), and the underlying "can't tell if already starred on load" limitation from TASK-018/TASK-030 is untouched. - Next recommended task: get live confirmation that starring/un-starring now feels instant. - Next owner: Claude, continuing per the standing direction. + +## 2026-08-29 (continued) — TASK-035: remove the redundant per-file "커밋" button in the code browser + +- Agent and role: Claude, direct continuation of the same live session — "현재 보고있는 페이지 우상단에 커밋 버튼은 이제 필요 없지 않나" (the Commit button in the top-right of the current page isn't needed anymore, is it). +- Commit/diff: on `claude/task-015-notifications`; `feature/codebrowser/CodeBrowserScreen.kt`, `feature/branchdetail/BranchDetailScreen.kt`, `strings_codebrowser.xml`, `tasks/035-remove-redundant-code-browser-commit-button.md`. +- Confirmed the observation was correct before acting on it: traced the header's "커밋" button (`onViewHistory`) all the way to its single call site in `BranchDetailScreen.kt` and found it just switched the pager to page 1 — the exact same action the top-level "커밋" tab's own `onClick` already performs a few lines above it. TASK-031's rename (both controls used to be independently labeled "히스토리"/differently-purposed before that) is what made the duplication visually obvious. +- How it was implemented: removed the button and the `onViewHistory` parameter threaded through `CodeBrowserScreen`/`BrowserHeader`, the call site in `BranchDetailScreen.kt`, and the now-unused `codebrowser_history` string resource (confirmed unused via a repo-wide search first). `BrowserHeader`'s title/ref layout simplified from a `Row`-wrapped `Column` to a plain `Column` now that there's no trailing button to share the row with. Also fixed a stale "History" word in a doc comment one line above (predating TASK-031's rename). +- Files changed: `CodeBrowserScreen.kt`, `BranchDetailScreen.kt`, `strings_codebrowser.xml`, `tasks/035-remove-redundant-code-browser-commit-button.md`. +- Checks and results: `./gradlew compileDebugKotlin compileDebugUnitTestKotlin testDebugUnitTest assembleDebug lintDebug` BUILD SUCCESSFUL, all unit tests passing, 0 lint errors. Installed on the human product owner's device over wireless `adb`. +- Review findings and resolutions: None from a separate review pass. +- Known limitations: None identified. +- Next recommended task: get live confirmation the header looks right without the button; otherwise continue per the standing direction on the rest of this session's queued items. +- Next owner: Claude, continuing per the standing direction. diff --git a/tasks/035-remove-redundant-code-browser-commit-button.md b/tasks/035-remove-redundant-code-browser-commit-button.md new file mode 100644 index 0000000..1be5b1a --- /dev/null +++ b/tasks/035-remove-redundant-code-browser-commit-button.md @@ -0,0 +1,84 @@ +# TASK-035: Remove the redundant per-file "커밋" button in the code browser + +Status: implemented; all required checks pass; live-installed, awaiting the human product owner's confirmation +Owner: Claude +Reviewer: Codex + +## Outcome + +Live-reported: "현재 보고있는 페이지 우상단에 커밋 버튼은 이제 필요 없지 않나" (the Commit button +in the top-right of the current page isn't needed anymore, is it). Confirmed correct: the code +browser's header showed an `OutlinedButton` (originally labeled "히스토리", renamed to "커밋" by +TASK-031) that just switched `BranchDetailScreen`'s pager to page 1 — the exact same destination +the top-level "커밋" tab (also renamed by TASK-031) already switches to with one tap, with no +file-specific filtering or other distinct behavior. Since TASK-031 gave both controls the identical +label doing the identical thing, the in-content button became pure duplication. Removed it. + +## Scope + +- Included: `feature/codebrowser/CodeBrowserScreen.kt` (`BrowserHeader`'s button and the + `onViewHistory` parameter threaded through `CodeBrowserScreen`), `feature/branchdetail/BranchDetailScreen.kt` + (the call site, plus a doc-comment word fix from a prior rename), `strings_codebrowser.xml` + (removed the now-unused `codebrowser_history` string). +- Excluded: The top-level "커밋" tab and `HistoryScreen` itself are untouched — this removes only + the duplicate shortcut, not the feature it duplicated. No change to any other button or gesture + in the code browser. + +## Design notes + +- **Confirmed it was truly redundant before removing it, not just probably**: traced + `onViewHistory` all the way from `BrowserHeader`'s button through `CodeBrowserScreen`'s parameter + to its one call site in `BranchDetailScreen.kt` — + `onViewHistory = { scope.launch { pagerState.animateScrollToPage(1) } }` — which is exactly the + same action the "커밋" `Tab`'s own `onClick` performs a few lines above it. No path/file + information ever flowed through `onViewHistory`, so this was never actually "view this specific + file's history," despite once being labeled that way before TASK-031's rename made the + duplication obvious. +- **Header simplified along with the removal**: `BrowserHeader`'s title/ref `Column` no longer + needs to share a `Row` with a trailing button, so it collapsed back to a plain `Column` — one + less nested layout, not just a deleted button. + +## Acceptance criteria + +- [x] The code browser's header (shown across Loading/Directory/File/UnsupportedEntry/Error states) + no longer shows a "커밋" button. +- [x] The top-level "커밋" tab in `BranchDetailScreen` is unaffected and remains the only way to + reach commit history from this screen. +- [x] No leftover references to `onViewHistory` or the removed `codebrowser_history` string + resource anywhere in the app. +- [x] Build, unit tests, and lint pass. +- [ ] Live confirmation from the human product owner that the header now looks right without it — + installed on the device but not yet re-viewed in this exact session. + +## Required checks + +```text +./gradlew compileDebugKotlin compileDebugUnitTestKotlin +./gradlew testDebugUnitTest +./gradlew assembleDebug lintDebug +``` + +## Evidence + +- `CodeBrowserScreen.kt`: `onViewHistory` parameter removed from `CodeBrowserScreen` and + `BrowserHeader`; the `OutlinedButton` removed; `BrowserHeader`'s title/ref layout simplified from + a `Row`-wrapped `Column` to a plain `Column`. +- `BranchDetailScreen.kt`: the `onViewHistory = { ... }` argument removed from the one + `CodeBrowserScreen(...)` call site; a stale doc-comment reference to "History" (predating + TASK-031's tab rename) corrected to "Commit". +- `strings_codebrowser.xml`: `codebrowser_history` string removed (confirmed unused via a + repository-wide search before deleting it). +- `./gradlew compileDebugKotlin compileDebugUnitTestKotlin`: BUILD SUCCESSFUL. +- `./gradlew testDebugUnitTest`: BUILD SUCCESSFUL, all unit tests passing (no test referenced the + removed button/parameter/string). +- `./gradlew assembleDebug lintDebug`: BUILD SUCCESSFUL, 0 lint errors. +- Installed on the human product owner's device over wireless `adb` (`adb install -r`, streamed + install succeeded); not yet re-viewed live in this exact session. + +## Known limitations + +None identified. + +## Review findings + +None yet — pending Codex/human review.