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 e50c211..31d4f74 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 @@ -67,7 +67,8 @@ fun BranchDetailScreen( 0 -> codeBrowserState is CodeBrowserUiState.File && (codeBrowserState.content is ContentClassification.Text || codeBrowserState.content is ContentClassification.Image || - codeBrowserState.content is ContentClassification.Csv) + codeBrowserState.content is ContentClassification.Csv || + codeBrowserState.content is ContentClassification.Pdf) else -> historyState is HistoryUiState.CommitDetailContent && historyState.selectedFile != null } Column(modifier.fillMaxSize().statusBarsPadding()) { 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 11e1777..bbe186a 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 @@ -1,6 +1,11 @@ package com.ubisam.ubgitea.feature.codebrowser +import android.content.Context +import android.graphics.Bitmap import android.graphics.BitmapFactory +import android.graphics.Color +import android.graphics.pdf.PdfRenderer +import android.os.ParcelFileDescriptor import androidx.activity.compose.BackHandler import androidx.compose.foundation.Image import androidx.compose.foundation.background @@ -17,6 +22,7 @@ import androidx.compose.foundation.gestures.scrollable import androidx.compose.foundation.horizontalScroll import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.BoxScope import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.ColumnScope import androidx.compose.foundation.layout.Row @@ -39,9 +45,11 @@ import androidx.compose.material3.OutlinedButton import androidx.compose.material3.Scaffold import androidx.compose.material3.Text import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.State import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableFloatStateOf +import androidx.compose.runtime.mutableIntStateOf import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.setValue @@ -49,6 +57,7 @@ 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.ImageBitmap import androidx.compose.ui.graphics.asImageBitmap import androidx.compose.ui.graphics.graphicsLayer import androidx.compose.ui.input.pointer.pointerInput @@ -57,6 +66,7 @@ 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.LocalContext import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.res.stringResource import androidx.compose.ui.semantics.Role @@ -68,10 +78,15 @@ import androidx.compose.ui.unit.IntSize import androidx.compose.ui.unit.dp import com.ubisam.ubgitea.R import com.ubisam.ubgitea.domain.model.RepositoryPath +import com.ubisam.ubgitea.domain.model.SourceContent import com.ubisam.ubgitea.domain.model.TreeEntry import com.ubisam.ubgitea.domain.model.TreeEntryType import com.ubisam.ubgitea.feature.codebrowser.content.ContentClassification +import java.io.File +import java.io.IOException import kotlin.math.roundToInt +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext @Composable fun CodeBrowserScreen( @@ -231,6 +246,7 @@ private fun FileContent( ImageContent(content, index, siblingImagePaths.size, onNavigateImage) } is ContentClassification.Csv -> CsvTable(content) + is ContentClassification.Pdf -> PdfContent(content) is ContentClassification.Binary -> Fallback(R.string.codebrowser_binary, content.sizeBytes) is ContentClassification.UnsupportedEncoding -> Fallback(R.string.codebrowser_unsupported_encoding, content.sizeBytes) is ContentClassification.Oversized -> Fallback(R.string.codebrowser_oversized, content.sizeBytes) @@ -251,11 +267,6 @@ 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), @@ -266,60 +277,7 @@ private fun ImageContent(content: ContentClassification.Image, currentIndex: Int Text(stringResource(R.string.codebrowser_image_index, currentIndex + 1, totalCount), style = MaterialTheme.typography.bodySmall) } } - Box( - 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) - } - } - } - .pointerInput(content) { - detectTapGestures( - onDoubleTap = { - scale = if (scale > 1f) 1f else DoubleTapZoomScale - panOffset = Offset.Zero - }, - ) - }, - contentAlignment = Alignment.Center, - ) { + ZoomableSwipeableBox(resetKey = content, canNavigate = canNavigate, onNavigate = onNavigateImage, modifier = Modifier.fillMaxSize()) { scale, panOffset -> val decoded = bitmap if (decoded != null) { Image( @@ -340,16 +298,172 @@ 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. +@Composable +private fun PdfContent(content: ContentClassification.Pdf) { + val context = LocalContext.current + var pageIndex by remember(content) { mutableIntStateOf(0) } + var page by remember(content) { mutableStateOf(null) } + var loadFailed by remember(content) { mutableStateOf(false) } + LaunchedEffect(content, pageIndex) { + val rendered = withContext(Dispatchers.IO) { renderPdfPage(context, content.content, pageIndex) } + page = rendered + loadFailed = rendered == null + } + val current = page + val canNavigate = current != null && current.pageCount > 1 + Column(Modifier.fillMaxSize()) { + Row( + Modifier.fillMaxWidth().padding(horizontal = 16.dp, vertical = 8.dp), + horizontalArrangement = Arrangement.End, + ) { + if (current != null) { + Text(stringResource(R.string.codebrowser_image_index, pageIndex + 1, current.pageCount), style = MaterialTheme.typography.bodySmall) + } + } + ZoomableSwipeableBox( + resetKey = content, + canNavigate = canNavigate, + onNavigate = { delta -> + val count = current?.pageCount ?: return@ZoomableSwipeableBox + val next = pageIndex + delta + if (next in 0 until count) pageIndex = next + }, + modifier = Modifier.fillMaxSize(), + ) { scale, panOffset -> + when { + current != null -> Image( + current.bitmap, + contentDescription = null, + contentScale = ContentScale.Fit, + modifier = Modifier.fillMaxSize().graphicsLayer { + scaleX = scale + scaleY = scale + translationX = panOffset.x + translationY = panOffset.y + }, + ) + loadFailed -> Text(stringResource(R.string.codebrowser_pdf_decode_failed)) + else -> CircularProgressIndicator() + } + } + } +} + +// Shared by the image viewer and the PDF page viewer: one hand-rolled gesture loop combining +// pinch-zoom, pan-while-zoomed, and a discrete swipe-to-navigate (to the next/previous image, or +// PDF page), decided per pointer event by live pointer count rather than by stacking separate +// competing detectors — see the History diff view / tab-swipe gesture-conflict fixes this same +// session for why two independent detectors racing for one single-finger drag is the bug class +// this avoids by construction. `resetKey` scopes zoom/pan state so switching to different content +// (a different image, a different PDF page) always starts unzoomed rather than carrying over the +// previous content's zoom. +@Composable +private fun ZoomableSwipeableBox( + resetKey: Any?, + canNavigate: Boolean, + onNavigate: (Int) -> Unit, + modifier: Modifier = Modifier, + content: @Composable BoxScope.(scale: Float, panOffset: Offset) -> Unit, +) { + var scale by remember(resetKey) { mutableFloatStateOf(1f) } + var panOffset by remember(resetKey) { mutableStateOf(Offset.Zero) } + var boxSizePx by remember(resetKey) { mutableStateOf(IntSize.Zero) } + Box( + modifier + .onSizeChanged { boxSizePx = it } + .pointerInput(resetKey, 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, MaxZoomScale) + 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 <= -SwipeNavigateThresholdPx) onNavigate(1) + else if (horizontalDragPx >= SwipeNavigateThresholdPx) onNavigate(-1) + } + } + } + .pointerInput(resetKey) { + detectTapGestures( + onDoubleTap = { + scale = if (scale > 1f) 1f else DoubleTapZoomScale + panOffset = Offset.Zero + }, + ) + }, + contentAlignment = Alignment.Center, + ) { + content(scale, panOffset) + } +} + +// Keeps zoomed-in content from being panned so far that it leaves the viewport entirely — an +// approximation (treats the content 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 content 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)) } +private class PdfPage(val bitmap: ImageBitmap, val pageCount: Int) + +// android.graphics.pdf.PdfRenderer only opens a ParcelFileDescriptor backed by a real file, not +// an in-memory byte array — the bytes already fetched via the Gitea contents API are written to a +// throwaway temp file (deleted in `finally`, regardless of outcome) purely to satisfy that. Reopens +// the file and re-renders on every page turn rather than keeping a PdfRenderer instance alive +// across recompositions/navigations — simpler and safer against lifecycle bugs (a held-open +// renderer surviving past its Composable, a leaked file descriptor) than the modest redundant I/O +// costs, for how infrequently a user flips PDF pages compared to, say, scrolling a list. +private fun renderPdfPage(context: Context, source: SourceContent, pageIndex: Int): PdfPage? { + val tempFile = File.createTempFile("codebrowser_pdf_", ".pdf", context.cacheDir) + return try { + tempFile.writeBytes(source.copyBytes()) + ParcelFileDescriptor.open(tempFile, ParcelFileDescriptor.MODE_READ_ONLY).use { descriptor -> + PdfRenderer(descriptor).use { renderer -> + val pageCount = renderer.pageCount + if (pageIndex !in 0 until pageCount) return null + renderer.openPage(pageIndex).use { page -> + val bitmap = Bitmap.createBitmap(page.width * PdfRenderScale, page.height * PdfRenderScale, Bitmap.Config.ARGB_8888) + bitmap.eraseColor(Color.WHITE) + page.render(bitmap, null, null, PdfRenderer.Page.RENDER_MODE_FOR_DISPLAY) + PdfPage(bitmap.asImageBitmap(), pageCount) + } + } + } + } catch (_: IOException) { + null + } catch (_: SecurityException) { + null + } finally { + tempFile.delete() + } +} + @Composable private fun CsvTable(content: ContentClassification.Csv) { if (content.rows.isEmpty()) { @@ -440,9 +554,10 @@ private fun CenterMessage(content: @Composable ColumnScope.() -> Unit) { ) } -private const val ImageSwipeThresholdPx = 120f -private const val MaxImageScale = 5f +private const val SwipeNavigateThresholdPx = 120f +private const val MaxZoomScale = 5f private const val DoubleTapZoomScale = 2f +private const val PdfRenderScale = 2 private val CsvCellWidth = 120.dp private val CodeBrowserError.messageResource: Int diff --git a/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/CodeBrowserViewModel.kt b/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/CodeBrowserViewModel.kt index 5a96c03..b567c9b 100644 --- a/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/CodeBrowserViewModel.kt +++ b/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/CodeBrowserViewModel.kt @@ -79,7 +79,12 @@ sealed interface CodeBrowserUiState { class CodeBrowserViewModel( private val browser: GiteaRepositoryBrowser, private val classifier: SourceContentClassifier = SourceContentClassifier( - maxInputBytes = 1_000_000, + // 1MB was fine while only text needed this budget (text rendering is separately bounded + // by maxRenderedCharacters/maxRenderedLines below regardless of how many raw bytes were + // fetched). Now that images and PDFs share the same cutoff before ever being classified, + // 1MB would reject most real-world photos and PDFs as Oversized before they're even + // looked at — raised to a still-mobile-safe 10MB instead. + maxInputBytes = 10_000_000, maxRenderedCharacters = 200_000, maxRenderedLines = 10_000, ), diff --git a/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/content/ContentClassification.kt b/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/content/ContentClassification.kt index b600f26..20c48c2 100644 --- a/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/content/ContentClassification.kt +++ b/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/content/ContentClassification.kt @@ -30,6 +30,14 @@ sealed class ContentClassification { val isTruncated: Boolean, ) : ContentClassification() + // Page rendering (android.graphics.pdf.PdfRenderer) is an Android-framework concern, not a + // pure-Kotlin one — this case, like Image, only carries the raw bytes; the UI layer decides + // page count and renders pages on demand. + data class Pdf( + override val sizeBytes: Int, + val content: SourceContent, + ) : ContentClassification() + data class Binary(override val sizeBytes: Int) : ContentClassification() data class LfsPointer( diff --git a/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/content/SourceContentClassifier.kt b/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/content/SourceContentClassifier.kt index d16ad31..8fbf6aa 100644 --- a/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/content/SourceContentClassifier.kt +++ b/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/content/SourceContentClassifier.kt @@ -37,6 +37,10 @@ class SourceContentClassifier( return ContentClassification.Image(sizeBytes, SourceContent.from(bytes), format) } + if (bytes.startsWith(PDF_SIGNATURE)) { + return ContentClassification.Pdf(sizeBytes, SourceContent.from(bytes)) + } + if (bytes.any { it == NUL_BYTE }) { return ContentClassification.Binary(sizeBytes) } @@ -202,5 +206,6 @@ class SourceContentClassifier( val GIF89A_SIGNATURE = "GIF89a".toByteArray(Charsets.US_ASCII) val RIFF_SIGNATURE = "RIFF".toByteArray(Charsets.US_ASCII) val WEBP_SIGNATURE = "WEBP".toByteArray(Charsets.US_ASCII) + val PDF_SIGNATURE = "%PDF-".toByteArray(Charsets.US_ASCII) } } diff --git a/app/src/main/res/values/strings_codebrowser.xml b/app/src/main/res/values/strings_codebrowser.xml index ba718e8..db58190 100644 --- a/app/src/main/res/values/strings_codebrowser.xml +++ b/app/src/main/res/values/strings_codebrowser.xml @@ -29,4 +29,5 @@ %1$d / %2$d 이미지를 표시할 수 없습니다. 이 CSV 파일은 비어 있습니다. + PDF를 표시할 수 없습니다. diff --git a/app/src/test/java/com/ubisam/ubgitea/feature/codebrowser/content/SourceContentClassifierTest.kt b/app/src/test/java/com/ubisam/ubgitea/feature/codebrowser/content/SourceContentClassifierTest.kt index 18b61ab..a1cfe91 100644 --- a/app/src/test/java/com/ubisam/ubgitea/feature/codebrowser/content/SourceContentClassifierTest.kt +++ b/app/src/test/java/com/ubisam/ubgitea/feature/codebrowser/content/SourceContentClassifierTest.kt @@ -141,6 +141,36 @@ class SourceContentClassifierTest { assertEquals(ContentClassification.Oversized(sizeBytes = 8), result) } + // --- PDF --- + + @Test + fun classifiesPdfSignatureAsPdf() { + // A PDF's bytes contain NULs too (in cross-reference/stream data), so — like the image + // tests above — this also proves PDF detection runs before the NUL-byte Binary check. + val bytes = "%PDF-1.7\n".toByteArray(Charsets.US_ASCII) + byteArrayOf(0, 0, 0, 0) + val result = classifier.classify(SourceContent.from(bytes)) + + assertTrue(result is ContentClassification.Pdf) + assertEquals(13, (result as ContentClassification.Pdf).sizeBytes) + } + + @Test + fun oversizedPdfContentIsRejectedBeforePdfDetection() { + val exact = SourceContentClassifier(maxInputBytes = 2, maxRenderedCharacters = 40, maxRenderedLines = 5) + val bytes = "%PDF-1.7".toByteArray(Charsets.US_ASCII) + + val result = exact.classify(SourceContent.from(bytes)) + + assertEquals(ContentClassification.Oversized(sizeBytes = 8), result) + } + + @Test + fun doesNotMisclassifyPlainTextStartingWithPercentAsPdf() { + val result = classifier.classify(content("%not a pdf")) + + assertTrue(result is ContentClassification.Text) + } + // --- CSV --- @Test diff --git a/docs/development-log.md b/docs/development-log.md index 13aa4ef..b474307 100644 --- a/docs/development-log.md +++ b/docs/development-log.md @@ -465,3 +465,16 @@ This is the append-only implementation and handoff record shared by Codex and Cl - 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. + +## 2026-08-27 — TASK-027 follow-up: PDF viewer + +- Agent and role: Claude, direct continuation of the same live session — the human product owner tried opening a PDF file next and asked "pdf 파일은 못읽나?" (can't PDF files be read?). +- Commit/diff: continued on `claude/task-015-notifications`; `feature/codebrowser/content/ContentClassification.kt`, `feature/codebrowser/content/SourceContentClassifier.kt`, `feature/codebrowser/CodeBrowserViewModel.kt`, `feature/codebrowser/CodeBrowserScreen.kt`, `feature/branchdetail/BranchDetailScreen.kt`, plus a matching test and `tasks/027-image-and-csv-viewer.md`. +- Feature implemented: opening a `.pdf` file (detected by `%PDF-` magic bytes) now renders its pages, with the same left/right swipe-between-pages, pinch-zoom, pan, and double-tap-to-toggle-zoom interaction already built for the image viewer. +- How it was implemented: rendering uses Android's built-in `android.graphics.pdf.PdfRenderer` (API 21+, no compatibility concern against this app's `minSdk = 28`) — no new dependency, matching how the image viewer already used the platform's own `BitmapFactory` instead of a third-party decoding library. `PdfRenderer` only accepts a `ParcelFileDescriptor` backed by a real file, so the already-fetched bytes get written to a throwaway temp file purely to satisfy that, deleted in a `finally` block regardless of outcome; the file is fully re-opened and the requested page re-rendered on every page turn rather than keeping a `PdfRenderer` held open across recompositions, trading some redundant I/O for avoiding a class of lifecycle bug that's easy to introduce holding a stateful Android resource across Compose's recomposition lifecycle. The pinch-zoom/pan/swipe-navigate/double-tap gesture code from the prior pinch-zoom follow-up was extracted into a shared `ZoomableSwipeableBox` composable specifically so this viewer could reuse it verbatim rather than duplicating that fairly intricate gesture-arbitration logic a second time. Classification itself only detects "is this a PDF" by magic bytes — page count and rendering stay entirely in the UI layer (`LaunchedEffect`), the same split already used for images, keeping the classifier framework-agnostic and unit-testable without Robolectric. Separately, `maxInputBytes` was raised from 1MB to 10MB in `CodeBrowserViewModel`'s classifier config — at 1MB, most real-world photos and PDFs would have been rejected as Oversized before ever being classified, which would have made both this feature and the image viewer close to useless in practice; 10MB is still a deliberately conservative mobile-safe ceiling, not "however large." +- Files changed: see `tasks/027-image-and-csv-viewer.md`'s Scope section for the full list. +- Checks and results: `./gradlew compileDebugKotlin` BUILD SUCCESSFUL; `./gradlew compileDebugUnitTestKotlin testDebugUnitTest` BUILD SUCCESSFUL, all unit tests passing (new coverage: `%PDF-` signature detection, its embedded NUL bytes not short-circuiting to Binary, oversized-before-PDF-detection ordering, plain text starting with `%` not misclassified as PDF); `./gradlew assembleDebug lintDebug` BUILD SUCCESSFUL, 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: Neither the PDF rendering path nor the shared gesture code has a dedicated unit test — both are Android-framework/pure-UI concerns with no non-UI logic to isolate (`PdfRenderer` specifically isn't available in a plain JVM unit test without Robolectric). Not yet confirmed against a real `.pdf` file on the live device — no repository browsed this session had one readily reachable, same as the still-outstanding CSV table confirmation. Re-opening and fully re-rendering the file on every page turn hasn't been performance-checked against a real large, many-page PDF. +- Next recommended task: Get live confirmation of the PDF viewer, the CSV table, pinch-zoom/pan/double-tap, and the Issues/PR numbered pagination — everything built this session that's still pending a live look — otherwise continue per the standing direction. No other items remain queued. +- 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 936809d..7810e24 100644 --- a/tasks/027-image-and-csv-viewer.md +++ b/tasks/027-image-and-csv-viewer.md @@ -1,37 +1,42 @@ -# TASK-027: Image viewer (swipe gallery navigation, pinch-to-zoom) and CSV table viewer +# TASK-027: Image viewer (swipe gallery, pinch-to-zoom), CSV table viewer, PDF viewer Status: implemented; all required checks pass; the human product owner confirmed the image opens -live, pinch-zoom is unconfirmed live — see Known limitations +live; pinch-zoom and the PDF viewer are unconfirmed live — see Known limitations Owner: Claude Reviewer: Codex ## Outcome -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. +Four live requests, delivered as one connected feature, each building on live feedback from the +previous: 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; +pinch-to-zoom with pan (plus double-tap to toggle zoom), added 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; +and, asked as a follow-up ("pdf 파일은 못읽나?"), opening a `.pdf` file now renders its pages +(via Android's built-in `PdfRenderer` — no new dependency) with the same swipe-between-pages and +pinch-zoom/pan interaction as the image viewer, reusing its gesture-handling rather than +duplicating it. All of these reuse the tab-swipe/content-scroll gesture-conflict fix from +TASK-025's last follow-up: interacting with an image, a CSV table, or a PDF page never triggers +the Files/History tab to change underneath it. ## Scope -- Included: `feature/codebrowser/content/ContentClassification.kt` (new `Image`/`Csv` cases, new - `ImageFormat` enum), `feature/codebrowser/content/SourceContentClassifier.kt` (magic-byte image - detection, `.csv`-extension table parsing), `feature/codebrowser/CodeBrowserViewModel.kt` - (`siblingImagePaths` on `File` state, `navigateImage`, `type=issues`-adjacent-but-unrelated — - see TASK-028 for that; this task only touches the image gallery/path plumbing), - `feature/codebrowser/CodeBrowserScreen.kt` (`ImageContent`, `CsvTable`/`CsvRow`/`CsvCell`), - `feature/branchdetail/BranchDetailScreen.kt` (pager-gating condition widened to Image/Csv), +- Included: `feature/codebrowser/content/ContentClassification.kt` (new `Image`/`Csv`/`Pdf` cases, + new `ImageFormat` enum), `feature/codebrowser/content/SourceContentClassifier.kt` (magic-byte + image and PDF detection, `.csv`-extension table parsing), `feature/codebrowser/CodeBrowserViewModel.kt` + (`siblingImagePaths` on `File` state, `navigateImage`, `maxInputBytes` raised from 1MB to 10MB — + see Design notes; `type=issues` is TASK-028's, unrelated to this task), `feature/codebrowser/CodeBrowserScreen.kt` + (`ImageContent`, `CsvTable`/`CsvRow`/`CsvCell`, `PdfContent`, shared `ZoomableSwipeableBox`), + `feature/branchdetail/BranchDetailScreen.kt` (pager-gating condition widened to Image/Csv/Pdf), `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 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. +- Excluded: No new image/CSV/PDF editing, no zoom/pan on the CSV table (only the image and PDF + viewers 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 PDF + text extraction/search/selection (pages render as images only), no change to how other file + types render. ## Design notes @@ -67,7 +72,35 @@ triggers the Files/History tab to change underneath it. 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. + image's zoom. This whole gesture loop, along with the double-tap and pan-clamping, was extracted + into a shared `ZoomableSwipeableBox` composable specifically so the PDF page viewer (added right + after, see below) could reuse it verbatim instead of duplicating fairly intricate gesture code a + second time. +- **PDF pages render via Android's built-in `android.graphics.pdf.PdfRenderer`** (API 21+, so no + compatibility concern against this app's `minSdk = 28`) — no new dependency, matching how the + image viewer uses the platform's own `BitmapFactory` rather than a third-party decoding library. + `PdfRenderer` only opens a `ParcelFileDescriptor` backed by a real file, not an in-memory byte + array, so the already-fetched bytes are written to a throwaway temp file (`File.createTempFile` + in the app's cache dir, deleted in a `finally` block regardless of outcome) purely to satisfy + that API shape. Classification itself only detects "is this a PDF" by magic bytes (`%PDF-`) — + page count and page rendering are Android-framework concerns handled entirely in the UI layer via + `LaunchedEffect`, the same split already used for images (`SourceContentClassifier` decides + format from bytes; `CodeBrowserScreen` does the actual `BitmapFactory`/`PdfRenderer` decode) — + keeping the classifier framework-agnostic and unit-testable without Robolectric. +- **The PDF viewer re-opens and re-renders the file on every page turn** rather than keeping one + `PdfRenderer` instance alive across recompositions and page navigations. This trades some + redundant file I/O per page flip for avoiding a class of lifecycle bug (a renderer or file + descriptor outliving its Composable, e.g. across process death or rapid navigation) that would + be easy to introduce holding a stateful Android resource open across Compose recompositions — + judged a reasonable trade given how infrequently a user flips PDF pages compared to, say, + scrolling a list. +- **`maxInputBytes` was raised from 1MB to 10MB** in `CodeBrowserViewModel`'s classifier config. + It was fine at 1MB while only text needed the budget (a large text file is separately bounded for + *display* by `maxRenderedCharacters`/`maxRenderedLines`, regardless of how many raw bytes were + fetched) — but now that images and PDFs share the same pre-classification cutoff, 1MB would have + rejected most real-world photos and PDFs as Oversized before they were ever even looked at, + making both features close to useless in practice. 10MB is still a deliberately conservative, + mobile-safe ceiling, not "however large." - **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 @@ -86,9 +119,10 @@ triggers the Files/History tab to change underneath it. file alone. - **Pager-swipe gating (from TASK-025's last follow-up) was widened, not duplicated**: the same `currentPageShowsScrollableCode` check in `BranchDetailScreen.kt` now also covers - `ContentClassification.Image` and `ContentClassification.Csv`, so the human product owner's - explicit requirement — "탭 전환은 발생하면 안돼" (tab switching must not happen) while viewing - either — is met by the existing mechanism rather than a new one. + `ContentClassification.Image`, `ContentClassification.Csv`, and `ContentClassification.Pdf`, so + the human product owner's explicit requirement — "탭 전환은 발생하면 안돼" (tab switching must + not happen) while viewing an image — is met the same way for PDF pages too, by the existing + mechanism rather than a new one. ## Acceptance criteria @@ -106,11 +140,16 @@ triggers the Files/History tab to change underneath it. 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] Opening a `.pdf` file (detected by magic bytes) renders its first page; swiping left/right + moves between pages when there's more than one; pinch-zoom/pan/double-tap work identically + to the image viewer, reusing the same gesture code. +- [x] A file larger than 1MB but no larger than 10MB (an image or PDF that would previously have + been rejected as Oversized) now opens correctly. - [x] Build, unit tests, and lint pass. - [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. +- [ ] Live device confirmation of pinch-zoom/pan/double-tap specifically, the `.csv` table, and the + PDF viewer — not yet possible this session; see Known limitations. ## Required checks @@ -122,45 +161,55 @@ triggers the Files/History tab to change underneath it. ## Evidence -- `ContentClassification.kt`: new `Image(sizeBytes, content: SourceContent, format: ImageFormat)` - and `Csv(sizeBytes, rows: List>, isTruncated: Boolean)` cases; `content` reuses - `SourceContent`'s already-correct content-based `equals()`/`hashCode()` instead of holding a raw - `ByteArray` (which would default to reference equality on a `data class`). +- `ContentClassification.kt`: new `Image(sizeBytes, content: SourceContent, format: ImageFormat)`, + `Csv(sizeBytes, rows: List>, isTruncated: Boolean)`, and + `Pdf(sizeBytes, content: SourceContent)` cases; `content` reuses `SourceContent`'s already-correct + content-based `equals()`/`hashCode()` instead of holding a raw `ByteArray` (which would default to + reference equality on a `data class`). - `SourceContentClassifier.kt`: `classify(content, path: RepositoryPath? = null)` — `path` is optional/defaulted so every pre-existing call site keeps compiling unchanged; `detectImageFormat` - checks PNG/JPEG/GIF/BMP/WebP signatures before the NUL-byte Binary check; CSV parsing - (`renderCsv`/`parseCsv`, a small RFC-4180-ish hand-written parser) only runs when `path` ends in - `.csv`, reusing the same line/character truncation budgets as `renderText`. + checks PNG/JPEG/GIF/BMP/WebP signatures, and a separate `%PDF-` check, both before the NUL-byte + Binary check; CSV parsing (`renderCsv`/`parseCsv`, a small RFC-4180-ish hand-written parser) only + runs when `path` ends in `.csv`, reusing the same line/character truncation budgets as `renderText`. - `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` 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. + `RepositoryPath.hasImageExtension()`); new `navigateImage(delta: Int)`; `maxInputBytes` raised + 1MB → 10MB. +- `CodeBrowserScreen.kt`: new `ImageContent` (BitmapFactory decode), `PdfContent` + (`android.graphics.pdf.PdfRenderer` via a throwaway temp file, `LaunchedEffect`-driven page + render, re-opened per page turn), both rendered through a new shared `ZoomableSwipeableBox` + (Compose `Image` via `Modifier.graphicsLayer { scaleX/scaleY/translationX/translationY }`, one + `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 the image gallery. - `BranchDetailScreen.kt`: `currentPageShowsScrollableCode`'s page-0 branch now also checks - `ContentClassification.Image`/`ContentClassification.Csv`, not just `Text`. + `ContentClassification.Image`/`ContentClassification.Csv`/`ContentClassification.Pdf`, not just + `Text`. - New tests: `SourceContentClassifierTest.kt` (PNG/JPEG/GIF/BMP/WebP signature detection, an image signature's embedded NUL byte not short-circuiting to Binary, oversized-before-image-detection, - `.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). 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. + `%PDF-` signature detection with the same NUL-byte and oversized-ordering checks, a plain-text + file starting with `%` not misclassified as PDF, `.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). Neither the + pinch-zoom/pan/double-tap gesture code nor the PDF-rendering code (an Android-framework + concern — `PdfRenderer` isn't available in a plain JVM unit test without Robolectric) has a + dedicated unit test; both are pure UI/framework logic with no non-UI behavior to isolate (see + Known limitations for how they were verified instead). +- `./gradlew compileDebugKotlin compileDebugUnitTestKotlin`: BUILD SUCCESSFUL, across all three + passes (initial image/CSV viewer, pinch-zoom, PDF viewer). - `./gradlew testDebugUnitTest`: BUILD SUCCESSFUL, all unit tests passing (including the new ones - 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). + above), across all three passes. +- `./gradlew assembleDebug lintDebug`: BUILD SUCCESSFUL, 0 lint errors, across all three passes. +- Live device: build installed over wireless `adb` three times — the initial image/CSV viewer, + after adding pinch-zoom, and after adding the PDF viewer. The human product owner confirmed the + image itself opens correctly on the first install, then asked for zoom directly from that same + live session, then asked about PDF support as a further follow-up — all in one continuous + session rather than separate reports. Navigating repositories looking for an existing `.csv` or + `.pdf` file to confirm either rendering end-to-end found none among those browsed this session + (see Known limitations). ## Known limitations @@ -173,17 +222,25 @@ triggers the Files/History tab to change underneath it. 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. +- **CSV table and PDF viewer still not confirmed against real files.** Every repository browsed + this session for a `.csv` or `.pdf` example didn't have one readily reachable. The PDF viewer in + particular has no unit-test coverage at all (see Evidence) since `PdfRenderer` is an + Android-framework class, so this is entirely pending a live check — decode success, page-turn + correctness, and the shared zoom/pan/swipe gestures working the same way they were confirmed to + (by construction, not yet by touch) work for images. - 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. +- Pan-while-zoomed (image or PDF page) is clamped to an approximate bound (see Design notes), not + the exact fitted rectangle — for most non-square content this means the clamp is slightly looser + or tighter than the 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. +- The PDF viewer re-opens and fully re-renders the file on every page turn (see Design notes) — + chosen deliberately for lifecycle safety over performance; a large, many-page PDF would re-pay + the file-write/renderer-open cost on each swipe, which hasn't been measured against a real + multi-page PDF yet. ## Review findings