Ubisam-MobileGit/tasks/027-image-and-csv-viewer.md
choiminyeong f6ac20c8ed TASK-063: task records say what is actually open
No task waits on a Codex review; the redesign tasks are confirmed on the
product owner's Z Fold7; TASK-016's leftovers are on hold; no biometric
reauthentication.

Agent: main (opus)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-09-29 20:25:49 +09:00

248 lines
18 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# 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 and the PDF viewer are unconfirmed live — see Known limitations
Owner: Claude
Reviewer: Codex
## Outcome
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`/`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/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
- **Image detection is by magic bytes, not file extension** (`detectImageFormat` checks the PNG/
JPEG/GIF/BMP/WebP signatures directly): a mislabeled or extensionless file still renders
correctly if it genuinely is one of these formats, and a renamed non-image file isn't
misclassified just because of its name. It runs *before* the existing NUL-byte-means-Binary
check, since every one of these formats' bytes contain NULs and would otherwise always be caught
as generic Binary first.
- **The sibling-image gallery is built from the directory listing already in hand**, not a second
fetch: `CodeBrowserViewModel.openEntry` captures the current `Directory` state's file entries
(filtered to image extensions — extension-only, since building the gallery list can't afford to
fetch and classify every sibling's bytes just to find out which are images) into the new `File`
state's `siblingImagePaths` at the moment a file is opened. `navigateImage(delta)` then looks up
the current path's index in that list and loads the neighbor, with no bound past either end.
Opening a file *outside* a directory listing (currently only possible via `retry()` after an
error) leaves `siblingImagePaths` empty — swipe navigation is simply unavailable there, a known,
accepted edge case rather than extra plumbing through `Loading`/`Error` states for it.
- **The image swipe gesture is a discrete drag-distance threshold** (a 120px total-drag check on
release), not a continuous scroll position — unlike the diff view's horizontal scroll, there's
no partial-scroll state to render mid-drag, only "did the user swipe far enough to go to the
next/previous image."
- **Pinch-zoom, pan, and swipe-navigate share one hand-rolled gesture loop** (`awaitEachGesture`,
reading live pointer count each event), not three separate detector modifiers stacked on the
same node — Compose broadcasts pointer events to every `pointerInput` block on a node
cooperatively rather than picking one exclusive winner, so two independent detectors racing for
the same single-finger drag (one wanting to pan/zoom, one wanting to swipe-navigate) would have
fought each other exactly like the tab-swipe-vs-content-scroll bug TASK-025 already hit twice.
Deciding inline, per pointer event, which behavior applies avoids that class of bug outright: 2+
pointers is always pinch-zoom/pan; 1 pointer while already zoomed in (`scale > 1f`) pans; 1
pointer at 1x is reserved for the existing swipe-navigate accumulator. A double-tap
(`detectTapGestures` in a second, non-competing `pointerInput` — taps don't accumulate drag
distance, so it doesn't fight the first block) toggles between 1x and 2x zoom, matching common
photo-viewer convention. Zoom/pan/offset are `remember(content)`-scoped, so switching to a
different image via gallery swipe always starts that image at 1x, not carrying over the previous
image's zoom. 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
container aspect ratio compared), which wasn't worth the extra complexity for keeping a zoomed
image roughly on screen.
- **The CSV table reuses the diff view's manually-driven shared-offset pattern** for its horizontal
scroll (`rememberScrollableState` + `Modifier.offset`, one offset shared across all rows via
`Modifier.scrollable` per row) rather than a plain shared `Modifier.horizontalScroll`. Every CSV
cell here is the same fixed width, so the specific bound-thrashing bug that pattern was built to
avoid (TASK-025's "글자가 춤을 추네") couldn't actually occur with uniform-width cells — but
reusing the already-verified-correct pattern costs nothing and keeps the codebase's one way of
doing synced horizontal scroll, rather than two.
- **No header-row detection for CSV**: the first row is always rendered with a highlighted
background, matching how most spreadsheet tools default a first-row-as-header assumption, since
actually detecting whether a CSV has a header (vs. all-data) isn't reliably possible from the
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`, `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
- [x] Opening a PNG/JPEG/GIF/BMP/WebP file (detected by magic bytes) shows the decoded image.
- [x] Swiping left/right on an image with siblings in the same folder moves to the next/previous
image in that folder's listing order; swiping past either end does nothing.
- [x] Swiping (or any horizontal drag) on an image or a CSV table never changes the Files/History
tab underneath it.
- [x] Opening a `.csv` file shows a scrollable table with the first row styled as a header; a
non-`.csv` file with comma-separated content is unaffected (still plain text).
- [x] Quoted CSV fields containing commas and escaped (`""`) quotes parse correctly.
- [x] Pinching zooms an image in/out (clamped 1x–5x); a single finger pans while zoomed in;
double-tap toggles between 1x and 2x.
- [x] Pinch/pan/double-tap never trigger gallery swipe-navigation, and swipe-navigation never
triggers pinch/pan/zoom — the three are mutually exclusive per gesture, decided by live
pointer count.
- [x] Zoom/pan resets to 1x/centered when navigating to a different image in the gallery.
- [x] 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, the `.csv` table, and the
PDF viewer — not yet possible this session; see Known limitations.
## Required checks
```text
./gradlew assembleDebug
./gradlew testDebugUnitTest
./gradlew lintDebug
```
## Evidence
- `ContentClassification.kt`: new `Image(sizeBytes, content: SourceContent, format: ImageFormat)`,
`Csv(sizeBytes, rows: List<List<String>>, 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, 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<RepositoryPath> = emptyList()`;
`openEntry` captures it from the current `Directory` state's file entries (via new private
`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`/`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,
`%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), 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
- **Image opening itself is live-confirmed; pinch-zoom/pan/double-tap is not yet.** The human
product owner confirmed the decoded image renders correctly, then asked for zoom in the same
session — there hasn't yet been a follow-up live confirmation specifically of the zoom/pan/
double-tap gestures themselves. Unlike the earlier gesture-conflict bugs in this session (found
only by live testing, not reasoning), there was no repo/file readily available to hand-test
against interactively beyond a single already-open image; the gesture-arbitration design (live
pointer-count branching in one gesture loop, see Design notes) was chosen specifically to
structurally prevent the same class of bug TASK-025 hit twice, but that's a design argument, not
a substitute for actually pinching/panning it on the device.
- **CSV table 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 (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
None yet — no Codex review pending (Codex retired, 2026-09-29).