The Issues/PR numbered-pagination page-jump field silently clamped any typed number to [1, totalPages] only on submit -- typing '999' would quietly land on the last page with no explanation. The field now shows isError/supportingText whenever the currently-typed value is out of range, making the clamp visible before the tap. Submitting still clamps exactly as before; this only adds visible feedback. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
138 lines
9.0 KiB
Markdown
138 lines
9.0 KiB
Markdown
# TASK-028: Numbered, jump-to-page pagination for the Issues/PR list
|
||
|
||
Status: implemented; all required checks pass; live device verification pending (no repository
|
||
with existing issues/PRs was found this session — see Known limitations)
|
||
Owner: Claude
|
||
Reviewer: Codex
|
||
|
||
## Outcome
|
||
|
||
The Issues/PR list now shows up to 5 numbered page buttons (tap to jump directly) plus previous/
|
||
next, and a `[ page / total ]`-style input field to type an exact page number and jump straight
|
||
there — replacing the plain previous/next-only footer, on this screen only. Per the earlier
|
||
AskUserQuestion resolution in this session ("이 기능은 전체 커밋/이슈 수를 알 수 있는 화면에만
|
||
적용"), every other paginated list in the app (repositories, history, notifications) keeps plain
|
||
previous/next, since Gitea's endpoints for those don't report a total count — Issues/PRs do.
|
||
|
||
## Scope
|
||
|
||
- Included: `domain/model/RepositoryModels.kt` (`Page.totalCount: Int? = null`),
|
||
`data/workitems/GiteaWorkItemsApi.kt` (`X-Total-Count` header parsing; `listIssues` now sends
|
||
`type=issues` so both the returned items and that header exclude pull requests server-side),
|
||
`feature/workitems/WorkItemsViewModel.kt` (`totalPages` on `IssueList`/`PullRequestList`,
|
||
`goToPage`), `feature/workitems/WorkItemsScreen.kt` (`NumberedPageFooter`, page-window logic,
|
||
the `[ input / total ]` jump field), `MainActivity.kt` (`onGoToPage` wiring).
|
||
- Excluded: No change to repository/history/notification pagination (deliberately kept as plain
|
||
previous/next — see Outcome). No change to how issue/PR *detail* screens work, only the list.
|
||
|
||
## Design notes
|
||
|
||
- **`type=issues` is a real, documented Gitea query parameter, not a guess**: verified directly
|
||
against Gitea's own source (`routers/api/v1/repo/issue.go`'s `ListIssues` swagger doc comment
|
||
lists `type` with `enum: [issues, pulls]`) before relying on it — this project's established
|
||
convention for anything not obviously provable from the client alone. The class-level doc
|
||
comment on `GiteaWorkItemsApi` previously explained *not* using this parameter as a deliberate
|
||
choice against "a guessed, unverified" one; that reasoning no longer applies now that it's
|
||
verified, so the comment was rewritten rather than left stale. The existing client-side
|
||
`pullRequest == null` filter is kept anyway, as defense in depth.
|
||
- **Why `type=issues` specifically matters for this task**: without it, Gitea's issue-list endpoint
|
||
returns issues *and* pull requests mixed together (a PR is internally an issue with a non-null
|
||
`pull_request` field), and its `X-Total-Count` header would count both — producing a total-page
|
||
count that overstates how many pages of genuine issues actually exist. Sending `type=issues`
|
||
makes the count (and the returned items) issues-only at the source.
|
||
- **`Page<T>.totalCount` is optional and defaulted** (`totalCount: Int? = null` on both the class
|
||
and `Page.of(...)`), specifically so none of the other 9+ existing `Page.of(...)` call sites
|
||
(repositories, branches, tags, commits, notifications, comments, reviews — none of which have a
|
||
total count available) needed to change.
|
||
- **The numbered-pagination UI falls back to the plain previous/next footer whenever `totalPages`
|
||
is null** (e.g. a hypothetical Gitea instance old enough to not send the header) — a defensive
|
||
fallback, not the expected path, but one that avoids ever showing a broken or missing page count
|
||
instead of just quietly degrading to what every other list already does.
|
||
- **The 5-button page window slides, centered on the current page**, clamped to stay within
|
||
`[1, totalPages]` — page 1 of a large list shows pages 1–5 (matching "1페이지에선 5까지
|
||
보여줌" exactly), and the window keeps recentering as the user navigates rather than freezing
|
||
once it nears either end.
|
||
- **No icon library dependency was added** for the "🔍"-style jump control asked for — this
|
||
project has never used `androidx.compose.material.icons` (every other screen uses plain text
|
||
labels, e.g. "다시 시도" instead of a retry icon), so the jump button uses a plain text label
|
||
("이동") consistent with that, rather than pulling in `material-icons-core` for one button.
|
||
|
||
## Acceptance criteria
|
||
|
||
- [x] The Issues list and the PR list each show up to 5 numbered page buttons, previous/next, and
|
||
a page-jump input field, when the server reports a total count.
|
||
- [x] Tapping a numbered page button jumps directly to that page.
|
||
- [x] Typing a page number and submitting (IME "Go" or the "이동" button) jumps to that page,
|
||
clamped to `[1, totalPages]`.
|
||
- [x] `X-Total-Count` is correctly excluded of pull requests for the Issues list (via `type=issues`)
|
||
and reflects genuine issue count.
|
||
- [x] A fractional last page rounds up (45 items at page size 20 is 3 pages, not 2).
|
||
- [x] Every other paginated list (repositories, history, notifications) is unchanged — still plain
|
||
previous/next.
|
||
- [x] Build, unit tests, and lint pass.
|
||
- [ ] Live device confirmation against a repository with enough real issues/PRs to see the numbered
|
||
UI populated — not yet possible this session; see Known limitations.
|
||
|
||
## Required checks
|
||
|
||
```text
|
||
./gradlew assembleDebug
|
||
./gradlew testDebugUnitTest
|
||
./gradlew lintDebug
|
||
```
|
||
|
||
## Evidence
|
||
|
||
- `RepositoryModels.kt`: `Page<T>` gained `totalCount: Int? = null`; `Page.of(...)` validates it's
|
||
non-negative when present.
|
||
- `GiteaWorkItemsApi.kt`: `listIssues` now requests `type=issues`; `toPage` parses `X-Total-Count`
|
||
(case-insensitive header lookup, matching the existing `X-HasMore` pattern) into `Page.totalCount`.
|
||
- `WorkItemsViewModel.kt`: `IssueList`/`PullRequestList` gained `totalPages: Int? = null`, computed
|
||
via a new private `Page<T>.totalPages(pageSize)` extension (`ceil(total / pageSize)`, at least
|
||
1); new `goToPage(page: Int)`.
|
||
- `WorkItemsScreen.kt`: `PageFooter` now branches on `totalPages` — null falls back to the original
|
||
previous/next row unchanged; non-null renders the new `NumberedPageFooter` (page-window buttons +
|
||
`[ input / total ]` jump row) instead. New private `pageWindow(current, totalPages, windowSize=5)`.
|
||
- New tests: `GiteaWorkItemsApiTest.kt` (`type=issues` sent, `X-Total-Count` parsed for both issues
|
||
and pull requests, absent header leaves `totalCount` null), `WorkItemsViewModelTest.kt` (45
|
||
items/page size 20 rounds up to 3 pages, missing total count leaves `totalPages` null, `goToPage`
|
||
jumps directly to the requested page), `RepositoryModelsTest.kt` (`totalCount` defaults to null,
|
||
is exposed when given, rejects a negative value).
|
||
- `./gradlew compileDebugKotlin compileDebugUnitTestKotlin`: BUILD SUCCESSFUL.
|
||
- `./gradlew testDebugUnitTest`: BUILD SUCCESSFUL, all unit tests passing (including the new ones
|
||
above).
|
||
- `./gradlew assembleDebug lintDebug`: BUILD SUCCESSFUL, 0 lint errors.
|
||
- Live device: build installed over wireless `adb`. Checked the Issues and Pull Request tabs on
|
||
four of the human product owner's actual repositories; every one had zero issues and zero pull
|
||
requests, so the numbered-pagination UI itself was never actually populated with real data this
|
||
session (see Known limitations) — confirmed instead that the screen doesn't crash and correctly
|
||
shows the existing empty state on all four.
|
||
- Same-session follow-up: `WorkItemsScreen.kt`'s page-jump field gained `isError`/`supportingText`
|
||
for an out-of-range typed value; `strings_workitems.xml` gained `workitems_page_jump_out_of_range`.
|
||
`./gradlew compileDebugKotlin compileDebugUnitTestKotlin testDebugUnitTest assembleDebug lintDebug`
|
||
BUILD SUCCESSFUL; installed on-device, no crash.
|
||
|
||
## Known limitations
|
||
|
||
- **Not yet confirmed against a real repository with enough issues/PRs to populate the numbered UI
|
||
with real data.** Every repository checked live this session had zero issues and zero pull
|
||
requests. The header-parsing, total-page-count, and jump-to-page logic are covered by unit tests
|
||
instead, but the actual rendered page-button row and jump field haven't been seen with real
|
||
multi-page data yet.
|
||
- ~~The page-jump text field accepts any positive integer typed, including one exceeding
|
||
`totalPages` — it's clamped before being submitted (`coerceIn(1, totalPages)`), so this can't
|
||
request an out-of-range page, but the field itself doesn't visually reject the keystroke.~~
|
||
**Addressed in a same-session follow-up**: the field now sets `isError`/`supportingText` (a new
|
||
string, `workitems_page_jump_out_of_range`) whenever the currently-typed number falls outside
|
||
`[1, totalPages]`, so the clamp-on-submit behavior is now visible before the tap instead of a
|
||
silent "999 quietly becomes the last page." Submitting still clamps regardless — this only adds
|
||
visible feedback, it doesn't change what actually happens on submit. Like the rest of this
|
||
feature, this specific change hasn't been seen live against real multi-page data either (same
|
||
blocker as the item above), but it's a small, self-contained Compose-only change consistent with
|
||
this file's existing, unit-tested `coerceIn` behavior — build/lint pass, installed on-device with
|
||
no crash.
|
||
|
||
## Review findings
|
||
|
||
None yet — pending Codex/human review.
|