Repository navigation
Conversation
chattymin
requested changes
Sep 24, 2026
chattymin
left a comment
Owner
There was a problem hiding this comment.
Please rebase this feature onto the current Pokémon detail screen and add flavor text as a section instead of replacing the existing stats, nature, abilities, moves, and instance selection UI. The request identity must include both species ID and language, and a response should only be applied if the current language still matches. Please add a regression test for out-of-order responses during a language change and keep the existing detail content available on network failure.
One GraphQL query returns the requested language's entries with localized version names. Any GraphQL failure (transport, HTTP status, an `errors` payload, an undecodable body) falls back to REST; that result carries slug labels, so it is marked degraded and never cached. A language with no entries at all (PokéAPI has no `pt`) is answered in English. PokemonFlavorTextProviding is its own protocol, like PokemonDetailProviding, so existing test doubles stay untouched. The client's transport is injectable for tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Results, failures and in-flight tasks are keyed by FlavorTextRequest (species ID + language). The request is captured before the first await and re-checked before a success or a failure is stored, so a response for a language that is no longer selected is dropped. The store owns the in-flight task: a detail page that closes does not cancel it, and reopening joins it. Degraded results stay visible and are fetched again on the next visit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ersion The species data card shows the newest entry and an "All Pokédex entries" button. The button opens every version's entry, oldest first, on a page laid over the detail page, so Back returns to the same individual and scroll position. Loading, failure and retry stay inside the description area, and a failed refetch keeps the entry already shown. Entries load in their own task, alongside battle details. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tore Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
LeeTaek
force-pushed
the
feat/pokedex_desc
branch
from
October 6, 2026 01:12
56ceaa1 to
6268b69
Compare
Contributor
Author
|
Thanks for the review. I rebuilt this branch on current
Images and validation results are in the description. |
CI (Xcode 16.4, Swift 6.1.2) runs a synchronous tearDown override of a @mainactor XCTestCase as nonisolated, so the main-actor arrays and the shared sprite cache it touched failed to compile there; local Swift 6.2 accepted them. Temporary files now live under one `let` directory that tearDown removes, and the sprite-cache cleanup moved into the test body, as other rendering tests already do. Recorded in the defect log. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
On the macOS 15 runner, Vision read body-size text as "Samole" or "Pokedey" and three rendering tests failed although the page was right; locally (Retina, newer Vision) the reads were exact. Capturing at a higher scale alone does not help: cacheDisplay resamples text that was already rasterized at the window's scale. OCR now runs on a 3x enlargement with the expected words as custom words, and text that must be present may differ by up to 15% of its letters, while text that must be gone is still matched exactly. The entries-page fixture uses different first words so a tolerant match cannot confuse entries. With a capture degraded to 0.8x, 1x and 2x all six tests pass (0.6x: 5 of 6, from 1 of 6 before), and each of the seven injected view defects still fails its test. Recorded in the defect log. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Resolve the PokemonDetailView conflict with chattymin#376 (alternate forms): the detail page's .task stays on the ZStack that hosts the entries page, and uses the renamed selectedForm / displayedSpecies.form. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Under heavy load Vision dropped the short accent-colored "Sword" label while reading every other line of the card, failing the refetch test although the card was right. The entry text and the retry row still prove the entry survived the failed refetch; the store now confirms which label it carries. Injecting a failed refetch that hides the entry still fails the test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds version-specific Pokédex entries (PokéAPI flavor text) to the existing Pokémon detail page, following the review on this PR. The Species data card shows the newest entry in the app language, and All Pokédex entries opens every version's entry, oldest first, on a page laid over the detail page; Back returns to the same individual and scroll position. Stats, nature, abilities, moves and the individual picker are unchanged.
The branch is rebuilt on current
main(9be4ea9). Changes from the earlier revision thatmainhas since handled differently — theja-hrktcasing, shiny sprite seeding, the representative button and cell taps opening the detail page — are no longer part of this PR.Review follow-up
FlavorTextRequest(speciesID, language)keys results, failures and in-flight tasks. The language is captured before the firstawaitand checked again before a success or a failure is stored.DexFlavorTextStoreTestscovers both completion orders and a stale failure. Separate tests lock the language-keyed cache and the in-flight identity, because with the landing guard in place a species-only key would still pass the out-of-order test. Each test was confirmed to fail with its defect injected..task. Loading, failure and retry stay inside the description area, and a failed refetch keeps the entry already shown.DexFlavorTextRenderingTestsrenders the real detail page in these states.Implementation notes
errorspayload (PokéAPI returns these with HTTP 200 and nodata), or an undecodable body — falls back to RESTpokemon-species/{id}. Those results have slug version labels, so they are marked degraded: shown, never cached, and fetched again on the next visit.PokemonFlavorTextProvidingis separate fromPokeProviding, likePokemonDetailProviding, so existing test doubles are untouched and stores built with stub providers stay offline.PokeAPIClientconforms by delegating toPokemonFlavorTextClient, whose transport is injectable for tests.dexNameRequestsdoes. Closing the detail page does not cancel it, and reopening the page joins it.AppLanguage.apiCodes(ja-hrktfirst) whatever order the rows arrive in. A version without an entry in the selected language is omitted rather than filled from English. PokéAPI has non-English entries only from X/Y onward (French from Black/White) and none forptorru; those two show English with a notice, as species names already do.Type of change
UI changes
All images are sample-data renders of the production
PokemonDetailView, hosted offscreen at popover width in the light appearance and cropped to the affected area. Buttons render gray because the offscreen window is inactive; in the popover they use the accent color.Validation
swift buildand./scripts/test-gate.shon this commit: 1,522 tests, 15 skipped (existing opt-in harnesses), 0 failures; logic-core line coverage 93.71%. This PR adds 28 tests: 16 for the client, 6 for the store and 6 rendering tests.llvm-cov --show-regions. No^0in the new client, store and view code except the default URLSession transport, which tests always replace.kohas 12 entries with localized version names,ja12 (kana),en33 andde12, whileptfalls back to English. PokéAPI has norulanguage at all.tearDownoverrides that local Swift 6.2 accepted, and OCR misreads of body-size text on the macOS 15 runner ("Samole" for "Sample"). The rendering tests now run OCR on an enlarged capture, give Vision the expected words, and allow a few misread letters in text that must be present; text that must be gone is still matched exactly, and every injected view defect above still fails its test.Documentation
docs/reference/defect-log.md:setUp/tearDownof a@MainActorXCTestCase is nonisolated on CI's Swift 6.1, so it may only touchletconstants; and OCR on CI is less accurate than locally, so tests that read the screen must tolerate misreads.Checklist
swift buildandswift testpass locally🤖 Generated with Claude Code