Skip to content

fix(collection): return to the viewed Pokédex page and catch log row after Back - #435

Merged
chattymin merged 10 commits into
chattymin:mainfrom
heli-os:fix/pokedex-back-keeps-position
Oct 16, 2026
Merged

chattymin merged 10 commits into
chattymin:mainfrom
heli-os:fix/pokedex-back-keeps-position

Conversation

@heli-os

@heli-os heli-os commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Back from a Pokédex detail page always landed on page 1 of the grid, even when the species was opened from page 3. The catch log had the same problem: Back scrolled it to the top.

Why: CollectionView swaps the grid/log out for PokemonDetailView with an if/else. Opening a detail page unmounts DexGridView, which drops its @State page. Back mounts a fresh grid on page 1. The filters survived because they live one level up, in CollectionView.

Fix: CollectionView remembers what the detail page was opened from and hands it to the rebuilt view once.

  • Pokédex: on Back, the grid opens on the page of the species just viewed (DexScrollPager.page(containing:in:pageSize:)). Nothing is selected, the same as the catch log. The page follows the species, not a page number, so:
    • opening a species from a Home sprite also returns to that species' page (previously page 1)
    • a catch that reorders the list while the detail page is open still lands on the right page
    • a species filtered out of the current results falls back to page 1, as before
  • Catch log: the row that opened the detail page is scrolled back to the center. It scrolls twice. The first scrollTo places the row using estimated heights for the lazy rows, and once the rows it reveals are measured the target shifts (one row short on CI). The second scroll runs after that layout pass. scrollPosition(id:) was tried too: without anchor: (macOS 14) it overshot the end of the list and left the log blank.
  • Both anchors are used once, so a later rebuild (switching segments) still starts at the top.

Commits are split as: pure page lookup + tests → Pokédex fix → catch log fix → defect-log entry → PR images → catch log correction from the first CI run (+ its defect-log note) → grid anchor set only when Back lands on the grid → grid anchor split from the selection (+ re-rendered image).

Type of change

  • Bug fix
  • New feature
  • Refactor / cleanup
  • Documentation
  • Other:

UI changes

Sample-data renders of the production CollectionView (offline fixture, so the detail page shows its "could not be loaded" state). The "Before" column is the state Back produced before this change: a freshly built grid/log, rendered from the same fixture.

Pokédex: open #18 Pidgeot (page 2), e.g. from its Home sprite, then press Back.

Detail page of #18 — sample-data render

Before After
Before: page 1 after Back — sample-data render After: back on page 2, nothing selected — sample-data render

Catch log: scroll down, open Butterfree's entry, then press Back.

Catch log scrolled to Butterfree — sample-data render

Before After
Before: log back at the top — sample-data render After: Butterfree's row back in view — sample-data render

Checklist

  • swift build and swift test pass locally
  • PR title follows Conventional Commits and title/description are written in English
  • UI changes include embedded images (before/after for existing screens; image of a new screen), or there is no visible UI change
  • No copyrighted assets, secrets, or private tooling references are committed (see CONTRIBUTING)
  • Tests were added or updated for this change

Tests

  • DexScrollPagerTests: page lookup for first/last page, the 16th/17th boundary, nil, and filtered-out IDs.
  • DexBackNavigationTests (new): hosts the real CollectionView in a window, does the detail → Back round trip, and reads the result off the rendered pixels with Vision. The accessibility tree is empty without an AX client, so it can't be used here. Each guard was checked by injecting the defect: removing the anchor, the one-time use, the log anchor, or the second log scroll, keeping the grid anchor after a Back to the log, or letting the anchor select the species each makes a test fail. Presses retry until the screen changes, because right after a transition the view tree can still hit-test the previous screen (a missed Back press on the first CI run).
  • Full suite: 1514 tests, 0 failures (15 skipped), logic-core line coverage 93.74% (threshold 75%). Every new branch runs in tests (llvm-cov show, no ^0).
    • Two earlier full runs on a heavily loaded machine each had unrelated flaky failures (SessionKeySettingsRenderingTests, then SpriteImageCacheTests/UnownSpriteCacheTests). They pass alone and together with the new tests.
    • Locally, test-gate.sh couldn't locate the test binary under Xcode's Swift 6.4 build layout, so the coverage numbers above come from running its llvm-cov report step by hand. CI runs the gate as usual.

🤖 Generated with Claude Code

heli-os and others added 10 commits October 10, 2026 00:52
DexScrollPager.page(containing:in:pageSize:) returns the page that shows
a species, or 0 when it is nil or filtered out. Not wired up yet; the
next commit uses it to return to the viewed page after Back.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The detail page replaces the grid, so Back rebuilt DexGridView and its
page state started over at page 1. CollectionView now remembers the
species whose detail page was closed and the rebuilt grid starts with it
selected, deriving the page from that selection. Opening a species from
a Home sprite now also lands on its page on Back. The anchor is used
once, so a later rebuild (segment switch) still starts on page 1.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The catch log sits in the same branch the detail page replaces, so Back
scrolled it to the top. The row that opened the detail page is now
scrolled back to the center. The scroll waits one turn after the log
appears: before the first layout the lazy rows above it are estimated
and the scroll stopped short of the row.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sample-data renders of the real CollectionView for the PR.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
CI showed the restored log stopping a row short of the row that opened
the detail page. The first scrollTo places the target with estimated
heights for the lazy rows; once the rows it reveals are measured they
push the target off its spot. Scroll again after that layout pass. A
single scroll fails the test locally too.

The tests also retry a press until the screen changes: right after a
transition the view tree can still hit-test the previous screen, which
made a Back press miss on CI.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…he grid

Back always stored the closed species as the grid's return anchor, but
only the grid's onAppear clears it. When Back returned to the catch log
(the detail page was opened from a log row, or from a Home sprite while
the log segment was selected), the anchor stayed, and the next switch
to the Pokédex segment opened on that species' page with it selected
instead of page 1.

testBackToTheLogLeavesTheGridOnItsFirstPage covers that path and fails
when the anchor is stored unconditionally again.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… the species

The grid found its return page by starting with the closed species as
its selection. That brought back the selection border and the footer
line, which nothing had shown since the detail page started replacing
the grid (chattymin#264), and made the grid behave unlike the catch log, which
only scrolls back to the row.

DexGridView now keeps the anchor in its own state and uses it only to
pick the page. testBackFromDetailReturnsToThePageOfThatSpecies asserts
that the footer does not name the species and fails when the anchor
selects it again.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sample-data render of the real CollectionView after Back: page 2 with no
species selected.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@chattymin
chattymin merged commit a4d17d5 into chattymin:main Oct 16, 2026
2 checks passed
@chattymin

Copy link
Copy Markdown
Owner

Thank you for your contribution!

KillianHzr added a commit to KillianHzr/PokeTokenLBI that referenced this pull request Oct 19, 2026
…GridView init

Upstream chattymin#435 gave DexGridView an explicit init, which dropped the evolutionFamily property added by this branch.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants