Ubisam-MobileGit/tasks/027-image-and-csv-viewer.md
choiminyeong 8bf3930c3d TASK-027 follow-up: pinch-to-zoom and pan for the image viewer
Pinch-zoom (1x-5x), pan while zoomed, and double-tap to toggle zoom, added
to the image viewer. Combined with the existing swipe-navigate gesture in
one hand-rolled awaitEachGesture loop that branches on live pointer count,
rather than stacking a second competing pointerInput detector -- avoiding
the same class of tab-swipe/content-scroll gesture conflict TASK-025 hit
twice.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-27 22:01:09 +09:00

191 lines
14 KiB
Markdown
Raw 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 navigation, pinch-to-zoom) and CSV table viewer
Status: implemented; all required checks pass; the human product owner confirmed the image opens
live, pinch-zoom is unconfirmed live — see Known limitations
Owner: Claude
Reviewer: Codex
## Outcome
Three live requests, delivered as one connected feature: opening an image file in the code
browser now shows the image (not a generic "binary file" fallback), with left/right swipe to move
to the previous/next image in the same folder, and pinch-to-zoom with pan (plus double-tap to
toggle zoom) once the human product owner confirmed the image itself opens correctly and asked for
zoom next. Opening a `.csv` file shows a scrollable table (rows/columns, first row styled as a
header) instead of raw comma-separated text. All three reuse the tab-swipe/content-scroll
gesture-conflict fix from TASK-025's last follow-up: interacting with an image or a CSV table never
triggers the Files/History tab to change underneath it.
## Scope
- 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),
`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.
## 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.
- **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` 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.
## 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] 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.
## Required checks
```text
./gradlew assembleDebug
./gradlew testDebugUnitTest
./gradlew lintDebug
```
## Evidence
- `ContentClassification.kt`: new `Image(sizeBytes, content: SourceContent, format: ImageFormat)`
and `Csv(sizeBytes, rows: List<List<String>>, 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`).
- `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`.
- `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)`.
- `CodeBrowserScreen.kt`: new `ImageContent` (BitmapFactory decode + Compose `Image` rendered via
`Modifier.graphicsLayer { scaleX/scaleY/translationX/translationY }`, a single `awaitEachGesture`
loop combining pinch-zoom, pan-while-zoomed, and drag-threshold swipe-navigate by live pointer
count, plus a separate `detectTapGestures` for double-tap-to-toggle-zoom), `Offset.clampedToBox`
(approximate pan bounds), `CsvTable`/`CsvRow`/`CsvCell` (shared-offset horizontal scroll, matching
the diff view's pattern); `FileContent`'s signature grew `path`/`siblingImagePaths`/
`onNavigateImage` to support both.
- `BranchDetailScreen.kt`: `currentPageShowsScrollableCode`'s page-0 branch now also checks
`ContentClassification.Image`/`ContentClassification.Csv`, 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.
- `./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).
## 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 still not confirmed against a real `.csv` file.** Every repository browsed this
session for one didn't have a readily reachable example.
- The CSV table's column width is a fixed 120dp per cell, not sized to content — a column with
very short values wastes horizontal space, and a column with very long values still clips at
120dp (readable via horizontal scroll, but not auto-fit). Not raised as a concern by the human
product owner; flagged here in case it's worth revisiting once seen live.
- Pan-while-zoomed is clamped to an approximate bound (see Design notes), not the image's exact
fitted rectangle — for most non-square images this means the clamp is slightly looser or
tighter than the image's true edge along one axis, not pixel-exact.
- `navigateImage` after a `retry()`-recovered error loses gallery position (see Design notes) —
accepted, not treated as a defect.
## Review findings
None yet — pending Codex/human review.