TASK-027 follow-up: PDF viewer with swipe/pinch-zoom, reusing image gesture code
Opening a .pdf file (detected by %PDF- magic bytes) now renders its pages via Android's built-in PdfRenderer -- no new dependency -- with the same swipe-between-pages, pinch-zoom, pan, and double-tap-to-toggle-zoom interaction as the image viewer. The shared gesture logic was extracted into a ZoomableSwipeableBox composable so it's genuinely reused, not duplicated. maxInputBytes raised 1MB -> 10MB so real-world photos and PDFs aren't rejected as Oversized before ever being classified. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
8bf3930c3d
commit
bb83c215d3
@ -67,7 +67,8 @@ fun BranchDetailScreen(
|
|||||||
0 -> codeBrowserState is CodeBrowserUiState.File &&
|
0 -> codeBrowserState is CodeBrowserUiState.File &&
|
||||||
(codeBrowserState.content is ContentClassification.Text ||
|
(codeBrowserState.content is ContentClassification.Text ||
|
||||||
codeBrowserState.content is ContentClassification.Image ||
|
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
|
else -> historyState is HistoryUiState.CommitDetailContent && historyState.selectedFile != null
|
||||||
}
|
}
|
||||||
Column(modifier.fillMaxSize().statusBarsPadding()) {
|
Column(modifier.fillMaxSize().statusBarsPadding()) {
|
||||||
|
|||||||
@ -1,6 +1,11 @@
|
|||||||
package com.ubisam.ubgitea.feature.codebrowser
|
package com.ubisam.ubgitea.feature.codebrowser
|
||||||
|
|
||||||
|
import android.content.Context
|
||||||
|
import android.graphics.Bitmap
|
||||||
import android.graphics.BitmapFactory
|
import android.graphics.BitmapFactory
|
||||||
|
import android.graphics.Color
|
||||||
|
import android.graphics.pdf.PdfRenderer
|
||||||
|
import android.os.ParcelFileDescriptor
|
||||||
import androidx.activity.compose.BackHandler
|
import androidx.activity.compose.BackHandler
|
||||||
import androidx.compose.foundation.Image
|
import androidx.compose.foundation.Image
|
||||||
import androidx.compose.foundation.background
|
import androidx.compose.foundation.background
|
||||||
@ -17,6 +22,7 @@ import androidx.compose.foundation.gestures.scrollable
|
|||||||
import androidx.compose.foundation.horizontalScroll
|
import androidx.compose.foundation.horizontalScroll
|
||||||
import androidx.compose.foundation.layout.Arrangement
|
import androidx.compose.foundation.layout.Arrangement
|
||||||
import androidx.compose.foundation.layout.Box
|
import androidx.compose.foundation.layout.Box
|
||||||
|
import androidx.compose.foundation.layout.BoxScope
|
||||||
import androidx.compose.foundation.layout.Column
|
import androidx.compose.foundation.layout.Column
|
||||||
import androidx.compose.foundation.layout.ColumnScope
|
import androidx.compose.foundation.layout.ColumnScope
|
||||||
import androidx.compose.foundation.layout.Row
|
import androidx.compose.foundation.layout.Row
|
||||||
@ -39,9 +45,11 @@ import androidx.compose.material3.OutlinedButton
|
|||||||
import androidx.compose.material3.Scaffold
|
import androidx.compose.material3.Scaffold
|
||||||
import androidx.compose.material3.Text
|
import androidx.compose.material3.Text
|
||||||
import androidx.compose.runtime.Composable
|
import androidx.compose.runtime.Composable
|
||||||
|
import androidx.compose.runtime.LaunchedEffect
|
||||||
import androidx.compose.runtime.State
|
import androidx.compose.runtime.State
|
||||||
import androidx.compose.runtime.getValue
|
import androidx.compose.runtime.getValue
|
||||||
import androidx.compose.runtime.mutableFloatStateOf
|
import androidx.compose.runtime.mutableFloatStateOf
|
||||||
|
import androidx.compose.runtime.mutableIntStateOf
|
||||||
import androidx.compose.runtime.mutableStateOf
|
import androidx.compose.runtime.mutableStateOf
|
||||||
import androidx.compose.runtime.remember
|
import androidx.compose.runtime.remember
|
||||||
import androidx.compose.runtime.setValue
|
import androidx.compose.runtime.setValue
|
||||||
@ -49,6 +57,7 @@ import androidx.compose.ui.Alignment
|
|||||||
import androidx.compose.ui.Modifier
|
import androidx.compose.ui.Modifier
|
||||||
import androidx.compose.ui.draw.clipToBounds
|
import androidx.compose.ui.draw.clipToBounds
|
||||||
import androidx.compose.ui.geometry.Offset
|
import androidx.compose.ui.geometry.Offset
|
||||||
|
import androidx.compose.ui.graphics.ImageBitmap
|
||||||
import androidx.compose.ui.graphics.asImageBitmap
|
import androidx.compose.ui.graphics.asImageBitmap
|
||||||
import androidx.compose.ui.graphics.graphicsLayer
|
import androidx.compose.ui.graphics.graphicsLayer
|
||||||
import androidx.compose.ui.input.pointer.pointerInput
|
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.ContentScale
|
||||||
import androidx.compose.ui.layout.onSizeChanged
|
import androidx.compose.ui.layout.onSizeChanged
|
||||||
import androidx.compose.ui.platform.LocalClipboardManager
|
import androidx.compose.ui.platform.LocalClipboardManager
|
||||||
|
import androidx.compose.ui.platform.LocalContext
|
||||||
import androidx.compose.ui.platform.LocalDensity
|
import androidx.compose.ui.platform.LocalDensity
|
||||||
import androidx.compose.ui.res.stringResource
|
import androidx.compose.ui.res.stringResource
|
||||||
import androidx.compose.ui.semantics.Role
|
import androidx.compose.ui.semantics.Role
|
||||||
@ -68,10 +78,15 @@ import androidx.compose.ui.unit.IntSize
|
|||||||
import androidx.compose.ui.unit.dp
|
import androidx.compose.ui.unit.dp
|
||||||
import com.ubisam.ubgitea.R
|
import com.ubisam.ubgitea.R
|
||||||
import com.ubisam.ubgitea.domain.model.RepositoryPath
|
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.TreeEntry
|
||||||
import com.ubisam.ubgitea.domain.model.TreeEntryType
|
import com.ubisam.ubgitea.domain.model.TreeEntryType
|
||||||
import com.ubisam.ubgitea.feature.codebrowser.content.ContentClassification
|
import com.ubisam.ubgitea.feature.codebrowser.content.ContentClassification
|
||||||
|
import java.io.File
|
||||||
|
import java.io.IOException
|
||||||
import kotlin.math.roundToInt
|
import kotlin.math.roundToInt
|
||||||
|
import kotlinx.coroutines.Dispatchers
|
||||||
|
import kotlinx.coroutines.withContext
|
||||||
|
|
||||||
@Composable
|
@Composable
|
||||||
fun CodeBrowserScreen(
|
fun CodeBrowserScreen(
|
||||||
@ -231,6 +246,7 @@ private fun FileContent(
|
|||||||
ImageContent(content, index, siblingImagePaths.size, onNavigateImage)
|
ImageContent(content, index, siblingImagePaths.size, onNavigateImage)
|
||||||
}
|
}
|
||||||
is ContentClassification.Csv -> CsvTable(content)
|
is ContentClassification.Csv -> CsvTable(content)
|
||||||
|
is ContentClassification.Pdf -> PdfContent(content)
|
||||||
is ContentClassification.Binary -> Fallback(R.string.codebrowser_binary, content.sizeBytes)
|
is ContentClassification.Binary -> Fallback(R.string.codebrowser_binary, content.sizeBytes)
|
||||||
is ContentClassification.UnsupportedEncoding -> Fallback(R.string.codebrowser_unsupported_encoding, content.sizeBytes)
|
is ContentClassification.UnsupportedEncoding -> Fallback(R.string.codebrowser_unsupported_encoding, content.sizeBytes)
|
||||||
is ContentClassification.Oversized -> Fallback(R.string.codebrowser_oversized, 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()
|
BitmapFactory.decodeByteArray(bytes, 0, bytes.size)?.asImageBitmap()
|
||||||
}
|
}
|
||||||
val canNavigate = currentIndex != -1 && totalCount > 1
|
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()) {
|
Column(Modifier.fillMaxSize()) {
|
||||||
Row(
|
Row(
|
||||||
Modifier.fillMaxWidth().padding(horizontal = 16.dp, vertical = 8.dp),
|
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)
|
Text(stringResource(R.string.codebrowser_image_index, currentIndex + 1, totalCount), style = MaterialTheme.typography.bodySmall)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
Box(
|
ZoomableSwipeableBox(resetKey = content, canNavigate = canNavigate, onNavigate = onNavigateImage, modifier = Modifier.fillMaxSize()) { scale, panOffset ->
|
||||||
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,
|
|
||||||
) {
|
|
||||||
val decoded = bitmap
|
val decoded = bitmap
|
||||||
if (decoded != null) {
|
if (decoded != null) {
|
||||||
Image(
|
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
|
@Composable
|
||||||
// approximation (treats the image as filling the box, which ContentScale.Fit only does exactly
|
private fun PdfContent(content: ContentClassification.Pdf) {
|
||||||
// along one axis for most aspect ratios) rather than exact bounds, which is a reasonable trade
|
val context = LocalContext.current
|
||||||
// for how simple it keeps this over computing the fitted image rect precisely.
|
var pageIndex by remember(content) { mutableIntStateOf(0) }
|
||||||
|
var page by remember(content) { mutableStateOf<PdfPage?>(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 {
|
private fun Offset.clampedToBox(scale: Float, boxSizePx: IntSize): Offset {
|
||||||
val maxX = (boxSizePx.width * (scale - 1) / 2).coerceAtLeast(0f)
|
val maxX = (boxSizePx.width * (scale - 1) / 2).coerceAtLeast(0f)
|
||||||
val maxY = (boxSizePx.height * (scale - 1) / 2).coerceAtLeast(0f)
|
val maxY = (boxSizePx.height * (scale - 1) / 2).coerceAtLeast(0f)
|
||||||
return Offset(x.coerceIn(-maxX, maxX), y.coerceIn(-maxY, maxY))
|
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
|
@Composable
|
||||||
private fun CsvTable(content: ContentClassification.Csv) {
|
private fun CsvTable(content: ContentClassification.Csv) {
|
||||||
if (content.rows.isEmpty()) {
|
if (content.rows.isEmpty()) {
|
||||||
@ -440,9 +554,10 @@ private fun CenterMessage(content: @Composable ColumnScope.() -> Unit) {
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
private const val ImageSwipeThresholdPx = 120f
|
private const val SwipeNavigateThresholdPx = 120f
|
||||||
private const val MaxImageScale = 5f
|
private const val MaxZoomScale = 5f
|
||||||
private const val DoubleTapZoomScale = 2f
|
private const val DoubleTapZoomScale = 2f
|
||||||
|
private const val PdfRenderScale = 2
|
||||||
private val CsvCellWidth = 120.dp
|
private val CsvCellWidth = 120.dp
|
||||||
|
|
||||||
private val CodeBrowserError.messageResource: Int
|
private val CodeBrowserError.messageResource: Int
|
||||||
|
|||||||
@ -79,7 +79,12 @@ sealed interface CodeBrowserUiState {
|
|||||||
class CodeBrowserViewModel(
|
class CodeBrowserViewModel(
|
||||||
private val browser: GiteaRepositoryBrowser,
|
private val browser: GiteaRepositoryBrowser,
|
||||||
private val classifier: SourceContentClassifier = SourceContentClassifier(
|
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,
|
maxRenderedCharacters = 200_000,
|
||||||
maxRenderedLines = 10_000,
|
maxRenderedLines = 10_000,
|
||||||
),
|
),
|
||||||
|
|||||||
@ -30,6 +30,14 @@ sealed class ContentClassification {
|
|||||||
val isTruncated: Boolean,
|
val isTruncated: Boolean,
|
||||||
) : ContentClassification()
|
) : 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 Binary(override val sizeBytes: Int) : ContentClassification()
|
||||||
|
|
||||||
data class LfsPointer(
|
data class LfsPointer(
|
||||||
|
|||||||
@ -37,6 +37,10 @@ class SourceContentClassifier(
|
|||||||
return ContentClassification.Image(sizeBytes, SourceContent.from(bytes), format)
|
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 }) {
|
if (bytes.any { it == NUL_BYTE }) {
|
||||||
return ContentClassification.Binary(sizeBytes)
|
return ContentClassification.Binary(sizeBytes)
|
||||||
}
|
}
|
||||||
@ -202,5 +206,6 @@ class SourceContentClassifier(
|
|||||||
val GIF89A_SIGNATURE = "GIF89a".toByteArray(Charsets.US_ASCII)
|
val GIF89A_SIGNATURE = "GIF89a".toByteArray(Charsets.US_ASCII)
|
||||||
val RIFF_SIGNATURE = "RIFF".toByteArray(Charsets.US_ASCII)
|
val RIFF_SIGNATURE = "RIFF".toByteArray(Charsets.US_ASCII)
|
||||||
val WEBP_SIGNATURE = "WEBP".toByteArray(Charsets.US_ASCII)
|
val WEBP_SIGNATURE = "WEBP".toByteArray(Charsets.US_ASCII)
|
||||||
|
val PDF_SIGNATURE = "%PDF-".toByteArray(Charsets.US_ASCII)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@ -29,4 +29,5 @@
|
|||||||
<string name="codebrowser_image_index">%1$d / %2$d</string>
|
<string name="codebrowser_image_index">%1$d / %2$d</string>
|
||||||
<string name="codebrowser_image_decode_failed">이미지를 표시할 수 없습니다.</string>
|
<string name="codebrowser_image_decode_failed">이미지를 표시할 수 없습니다.</string>
|
||||||
<string name="codebrowser_csv_empty">이 CSV 파일은 비어 있습니다.</string>
|
<string name="codebrowser_csv_empty">이 CSV 파일은 비어 있습니다.</string>
|
||||||
|
<string name="codebrowser_pdf_decode_failed">PDF를 표시할 수 없습니다.</string>
|
||||||
</resources>
|
</resources>
|
||||||
|
|||||||
@ -141,6 +141,36 @@ class SourceContentClassifierTest {
|
|||||||
assertEquals(ContentClassification.Oversized(sizeBytes = 8), result)
|
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 ---
|
// --- CSV ---
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
|||||||
@ -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.
|
- 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 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.
|
- 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.
|
||||||
|
|||||||
@ -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
|
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
|
Owner: Claude
|
||||||
Reviewer: Codex
|
Reviewer: Codex
|
||||||
|
|
||||||
## Outcome
|
## Outcome
|
||||||
|
|
||||||
Three live requests, delivered as one connected feature: opening an image file in the code
|
Four live requests, delivered as one connected feature, each building on live feedback from the
|
||||||
browser now shows the image (not a generic "binary file" fallback), with left/right swipe to move
|
previous: opening an image file in the code browser now shows the image (not a generic "binary
|
||||||
to the previous/next image in the same folder, and pinch-to-zoom with pan (plus double-tap to
|
file" fallback), with left/right swipe to move to the previous/next image in the same folder;
|
||||||
toggle zoom) once the human product owner confirmed the image itself opens correctly and asked for
|
pinch-to-zoom with pan (plus double-tap to toggle zoom), added once the human product owner
|
||||||
zoom next. Opening a `.csv` file shows a scrollable table (rows/columns, first row styled as a
|
confirmed the image itself opens correctly and asked for zoom next; opening a `.csv` file shows a
|
||||||
header) instead of raw comma-separated text. All three reuse the tab-swipe/content-scroll
|
scrollable table (rows/columns, first row styled as a header) instead of raw comma-separated text;
|
||||||
gesture-conflict fix from TASK-025's last follow-up: interacting with an image or a CSV table never
|
and, asked as a follow-up ("pdf 파일은 못읽나?"), opening a `.pdf` file now renders its pages
|
||||||
triggers the Files/History tab to change underneath it.
|
(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
|
## Scope
|
||||||
|
|
||||||
- Included: `feature/codebrowser/content/ContentClassification.kt` (new `Image`/`Csv` cases, new
|
- Included: `feature/codebrowser/content/ContentClassification.kt` (new `Image`/`Csv`/`Pdf` cases,
|
||||||
`ImageFormat` enum), `feature/codebrowser/content/SourceContentClassifier.kt` (magic-byte image
|
new `ImageFormat` enum), `feature/codebrowser/content/SourceContentClassifier.kt` (magic-byte
|
||||||
detection, `.csv`-extension table parsing), `feature/codebrowser/CodeBrowserViewModel.kt`
|
image and PDF detection, `.csv`-extension table parsing), `feature/codebrowser/CodeBrowserViewModel.kt`
|
||||||
(`siblingImagePaths` on `File` state, `navigateImage`, `type=issues`-adjacent-but-unrelated —
|
(`siblingImagePaths` on `File` state, `navigateImage`, `maxInputBytes` raised from 1MB to 10MB —
|
||||||
see TASK-028 for that; this task only touches the image gallery/path plumbing),
|
see Design notes; `type=issues` is TASK-028's, unrelated to this task), `feature/codebrowser/CodeBrowserScreen.kt`
|
||||||
`feature/codebrowser/CodeBrowserScreen.kt` (`ImageContent`, `CsvTable`/`CsvRow`/`CsvCell`),
|
(`ImageContent`, `CsvTable`/`CsvRow`/`CsvCell`, `PdfContent`, shared `ZoomableSwipeableBox`),
|
||||||
`feature/branchdetail/BranchDetailScreen.kt` (pager-gating condition widened to Image/Csv),
|
`feature/branchdetail/BranchDetailScreen.kt` (pager-gating condition widened to Image/Csv/Pdf),
|
||||||
`MainActivity.kt` (`onNavigateImage` wiring), `domain/model/RepositoryModels.kt` is untouched by
|
`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
|
this task (see TASK-028 for its `Page.totalCount` addition, done in the same session but a
|
||||||
separate concern).
|
separate concern).
|
||||||
- Excluded: No new image/CSV editing, no zoom/pan on the CSV table or any other content type (only
|
- Excluded: No new image/CSV/PDF editing, no zoom/pan on the CSV table (only the image and PDF
|
||||||
the image viewer got zoom, per what was actually asked), no header-row auto-detection for CSV
|
viewers got zoom, per what was actually asked), no header-row auto-detection for CSV (the first
|
||||||
(the first row is always styled as a header — see Design notes), no thumbnail/preview generation,
|
row is always styled as a header — see Design notes), no thumbnail/preview generation, no PDF
|
||||||
no change to how non-image/non-CSV files render.
|
text extraction/search/selection (pages render as images only), no change to how other file
|
||||||
|
types render.
|
||||||
|
|
||||||
## Design notes
|
## 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
|
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
|
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
|
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
|
- **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
|
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
|
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.
|
file alone.
|
||||||
- **Pager-swipe gating (from TASK-025's last follow-up) was widened, not duplicated**: the same
|
- **Pager-swipe gating (from TASK-025's last follow-up) was widened, not duplicated**: the same
|
||||||
`currentPageShowsScrollableCode` check in `BranchDetailScreen.kt` now also covers
|
`currentPageShowsScrollableCode` check in `BranchDetailScreen.kt` now also covers
|
||||||
`ContentClassification.Image` and `ContentClassification.Csv`, so the human product owner's
|
`ContentClassification.Image`, `ContentClassification.Csv`, and `ContentClassification.Pdf`, so
|
||||||
explicit requirement — "탭 전환은 발생하면 안돼" (tab switching must not happen) while viewing
|
the human product owner's explicit requirement — "탭 전환은 발생하면 안돼" (tab switching must
|
||||||
either — is met by the existing mechanism rather than a new one.
|
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
|
## 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
|
triggers pinch/pan/zoom — the three are mutually exclusive per gesture, decided by live
|
||||||
pointer count.
|
pointer count.
|
||||||
- [x] Zoom/pan resets to 1x/centered when navigating to a different image in the gallery.
|
- [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] Build, unit tests, and lint pass.
|
||||||
- [x] Live confirmation that the image itself opens correctly — the human product owner confirmed
|
- [x] Live confirmation that the image itself opens correctly — the human product owner confirmed
|
||||||
this directly before asking for zoom.
|
this directly before asking for zoom.
|
||||||
- [ ] Live device confirmation of pinch-zoom/pan/double-tap specifically, and of the `.csv` table —
|
- [ ] Live device confirmation of pinch-zoom/pan/double-tap specifically, the `.csv` table, and the
|
||||||
not yet possible this session; see Known limitations.
|
PDF viewer — not yet possible this session; see Known limitations.
|
||||||
|
|
||||||
## Required checks
|
## Required checks
|
||||||
|
|
||||||
@ -122,45 +161,55 @@ triggers the Files/History tab to change underneath it.
|
|||||||
|
|
||||||
## Evidence
|
## Evidence
|
||||||
|
|
||||||
- `ContentClassification.kt`: new `Image(sizeBytes, content: SourceContent, format: ImageFormat)`
|
- `ContentClassification.kt`: new `Image(sizeBytes, content: SourceContent, format: ImageFormat)`,
|
||||||
and `Csv(sizeBytes, rows: List<List<String>>, isTruncated: Boolean)` cases; `content` reuses
|
`Csv(sizeBytes, rows: List<List<String>>, isTruncated: Boolean)`, and
|
||||||
`SourceContent`'s already-correct content-based `equals()`/`hashCode()` instead of holding a raw
|
`Pdf(sizeBytes, content: SourceContent)` cases; `content` reuses `SourceContent`'s already-correct
|
||||||
`ByteArray` (which would default to reference equality on a `data class`).
|
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
|
- `SourceContentClassifier.kt`: `classify(content, path: RepositoryPath? = null)` — `path` is
|
||||||
optional/defaulted so every pre-existing call site keeps compiling unchanged; `detectImageFormat`
|
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
|
checks PNG/JPEG/GIF/BMP/WebP signatures, and a separate `%PDF-` check, both before the NUL-byte
|
||||||
(`renderCsv`/`parseCsv`, a small RFC-4180-ish hand-written parser) only runs when `path` ends in
|
Binary check; CSV parsing (`renderCsv`/`parseCsv`, a small RFC-4180-ish hand-written parser) only
|
||||||
`.csv`, reusing the same line/character truncation budgets as `renderText`.
|
runs when `path` ends in `.csv`, reusing the same line/character truncation budgets as `renderText`.
|
||||||
- `CodeBrowserViewModel.kt`: `CodeBrowserUiState.File` gained `siblingImagePaths: List<RepositoryPath> = emptyList()`;
|
- `CodeBrowserViewModel.kt`: `CodeBrowserUiState.File` gained `siblingImagePaths: List<RepositoryPath> = emptyList()`;
|
||||||
`openEntry` captures it from the current `Directory` state's file entries (via new private
|
`openEntry` captures it from the current `Directory` state's file entries (via new private
|
||||||
`RepositoryPath.hasImageExtension()`); new `navigateImage(delta: Int)`.
|
`RepositoryPath.hasImageExtension()`); new `navigateImage(delta: Int)`; `maxInputBytes` raised
|
||||||
- `CodeBrowserScreen.kt`: new `ImageContent` (BitmapFactory decode + Compose `Image` rendered via
|
1MB → 10MB.
|
||||||
`Modifier.graphicsLayer { scaleX/scaleY/translationX/translationY }`, a single `awaitEachGesture`
|
- `CodeBrowserScreen.kt`: new `ImageContent` (BitmapFactory decode), `PdfContent`
|
||||||
loop combining pinch-zoom, pan-while-zoomed, and drag-threshold swipe-navigate by live pointer
|
(`android.graphics.pdf.PdfRenderer` via a throwaway temp file, `LaunchedEffect`-driven page
|
||||||
count, plus a separate `detectTapGestures` for double-tap-to-toggle-zoom), `Offset.clampedToBox`
|
render, re-opened per page turn), both rendered through a new shared `ZoomableSwipeableBox`
|
||||||
(approximate pan bounds), `CsvTable`/`CsvRow`/`CsvCell` (shared-offset horizontal scroll, matching
|
(Compose `Image` via `Modifier.graphicsLayer { scaleX/scaleY/translationX/translationY }`, one
|
||||||
the diff view's pattern); `FileContent`'s signature grew `path`/`siblingImagePaths`/
|
`awaitEachGesture` loop combining pinch-zoom, pan-while-zoomed, and drag-threshold
|
||||||
`onNavigateImage` to support both.
|
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
|
- `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
|
- 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,
|
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
|
`%PDF-` signature detection with the same NUL-byte and oversized-ordering checks, a plain-text
|
||||||
parsing, row-count truncation); `CodeBrowserViewModelTest.kt` (gallery capture in listing order
|
file starting with `%` not misclassified as PDF, `.csv` vs non-`.csv` path handling, no-path-given
|
||||||
excluding a non-image sibling, `navigateImage` moving and stopping at both ends, `navigateImage`
|
fallback to Text, quoted-field/escaped-quote parsing, row-count truncation); `CodeBrowserViewModelTest.kt`
|
||||||
no-op when opened outside a directory listing). The pinch-zoom/pan/double-tap gesture code itself
|
(gallery capture in listing order excluding a non-image sibling, `navigateImage` moving and
|
||||||
has no dedicated unit test — it's pure Compose gesture-handling with no non-UI logic to isolate
|
stopping at both ends, `navigateImage` no-op when opened outside a directory listing). Neither the
|
||||||
(see Known limitations for how it was verified instead).
|
pinch-zoom/pan/double-tap gesture code nor the PDF-rendering code (an Android-framework
|
||||||
- `./gradlew compileDebugKotlin compileDebugUnitTestKotlin`: BUILD SUCCESSFUL, both after the
|
concern — `PdfRenderer` isn't available in a plain JVM unit test without Robolectric) has a
|
||||||
initial image/CSV viewer and again after adding pinch-zoom.
|
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
|
- `./gradlew testDebugUnitTest`: BUILD SUCCESSFUL, all unit tests passing (including the new ones
|
||||||
above), both times.
|
above), across all three passes.
|
||||||
- `./gradlew assembleDebug lintDebug`: BUILD SUCCESSFUL, 0 lint errors, both times.
|
- `./gradlew assembleDebug lintDebug`: BUILD SUCCESSFUL, 0 lint errors, across all three passes.
|
||||||
- Live device: build installed over wireless `adb` twice — once for the initial image/CSV viewer,
|
- Live device: build installed over wireless `adb` three times — the initial image/CSV viewer,
|
||||||
once after adding pinch-zoom. The human product owner confirmed the image itself opens correctly
|
after adding pinch-zoom, and after adding the PDF viewer. The human product owner confirmed the
|
||||||
on the first install, then asked for zoom directly from that same live session rather than a
|
image itself opens correctly on the first install, then asked for zoom directly from that same
|
||||||
separate report. Navigating repositories looking for an existing `.csv` file to confirm the table
|
live session, then asked about PDF support as a further follow-up — all in one continuous
|
||||||
rendering end-to-end found none among those browsed this session (see Known limitations).
|
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
|
## 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
|
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
|
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.
|
a substitute for actually pinching/panning it on the device.
|
||||||
- **CSV table still not confirmed against a real `.csv` file.** Every repository browsed this
|
- **CSV table and PDF viewer still not confirmed against real files.** Every repository browsed
|
||||||
session for one didn't have a readily reachable example.
|
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
|
- 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
|
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
|
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.
|
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
|
- Pan-while-zoomed (image or PDF page) is clamped to an approximate bound (see Design notes), not
|
||||||
fitted rectangle — for most non-square images this means the clamp is slightly looser or
|
the exact fitted rectangle — for most non-square content this means the clamp is slightly looser
|
||||||
tighter than the image's true edge along one axis, not pixel-exact.
|
or tighter than the true edge along one axis, not pixel-exact.
|
||||||
- `navigateImage` after a `retry()`-recovered error loses gallery position (see Design notes) —
|
- `navigateImage` after a `retry()`-recovered error loses gallery position (see Design notes) —
|
||||||
accepted, not treated as a defect.
|
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
|
## Review findings
|
||||||
|
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user