Repository navigation
test(collection): import a Dreamborn export end to end, and keep the gear's focus through it (#1125) - #1127
Merged
Conversation
…gear's focus through it (#1125) No E2E test ever picked a file in the import dialog, so the CSV parser, the import and its receipt never ran in a browser. collection-import.spec.ts writes a small Dreamborn export from the card data (two finishes of a set 13 card, a set 3 card, a promo by its code, a row no card answers to and a census row with no copies) and imports it from Browse's gear and from the account page, checking the receipt and the collection the account receives. Three more tests cover the dialog's refusals: a file with no owned cards, a file the browser can't read, and a card list that won't load, whose Try again brings the file button back. Each test was shown to fail with the behaviour it covers broken. The first run found that Done left focus nowhere after an import from Browse's grid: the import opens the binder under the dialog, and the docked toolbar drew Collection and the gear in other elements in collection mode, so the gear the dialog handed focus back to was gone. DockedToolbar now keeps both in the same elements in either mode, changing only their boxes' style; screenshots of the toolbar in both modes match master's pixel for pixel. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J57kHm4KbVuqMeE1Gsqocv
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
size-limit report 📦
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
cubic analysis
All reported issues were addressed across 4 files
Linked issue analysis
Linked issue: Doberjohn/inkweave#1125: Add E2E for importing a Dreamborn collection file
| Status | Acceptance criteria | Notes |
|---|---|---|
| ✅ | Import from Browse and verify the receipt and uploaded collection | The Browse test checks the receipt, focus after Done, and the exact collection uploaded to the account. |
| ✅ | Import from the account page and verify the resulting collection | The account-page test checks the receipt, three cards owned, and the exact uploaded entries. |
| ✅ | Cover both finishes, older cards, promos, unmatched rows, zero counts, and CRLF | The generated export includes each case and derives card identifiers and numbers from the card data. |
| ✅ | Test the no-cards, unreadable-file, and card-list-failure refusals | Three tests assert each refusal message and verify recovery or the absence of an upload where applicable. |
| ✅ | Show that each new test fails when its covered behavior breaks | The PR description reports locally breaking the covered behaviors, and the tests assert their expected outcomes. |
| ✅ | List the new tests in E2E_TESTS.md | The inventory adds the collection-import section and updates the test and spec totals. |
Reply with feedback, questions, or to request a fix.
View guided diff | Fix all with cubic | Re-trigger cubic
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
… through cardData (#1125) Review fixes. "Cards owned" is now matched whole, so 13 or 30 no longer passes for 3. The two refusal tests import a good file next and expect its upload to be the only one: a refused file that still started an upload, which lands after the alert, failed only the new check (shown by calling onImport in the unreadable path). The fixture picks a card alone at its number among the set's cards and its chunk's printings, as the parser reads them, and a promo whose card the list holds. The chunk and promo readers move into e2e/helpers/cardData.ts beside the shapes it already declares. BrowseToolbar's no-raw-spacing count is now x5, its two trailing margins being one const. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J57kHm4KbVuqMeE1Gsqocv
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…1125) BrowseToolbar.tsx leaves the raw-spacing ledger, as the token rule asks of a file a PR touches. The four 10 px gaps take SPACING.md (12) and the chips' 6 px gap takes SPACING.sm (8), the larger step in each case. The phone's grid and collection rows are pixel-identical; the desktop controls and every screen's filter chips move apart by 2 px. LEDGER_CEILING drops to 171. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J57kHm4KbVuqMeE1Gsqocv
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Brings in #1117, #1124 and #1120. E2E_TESTS.md's total is resolved to the sum of both sides: 424 tests across 38 spec files. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J57kHm4KbVuqMeE1Gsqocv
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Brings in #1126 (binders). The docked toolbar keeps this branch's shape, where Collection and its actions stay in the same elements in both modes, and takes #1126's binder-sized picker group and slot (pickerSpanStyle, pickerSlotStyle). E2E_TESTS.md's total is the sum of both sides: 432 tests across 39 spec files. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J57kHm4KbVuqMeE1Gsqocv
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Owner
Author
|
The fix is #1129, which passed on all five browsers. I'm porting its two commits into this branch once the push I have running ends, then re-running this PR's full E2E. Generated by Claude Code |
…ison (#1128) Under reduced motion card B hides at once on BACK, stays for the exit's 480 ms and then unmounts. WebKit drew that stay in 3 frames, so the check for four frames at opacity 0 failed though each frame was 0. The sampler now stops once card B leaves, and the test needs at least one frame and every frame at 0. In Chromium, a 50 ms stay reproduces WebKit's ['0', '0', '0'] failure and the fixed test passes it. With the exit's reduced-motion branch skipped, the fixed test fails on card B's opacity of 1. Closes #1128 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J57kHm4KbVuqMeE1Gsqocv
…e it stays (#1128) The leaving check read at most four frames, so card B showing again later in its 480 ms stay still passed, and it needed a frame after the mode flip to read anything at all. The sampler now also reads card B when data-mode flips, from a MutationObserver that runs after the exit's layout effect, and then on every frame until card B is gone. The early stop applies only to leaving, so the comparison checks still read their four frames. The two leaving assertions become one Set comparison, which also shows the opacities read, and E2E_TESTS.md describes the new check. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J57kHm4KbVuqMeE1Gsqocv
There was a problem hiding this comment.
Gates Passed
See analysis details in CodeScene
Quality Gate Profile: Pay Down Tech Debt
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
This branch was successfully 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.
Before: no E2E test ever picked a file in the collection import dialog, so the Dreamborn CSV parser, the import and its receipt never ran in a browser, and neither did the dialog's refusals. And after an import from Browse's grid, pressing Done left keyboard focus nowhere: the import opens the binder under the dialog, which rebuilt the toolbar's Collection button and gear, so the gear the dialog handed focus back to was gone.
After:
collection-import.spec.tsimports a small Dreamborn export from Browse's gear and from the account page, and checks the receipt ("3 cards, 5 copies in all.", the unmatched row named) and the collection the account receives. Three more tests cover a file with no owned cards, a file the browser can't read, and a card list that won't load, whose Try again brings the file button back. Done after a Browse import now leaves focus on the gear. The toolbar's five raw gaps take spacing tokens, so on desktop its controls sit 12 px apart instead of 10, and filter chips sit 8 px apart instead of 6 on every screen.How: the export is written in the spec from the card data (two finishes of a set 13 card, a set 3 card, a promo by its code, a row no card answers to and a census row with no copies), with Windows line endings, so no card id or number is hardcoded. The unreadable file comes from an init script that makes
Blob.prototype.textreject for that file's name.DockedToolbarnow draws Collection and the collection's actions in the same elements in both modes and changes only their boxes' style (display: contentswhere they sit in the row), so a mode change keeps them and focus on them. Before the spacing change, screenshots of the docked toolbar in both modes matched master's pixel for pixel (maxDiffPixels: 0).BrowseToolbar.tsxthen leaves the raw-spacing ledger (the owner's pick): each 10 becomesSPACING.mdand the chips' 6 becomesSPACING.sm. The phone's grid and collection rows stay pixel-identical at 390 and 360 px. Before and after shots: Browse Toolbar Spacing.Each new E2E test was shown to fail with the behaviour it covers broken: the receipt's copy count, the zero-copies check, the unreadable-file message, the card list's Try again, an import that merged instead of replacing, an import that didn't upload, and the toolbar change reverted (focus off the gear). The new unit test in
BrowseToolbar.test.tsxfails on master's toolbar too. The other specs that drive Browse's toolbar (collection mode, start, binder, friends, Browse's grid and filters, mobile, search, loading geometry, accessibility, focus rings, skip link) pass in Chromium and mobile Chrome.Closes #1125
Generated by Claude Code
Summary by cubic
Adds end-to-end coverage for importing a Dreamborn collection export, and fixes two focus and animation-read bugs the tests exposed.
collection-import.spec.tswrites a small Dreamborn CSV from the card data (two finishes, a pre-Core card, a promo, an unmatched row, and a zero-count row) and imports it from Browse's gear and from the account page, checking the receipt and the upload the account receives. Three more tests cover refusals — a file with no owned cards, a file the browser can't read, and a card list that won't load — each verifying that a good file chosen next imports cleanly, its upload the only one the account gets.E2E_TESTS.mdrecords the new tests.The imports from Browse exposed a real bug: Done left keyboard focus nowhere, because the import opened the binder under the dialog and
DockedToolbarrebuilt Collection and the gear in new elements when switching modes. The toolbar now keeps both in the same elements in either mode, changing only their boxes' style, so focus survives; the refactor also snaps its five raw gaps to spacing tokens, so desktop controls sit 12 px apart instead of 10 and filter chips 8 px apart instead of 6.Bug Fixes
synergy-detail-modal.spec.tsno longer assumes card B lasts four frames after BACK; WebKit can draw its 480 ms stay in three, so the sampler now reads card B as the mode flips and on every frame it lasts.Written for commit 0898a80. Summary will update on new commits.