From 8bf3930c3d3490f7d1a253eb63470689a846ddd1 Mon Sep 17 00:00:00 2001 From: choiminyeong Date: Thu, 27 Aug 2026 22:01:09 +0900 Subject: [PATCH] TASK-027 follow-up: pinch-to-zoom and pan for the image viewer Pinch-zoom (1x-5x), pan while zoomed, and double-tap to toggle zoom, added to the image viewer. Combined with the existing swipe-navigate gesture in one hand-rolled awaitEachGesture loop that branches on live pointer count, rather than stacking a second competing pointerInput detector -- avoiding the same class of tab-swipe/content-scroll gesture conflict TASK-025 hit twice. Co-Authored-By: Claude Sonnet 5 --- .../feature/codebrowser/CodeBrowserScreen.kt | 109 +++++++++++++++--- docs/development-log.md | 13 +++ tasks/027-image-and-csv-viewer.md | 109 +++++++++++++----- 3 files changed, 181 insertions(+), 50 deletions(-) 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 46130af..11e1777 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 @@ -7,7 +7,11 @@ import androidx.compose.foundation.background import androidx.compose.foundation.clickable import androidx.compose.foundation.gestures.Orientation import androidx.compose.foundation.gestures.ScrollableState +import androidx.compose.foundation.gestures.awaitEachGesture +import androidx.compose.foundation.gestures.calculatePan +import androidx.compose.foundation.gestures.calculateZoom import androidx.compose.foundation.gestures.detectHorizontalDragGestures +import androidx.compose.foundation.gestures.detectTapGestures import androidx.compose.foundation.gestures.rememberScrollableState import androidx.compose.foundation.gestures.scrollable import androidx.compose.foundation.horizontalScroll @@ -36,14 +40,22 @@ import androidx.compose.material3.Scaffold import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.State +import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableFloatStateOf +import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clipToBounds +import androidx.compose.ui.geometry.Offset import androidx.compose.ui.graphics.asImageBitmap +import androidx.compose.ui.graphics.graphicsLayer import androidx.compose.ui.input.pointer.pointerInput +import androidx.compose.ui.input.pointer.positionChange +import androidx.compose.ui.input.pointer.positionChanged import androidx.compose.ui.layout.ContentScale +import androidx.compose.ui.layout.onSizeChanged import androidx.compose.ui.platform.LocalClipboardManager import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.res.stringResource @@ -52,6 +64,7 @@ import androidx.compose.ui.text.AnnotatedString import androidx.compose.ui.text.font.FontFamily import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.unit.IntOffset +import androidx.compose.ui.unit.IntSize import androidx.compose.ui.unit.dp import com.ubisam.ubgitea.R import com.ubisam.ubgitea.domain.model.RepositoryPath @@ -238,6 +251,11 @@ private fun ImageContent(content: ContentClassification.Image, currentIndex: Int BitmapFactory.decodeByteArray(bytes, 0, bytes.size)?.asImageBitmap() } val canNavigate = currentIndex != -1 && totalCount > 1 + // Reset to unzoomed whenever the displayed image changes (a fresh open, or navigating within + // the gallery) rather than carrying the previous image's zoom/pan over to this one. + var scale by remember(content) { mutableFloatStateOf(1f) } + var panOffset by remember(content) { mutableStateOf(Offset.Zero) } + var boxSizePx by remember(content) { mutableStateOf(IntSize.Zero) } Column(Modifier.fillMaxSize()) { Row( Modifier.fillMaxWidth().padding(horizontal = 16.dp, vertical = 8.dp), @@ -249,31 +267,72 @@ private fun ImageContent(content: ContentClassification.Image, currentIndex: Int } } Box( - Modifier.fillMaxSize().let { base -> - if (canNavigate) { - // A discrete "which way, how far" swipe — not a continuous scroll position like - // the diff view's — so a plain drag-distance threshold is enough; no shared - // offset state (and none of that pattern's per-row bound bookkeeping) needed. - base.pointerInput(content) { - var accumulatedDragPx = 0f - detectHorizontalDragGestures( - onDragEnd = { - if (accumulatedDragPx <= -ImageSwipeThresholdPx) onNavigateImage(1) - else if (accumulatedDragPx >= ImageSwipeThresholdPx) onNavigateImage(-1) - accumulatedDragPx = 0f - }, - onHorizontalDrag = { _, dragAmount -> accumulatedDragPx += dragAmount }, - ) + Modifier.fillMaxSize() + .onSizeChanged { boxSizePx = it } + // One gesture loop, not two competing detectors: it decides per pointer-event + // whether this is a pinch (2+ pointers — always zoom/pan), a single-finger pan + // while already zoomed in, or — only at 1x with exactly one pointer — an + // accumulating horizontal drag that may fire gallery swipe-navigation on release. + // A single-finger drag at 1x never reaches transform handling, and a pinch never + // reaches swipe handling, so neither can steal the other's gesture. + .pointerInput(content, canNavigate) { + awaitEachGesture { + var horizontalDragPx = 0f + var pinched = false + do { + val event = awaitPointerEvent() + val pointerCount = event.changes.count { it.pressed } + when { + pointerCount >= 2 -> { + pinched = true + val zoomChange = event.calculateZoom() + val panChange = event.calculatePan() + val newScale = (scale * zoomChange).coerceIn(1f, MaxImageScale) + scale = newScale + panOffset = (panOffset + panChange).clampedToBox(newScale, boxSizePx) + event.changes.forEach { if (it.positionChanged()) it.consume() } + } + pointerCount == 1 && scale > 1f -> { + val change = event.changes.first() + panOffset = (panOffset + change.positionChange()).clampedToBox(scale, boxSizePx) + change.consume() + } + pointerCount == 1 && !pinched && canNavigate -> { + val change = event.changes.first() + horizontalDragPx += change.positionChange().x + change.consume() + } + } + } while (event.changes.any { it.pressed }) + if (!pinched && scale <= 1f && canNavigate) { + if (horizontalDragPx <= -ImageSwipeThresholdPx) onNavigateImage(1) + else if (horizontalDragPx >= ImageSwipeThresholdPx) onNavigateImage(-1) + } } - } else { - base } - }, + .pointerInput(content) { + detectTapGestures( + onDoubleTap = { + scale = if (scale > 1f) 1f else DoubleTapZoomScale + panOffset = Offset.Zero + }, + ) + }, contentAlignment = Alignment.Center, ) { val decoded = bitmap if (decoded != null) { - Image(decoded, contentDescription = null, contentScale = ContentScale.Fit, modifier = Modifier.fillMaxSize()) + Image( + decoded, + contentDescription = null, + contentScale = ContentScale.Fit, + modifier = Modifier.fillMaxSize().graphicsLayer { + scaleX = scale + scaleY = scale + translationX = panOffset.x + translationY = panOffset.y + }, + ) } else { Text(stringResource(R.string.codebrowser_image_decode_failed)) } @@ -281,6 +340,16 @@ private fun ImageContent(content: ContentClassification.Image, currentIndex: Int } } +// Keeps a zoomed image from being panned so far that it leaves the viewport entirely — an +// approximation (treats the image as filling the box, which ContentScale.Fit only does exactly +// along one axis for most aspect ratios) rather than exact bounds, which is a reasonable trade +// for how simple it keeps this over computing the fitted image rect precisely. +private fun Offset.clampedToBox(scale: Float, boxSizePx: IntSize): Offset { + val maxX = (boxSizePx.width * (scale - 1) / 2).coerceAtLeast(0f) + val maxY = (boxSizePx.height * (scale - 1) / 2).coerceAtLeast(0f) + return Offset(x.coerceIn(-maxX, maxX), y.coerceIn(-maxY, maxY)) +} + @Composable private fun CsvTable(content: ContentClassification.Csv) { if (content.rows.isEmpty()) { @@ -372,6 +441,8 @@ private fun CenterMessage(content: @Composable ColumnScope.() -> Unit) { } private const val ImageSwipeThresholdPx = 120f +private const val MaxImageScale = 5f +private const val DoubleTapZoomScale = 2f private val CsvCellWidth = 120.dp private val CodeBrowserError.messageResource: Int diff --git a/docs/development-log.md b/docs/development-log.md index 143c22a..13aa4ef 100644 --- a/docs/development-log.md +++ b/docs/development-log.md @@ -452,3 +452,16 @@ This is the append-only implementation and handoff record shared by Codex and Cl - Known limitations: Checked the Issues and Pull Request tabs on four of the human product owner's actual repositories live; every one had zero issues and zero pull requests, so the numbered-pagination UI was never actually populated with real data this session — confirmed instead that the screen doesn't crash and correctly falls back to the existing empty state on all four. Relying on the unit test coverage above (header parsing, total-page-count math, jump behavior) until a repository with real issues/PRs is available to confirm the rendered UI itself. - Next recommended task: Get live confirmation of both this and TASK-027 (or supply/point to a repository with real issues/PRs and an image/CSV file), otherwise continue per the standing direction — no other items remain on the queued list from this session. - Next owner: Claude, continuing per the standing direction. + +## 2026-08-27 — TASK-027 follow-up: pinch-to-zoom and pan for the image viewer + +- Agent and role: Claude, direct continuation of the same live session — the human product owner confirmed the image opened correctly, then asked for zoom directly ("확대 축소 기능도 만들어줄래?"). +- Commit/diff: continued on `claude/task-015-notifications`; `feature/codebrowser/CodeBrowserScreen.kt` only. +- Feature implemented: pinch-to-zoom (1x–5x) with pan while zoomed in, and double-tap to toggle between 1x and 2x — added to the image viewer from the earlier TASK-027 commit. +- How it was implemented: rather than stacking a separate zoom/pan gesture detector alongside the existing swipe-navigate one (which would have reintroduced exactly the class of bug TASK-025 hit twice — two independent `pointerInput` detectors both trying to consume the same single-finger drag), all three behaviors — pinch-zoom, pan-while-zoomed, and swipe-navigate — were combined into one hand-rolled `awaitEachGesture` loop that reads live pointer count each event and branches: 2+ pointers is always pinch-zoom/pan (`calculateZoom`/`calculatePan`); 1 pointer while already zoomed in pans; 1 pointer at 1x is reserved for the pre-existing swipe-navigate drag accumulator. A double-tap toggle was added via a second, non-competing `pointerInput` (`detectTapGestures` doesn't accumulate drag distance, so it doesn't fight the first block). Zoom/pan state is `remember(content)`-scoped so navigating to a different image in the gallery always starts that image at 1x rather than carrying over the previous image's zoom. Pan is clamped to an approximate (not pixel-exact) bound to keep a zoomed image roughly on screen. +- Files changed: `feature/codebrowser/CodeBrowserScreen.kt`, `tasks/027-image-and-csv-viewer.md` (evidence updated in place, matching the pattern used for TASK-025's own follow-ups). +- Checks and results: `./gradlew compileDebugKotlin` BUILD SUCCESSFUL; `./gradlew assembleDebug testDebugUnitTest lintDebug` BUILD SUCCESSFUL, all unit tests passing, 0 lint errors. Reinstalled on the human product owner's device over wireless `adb`. +- Review findings and resolutions: None from a separate review pass — the single-gesture-loop design was chosen proactively, reasoning directly from the two prior tab-swipe/content-scroll gesture conflicts this session had already found and fixed, rather than discovered as a defect afterward. +- Known limitations: The pinch-zoom/pan/double-tap gestures themselves haven't been live-confirmed yet (only that the image itself opens, from the prior install) — no dedicated unit test exists for this either, since it's pure Compose gesture-handling with no non-UI logic to isolate. The single-gesture-loop design structurally prevents the earlier bug class by construction, but that's a design argument pending an actual live touch test, not a substitute for one. +- Next recommended task: Get live confirmation of the zoom/pan/double-tap gestures specifically, plus the still-outstanding CSV table and Issues/PR numbered pagination confirmations from earlier today, otherwise continue per the standing direction. +- Next owner: Claude, continuing per the standing direction. diff --git a/tasks/027-image-and-csv-viewer.md b/tasks/027-image-and-csv-viewer.md index 974d001..936809d 100644 --- a/tasks/027-image-and-csv-viewer.md +++ b/tasks/027-image-and-csv-viewer.md @@ -1,17 +1,19 @@ -# TASK-027: Image viewer (with swipe gallery navigation) and CSV table viewer in the code browser +# TASK-027: Image viewer (swipe gallery navigation, pinch-to-zoom) and CSV table viewer -Status: implemented; all required checks pass; live device verification pending (no image/CSV -files were found in the repositories browsed live this session — see Known limitations) +Status: implemented; all required checks pass; the human product owner confirmed the image opens +live, pinch-zoom is unconfirmed live — see Known limitations Owner: Claude Reviewer: Codex ## Outcome -Two live requests, delivered together: opening an image file in the code browser now shows the -image (not a generic "binary file" fallback), with left/right swipe to move to the previous/next -image in the same folder; opening a `.csv` file shows a scrollable table (rows/columns, first row -styled as a header) instead of raw comma-separated text. Both reuse the tab-swipe/content-scroll -gesture-conflict fix from TASK-025's last follow-up: swiping either an image or a CSV table never +Three live requests, delivered as one connected feature: opening an image file in the code +browser now shows the image (not a generic "binary file" fallback), with left/right swipe to move +to the previous/next image in the same folder, and pinch-to-zoom with pan (plus double-tap to +toggle zoom) once the human product owner confirmed the image itself opens correctly and asked for +zoom next. Opening a `.csv` file shows a scrollable table (rows/columns, first row styled as a +header) instead of raw comma-separated text. All three reuse the tab-swipe/content-scroll +gesture-conflict fix from TASK-025's last follow-up: interacting with an image or a CSV table never triggers the Files/History tab to change underneath it. ## Scope @@ -26,9 +28,10 @@ triggers the Files/History tab to change underneath it. `MainActivity.kt` (`onNavigateImage` wiring), `domain/model/RepositoryModels.kt` is untouched by this task (see TASK-028 for its `Page.totalCount` addition, done in the same session but a separate concern). -- Excluded: No new image/CSV editing, no zoom/pan on images (only swipe-to-navigate was asked - for), no header-row auto-detection for CSV (the first row is always styled as a header — see - Design notes), no thumbnail/preview generation, no change to how non-image/non-CSV files render. +- Excluded: No new image/CSV editing, no zoom/pan on the CSV table or any other content type (only + the image viewer got zoom, per what was actually asked), no header-row auto-detection for CSV + (the first row is always styled as a header — see Design notes), no thumbnail/preview generation, + no change to how non-image/non-CSV files render. ## Design notes @@ -47,10 +50,29 @@ triggers the Files/History tab to change underneath it. Opening a file *outside* a directory listing (currently only possible via `retry()` after an error) leaves `siblingImagePaths` empty — swipe navigation is simply unavailable there, a known, accepted edge case rather than extra plumbing through `Loading`/`Error` states for it. -- **The image swipe gesture is a discrete drag-distance threshold** (`detectHorizontalDragGestures` - + a 120px total-drag check on release), not a continuous scroll position — unlike the diff - view's horizontal scroll, there's no partial-scroll state to render mid-drag, only "did the user - swipe far enough to go to the next/previous image." +- **The image swipe gesture is a discrete drag-distance threshold** (a 120px total-drag check on + release), not a continuous scroll position — unlike the diff view's horizontal scroll, there's + no partial-scroll state to render mid-drag, only "did the user swipe far enough to go to the + next/previous image." +- **Pinch-zoom, pan, and swipe-navigate share one hand-rolled gesture loop** (`awaitEachGesture`, + reading live pointer count each event), not three separate detector modifiers stacked on the + same node — Compose broadcasts pointer events to every `pointerInput` block on a node + cooperatively rather than picking one exclusive winner, so two independent detectors racing for + the same single-finger drag (one wanting to pan/zoom, one wanting to swipe-navigate) would have + fought each other exactly like the tab-swipe-vs-content-scroll bug TASK-025 already hit twice. + Deciding inline, per pointer event, which behavior applies avoids that class of bug outright: 2+ + pointers is always pinch-zoom/pan; 1 pointer while already zoomed in (`scale > 1f`) pans; 1 + pointer at 1x is reserved for the existing swipe-navigate accumulator. A double-tap + (`detectTapGestures` in a second, non-competing `pointerInput` — taps don't accumulate drag + distance, so it doesn't fight the first block) toggles between 1x and 2x zoom, matching common + photo-viewer convention. Zoom/pan/offset are `remember(content)`-scoped, so switching to a + different image via gallery swipe always starts that image at 1x, not carrying over the previous + image's zoom. +- **Pan is clamped to an approximate bound**, not the image's exact fitted rectangle: it treats the + image as filling its container, which `ContentScale.Fit` only does exactly along one axis for + most non-square aspect ratios. Precise bounds would need the actual fitted rect (image and + container aspect ratio compared), which wasn't worth the extra complexity for keeping a zoomed + image roughly on screen. - **The CSV table reuses the diff view's manually-driven shared-offset pattern** for its horizontal scroll (`rememberScrollableState` + `Modifier.offset`, one offset shared across all rows via `Modifier.scrollable` per row) rather than a plain shared `Modifier.horizontalScroll`. Every CSV @@ -78,9 +100,17 @@ triggers the Files/History tab to change underneath it. - [x] Opening a `.csv` file shows a scrollable table with the first row styled as a header; a non-`.csv` file with comma-separated content is unaffected (still plain text). - [x] Quoted CSV fields containing commas and escaped (`""`) quotes parse correctly. +- [x] Pinching zooms an image in/out (clamped 1x–5x); a single finger pans while zoomed in; + double-tap toggles between 1x and 2x. +- [x] Pinch/pan/double-tap never trigger gallery swipe-navigation, and swipe-navigation never + triggers pinch/pan/zoom — the three are mutually exclusive per gesture, decided by live + pointer count. +- [x] Zoom/pan resets to 1x/centered when navigating to a different image in the gallery. - [x] Build, unit tests, and lint pass. -- [ ] Live device confirmation with a real image file and a real `.csv` file — not yet possible - this session; see Known limitations. +- [x] Live confirmation that the image itself opens correctly — the human product owner confirmed + this directly before asking for zoom. +- [ ] Live device confirmation of pinch-zoom/pan/double-tap specifically, and of the `.csv` table — + not yet possible this session; see Known limitations. ## Required checks @@ -104,8 +134,11 @@ triggers the Files/History tab to change underneath it. - `CodeBrowserViewModel.kt`: `CodeBrowserUiState.File` gained `siblingImagePaths: List = emptyList()`; `openEntry` captures it from the current `Directory` state's file entries (via new private `RepositoryPath.hasImageExtension()`); new `navigateImage(delta: Int)`. -- `CodeBrowserScreen.kt`: new `ImageContent` (BitmapFactory decode + Compose `Image` + - drag-threshold swipe), `CsvTable`/`CsvRow`/`CsvCell` (shared-offset horizontal scroll, matching +- `CodeBrowserScreen.kt`: new `ImageContent` (BitmapFactory decode + Compose `Image` rendered via + `Modifier.graphicsLayer { scaleX/scaleY/translationX/translationY }`, a single `awaitEachGesture` + loop combining pinch-zoom, pan-while-zoomed, and drag-threshold swipe-navigate by live pointer + count, plus a separate `detectTapGestures` for double-tap-to-toggle-zoom), `Offset.clampedToBox` + (approximate pan bounds), `CsvTable`/`CsvRow`/`CsvCell` (shared-offset horizontal scroll, matching the diff view's pattern); `FileContent`'s signature grew `path`/`siblingImagePaths`/ `onNavigateImage` to support both. - `BranchDetailScreen.kt`: `currentPageShowsScrollableCode`'s page-0 branch now also checks @@ -115,26 +148,40 @@ triggers the Files/History tab to change underneath it. `.csv` vs non-`.csv` path handling, no-path-given fallback to Text, quoted-field/escaped-quote parsing, row-count truncation); `CodeBrowserViewModelTest.kt` (gallery capture in listing order excluding a non-image sibling, `navigateImage` moving and stopping at both ends, `navigateImage` - no-op when opened outside a directory listing). -- `./gradlew compileDebugKotlin compileDebugUnitTestKotlin`: BUILD SUCCESSFUL. + no-op when opened outside a directory listing). The pinch-zoom/pan/double-tap gesture code itself + has no dedicated unit test — it's pure Compose gesture-handling with no non-UI logic to isolate + (see Known limitations for how it was verified instead). +- `./gradlew compileDebugKotlin compileDebugUnitTestKotlin`: BUILD SUCCESSFUL, both after the + initial image/CSV viewer and again after adding pinch-zoom. - `./gradlew testDebugUnitTest`: BUILD SUCCESSFUL, all unit tests passing (including the new ones - above). -- `./gradlew assembleDebug lintDebug`: BUILD SUCCESSFUL, 0 lint errors. -- Live device: build installed over wireless `adb`. Navigated several of the human product owner's - actual repositories looking for an existing image or `.csv` file to confirm the rendering - end-to-end; none were found among the repositories browsed this session (see Known limitations). + above), both times. +- `./gradlew assembleDebug lintDebug`: BUILD SUCCESSFUL, 0 lint errors, both times. +- Live device: build installed over wireless `adb` twice — once for the initial image/CSV viewer, + once after adding pinch-zoom. The human product owner confirmed the image itself opens correctly + on the first install, then asked for zoom directly from that same live session rather than a + separate report. Navigating repositories looking for an existing `.csv` file to confirm the table + rendering end-to-end found none among those browsed this session (see Known limitations). ## Known limitations -- **Not yet confirmed against a real image or CSV file on the live device.** Every repository - browsed this session for a suitable test file didn't have one readily reachable. The - classification and rendering logic is covered thoroughly by unit tests instead (magic-byte - detection, gallery navigation, CSV parsing), but a live human confirmation of the actual pixels - on screen — decoded image quality, table cell alignment/scroll feel — is still outstanding. +- **Image opening itself is live-confirmed; pinch-zoom/pan/double-tap is not yet.** The human + product owner confirmed the decoded image renders correctly, then asked for zoom in the same + session — there hasn't yet been a follow-up live confirmation specifically of the zoom/pan/ + double-tap gestures themselves. Unlike the earlier gesture-conflict bugs in this session (found + only by live testing, not reasoning), there was no repo/file readily available to hand-test + against interactively beyond a single already-open image; the gesture-arbitration design (live + pointer-count branching in one gesture loop, see Design notes) was chosen specifically to + structurally prevent the same class of bug TASK-025 hit twice, but that's a design argument, not + a substitute for actually pinching/panning it on the device. +- **CSV table still not confirmed against a real `.csv` file.** Every repository browsed this + session for one didn't have a readily reachable example. - The CSV table's column width is a fixed 120dp per cell, not sized to content — a column with very short values wastes horizontal space, and a column with very long values still clips at 120dp (readable via horizontal scroll, but not auto-fit). Not raised as a concern by the human product owner; flagged here in case it's worth revisiting once seen live. +- Pan-while-zoomed is clamped to an approximate bound (see Design notes), not the image's exact + fitted rectangle — for most non-square images this means the clamp is slightly looser or + tighter than the image's true edge along one axis, not pixel-exact. - `navigateImage` after a `retry()`-recovered error loses gallery position (see Design notes) — accepted, not treated as a defect.