diff --git a/.claude/memory/pr-screenshots-workflow.md b/.claude/memory/pr-screenshots-workflow.md index 4ab9c80b..5f67bb1b 100644 --- a/.claude/memory/pr-screenshots-workflow.md +++ b/.claude/memory/pr-screenshots-workflow.md @@ -37,3 +37,5 @@ metadata: - **Tab bar in narrow-frame shots (spec 077, 2026-11-11):** the fixed tab bar sits at the frame's bottom: a frame taller than the window (e.g. `open_in_narrow_frame` with `height: 1500`) keeps it out of an element shot of a panel; a full-page phone shot needs a frame no taller than the window's viewport to show it (spec 074). **A frame taller than the window's viewport loses the fixed tab bar (spec 074, 2026-11-11):** a narrow frame is clipped to the window, and the phone tab bar is fixed to the frame's bottom, so the picture shows no bar. Reproduced: an 815px frame in a 1280×800 window (a 715px viewport) gave an 815px shot with no bar, in a 1280×900 window (815px viewport) the same shot had it, and a 1700px frame in this suite's 1400×1400 window (1315px viewport) gave a 1315px shot with no bar. Spec 071's shots 03, 04, 05 and others look like this; its window can't be read back, so the cause is reproduced, not confirmed. Make the window taller than the frame (`page.current_window.resize_to(…)`, and the after hook restores it) or shoot in a frame no taller than the viewport, and look for the bar in every phone shot. + +**Scanner pane shots (spec 088, 2026-11-13):** from a throwaway system spec with the synthetic camera, as above. Seed an applied `Catalog::RefreshRun` or the "catalog not loaded" notice overlays the camera in every shot. Park the pointer off the controls before each shot: "Scan again" otherwise shows its hover colour in some shots. An empty live region takes no space, so the driver reports it not displayed: assert it with `visible: :all`. Wait for the pane at rest (`document.getAnimations()` idle and the hidden "Done." sentence). Never `rm` between runs (the harness prompts the maintainer): write each re-run to a fresh literal folder. diff --git a/CLAUDE.md b/CLAUDE.md index d2c4e4f4..26e97bbf 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -39,7 +39,7 @@ Ruby 4.0.7, Rails 8.1.4, SQLite in every environment (including production), Hot - **Admin catalog and jobs (specs 015, 024, 055; ADRs 0014, 0015):** the refresh and art build lifecycles, stall and interrupt rules, rate-limit retries, "Retry now" for a waiting retry and the `/admin/catalog` and `/admin/jobs` pages are in [`docs/architecture/catalog-operations.md`](docs/architecture/catalog-operations.md). In specs, tag an example `:solid_queue` to queue through Solid Queue (no worker runs) and move jobs with `queue_job`, `claim_job`, `fail_job`, `finish_job`. - **Footer and open catalog (specs 018, 023, 084, 087):** the footer's lines come from each source's `.content_notice`, the licence line is the core's last line, and core code never names a collectible (`spec/footer_core_spec.rb`, `spec/licences_core_spec.rb`). A new gem, pinned OCR package, importmap pin outside `app/javascript`, font or icon needs its entry in `config/third_party_licences.yml` (or, for a gem, a licence), or the suite fails. `Catalog.public?` is the one test of `COLLECTOR_PUBLIC_CATALOG`, never read from the environment in tests (tag an example `:public_catalog`), and specs set the build facts with `build_facts(...)`, never ENV; views use `collector?`, never the setting. Rules: [`catalog-core.md`](docs/architecture/catalog-core.md). - **Active Storage variants (spec 014, ADR 0013)** use libvips through `ruby-vips`, which is bundled with `require: false` so the app boots without libvips (it then logs "requires the ruby-vips gem"). Development needs libvips (`vips` on Fedora, `libvips` on Debian/Ubuntu): `bin/setup` prints a hint when it's missing (`Collector::LibvipsCheck`), CI installs it, and `bin/image-smoke` fails an image whose variant transformer isn't vips. The only Active Storage upload is the import preview's stored file (`ImportUpload`, spec 028, ADR 0022), which meets ADR 0013's owed validation for itself (content type by content, size; no pixels, it is no image); pixel limits stay with the first image upload. Active Storage's routes are gated app-wide by `ActiveStorageGate` (`config/initializers/active_storage_gate.rb`): sign-in, ownership of original blobs only, and 404 for direct uploads and the disk service's upload PUT until a feature needs them; the first image feature extends ownership to variants and previews (ADR 0022). -- **Card scanner (specs 005, 007–011, 040, 042, 047, 082):** `/scanner`, a one-screen focused page (spec 082), linked from the navigation ("Scan") and the collection page. How it reads a card, ranks candidates (with the optional art match), adds a copy, keeps the sitting with its condition and place, and records findings is in [`docs/architecture/scanner.md`](docs/architecture/scanner.md). Before you look: the OCR engine is fetched and checksum-verified into the ignored `vendor/ocr/v7.0.0/` by `bin/fetch-ocr-engine` (run by `bin/setup` and the Dockerfile), and scanner specs fail until it's installed; art matching (opt-in, `COLLECTOR_MTG_ART_MATCHING`) needs ImageMagick (`magick` or `convert`) for its build and specs; only scanner pages send a Content-Security-Policy (`ScannerPage` concern); the sitting's files name no collectible (`spec/scanner_sitting_core_spec.rb`); `script/scanner/detector_parity.rb` checks the detector (`app/javascript/scanner/detector.js`) against the spike's; measurement mode (`/scanner/measurement`, with the `scanner:` rake tasks and `script/scanner/` for the findings) is development-only (`config.x.scanner_measurement`; `COLLECTOR_SCANNER_MANIFEST`, `COLLECTOR_SCANNER_RUN_DIR`, `COLLECTOR_SCANNER_KEEP_FRAMES=1` keeps live frames) and writes outside the repo; the spikes are in `spikes/card_scanner/`. Phones need HTTPS: `bin/dev-certificate` makes a self-signed certificate outside the repo and prints the `bin/dev -b "ssl://…"` bind (alternative: a tunnel with `RAILS_DEVELOPMENT_HOSTS` + `COLLECTOR_HTTPS=true`). +- **Card scanner (specs 005, 007–011, 040, 042, 047, 082, 088):** `/scanner`, a one-screen focused page (spec 082), linked from the navigation ("Scan") and the collection page; a result opens in its selection pane (spec 088, `Scanner::Selection`, `scanners/_selection`): the capture's strip, a status band, a swipe row of printings or possible cards and one add button per finish. How it reads a card, ranks candidates (with the optional art match), adds a copy, keeps the sitting with its condition and place, and records findings is in [`docs/architecture/scanner.md`](docs/architecture/scanner.md). Before you look: the OCR engine is fetched and checksum-verified into the ignored `vendor/ocr/v7.0.0/` by `bin/fetch-ocr-engine` (run by `bin/setup` and the Dockerfile), and scanner specs fail until it's installed; art matching (opt-in, `COLLECTOR_MTG_ART_MATCHING`) needs ImageMagick (`magick` or `convert`) for its build and specs; only scanner pages send a Content-Security-Policy (`ScannerPage` concern); the sitting's files name no collectible (`spec/scanner_sitting_core_spec.rb`); `script/scanner/detector_parity.rb` checks the detector (`app/javascript/scanner/detector.js`) against the spike's; measurement mode (`/scanner/measurement`, with the `scanner:` rake tasks and `script/scanner/` for the findings) is development-only (`config.x.scanner_measurement`; `COLLECTOR_SCANNER_MANIFEST`, `COLLECTOR_SCANNER_RUN_DIR`, `COLLECTOR_SCANNER_KEEP_FRAMES=1` keeps live frames) and writes outside the repo; the spikes are in `spikes/card_scanner/`. Phones need HTTPS: `bin/dev-certificate` makes a self-signed certificate outside the repo and prints the `bin/dev -b "ssl://…"` bind (alternative: a tunnel with `RAILS_DEVELOPMENT_HOSTS` + `COLLECTOR_HTTPS=true`). - **Export format and import (specs 017, 022, 025, 028, 032, 062, 068; ADRs 0020–0023, 0025, 0028):** the current rules are in [`docs/architecture/data-portability.md`](docs/architecture/data-portability.md); the format is defined by `docs/export-format/README.md`. Version 4 is current (`ExportFormat::Rules::VERSIONS` is `[1, 2, 3, 4]`); a new table with `account_id` or `user_id`, or a new column on `accounts`, `lots`, `users` or `locations`, fails the suite until the document's Coverage tables list it; a new field of collector-owned data raises the version. - **Import preview and apply (specs 025, 028):** area doc above. The stored upload's retention and sweep (`ImportUpload`, `ImportUpload::SweepJob`, ADR 0022) and the 40 MB request body caps (Puma, `MAX_REQUEST_BODY` in the `Dockerfile`, `config/deploy.yml`) are in [`docs/architecture/release-and-deploy.md`](docs/architecture/release-and-deploy.md). `Location.upsert_all` and `CollectionImport::Apply#upsert_lots!` (a bound multi-row `INSERT .. ON CONFLICT`) skip validations on purpose (the reader already checked each row). - **Collection and lots (specs 004, 006, 021, 026, 029, 032–034, 041, 059, 062, 064–068; ADRs 0019, 0025, 0028):** a lot's identity (printing, finish, condition, price paid and place, `lot_key`), the 9,999 cap, merging, Move copies, the grid and table, bulk mode and its Undo, number entry, locations and their names, and decks, their zones and the label "Deck › Zone" are in [`docs/architecture/collection-and-lots.md`](docs/architecture/collection-and-lots.md); how the export, preview and apply cover lots, locations and decks is in [`docs/architecture/data-portability.md`](docs/architecture/data-portability.md). Guards an agent trips first: `Location::Name` is the one definition of a name (`spec/models/location/name_parity_spec.rb`); five places build a `lot_key` and all must carry the place; `Account` declares `lots` before `locations` because the restrict foreign key needs lots deleted first (`spec/models/account_spec.rb`); a location has a kind, so a list says which one it means (every picker and the collection filter offer places and zones through `Location::Choices`, and a write names its location through `Location.find_placeable!`; the Locations page and the export stay places only; [`collection-and-lots.md`](docs/architecture/collection-and-lots.md)), and the deck files (the deck page's `Catalog::DeckCards`, `Catalog::DeckShape` and `Catalog::DeckChecks` seams among them) name no collectible, format or zone key but `considering` (`spec/deck_core_spec.rb`); number entry and the location files name no collectible type (`spec/number_entry_core_spec.rb`, `spec/import_apply_core_spec.rb`). diff --git a/app/assets/stylesheets/collector/additions.css b/app/assets/stylesheets/collector/additions.css index d1858f82..a6153fb8 100644 --- a/app/assets/stylesheets/collector/additions.css +++ b/app/assets/stylesheets/collector/additions.css @@ -262,21 +262,11 @@ img.c-table__thumb { height:auto; object-fit:contain; } /* the export stretches: .c-scanner__photo { min-height:44px; min-width:44px; } .c-scanner__photo:focus-within { outline:2px solid var(--focus); outline-offset:2px; } .c-scanner__failure { display:flex; flex-direction:column; align-items:flex-start; gap:var(--space-2); } -.c-scanner__read { display:grid; grid-template-columns:max-content 1fr; gap:var(--space-1) var(--space-3); margin:0; - font:400 14px/20px var(--font-sans); } -.c-scanner__read dt { color:var(--ink-muted); } -.c-scanner__read dd { margin:0; white-space:pre-wrap; overflow-wrap:anywhere; } -.c-scanner__mono { font-family:var(--font-mono); } -.c-scanner__result { display:flex; flex-direction:column; gap:var(--space-6); } @media (prefers-reduced-motion: reduce) { .c-scanner * { transition:none; animation:none; } } -/* Scanner confirm flow (spec 009): evidence on a candidate, add buttons per finish, Other printings and the sitting. */ -.c-scanner__candidate { display:flex; flex-direction:column; align-items:flex-start; gap:var(--space-2); } -.c-scanner__evidence { font:400 13px/18px var(--font-sans); color:var(--ink-muted); } -.c-scanner__result .c-status__message + .c-grid { margin-top:var(--space-3); } /* the art overrule note above the candidates (spec 011) */ -.c-scanner__adds { display:flex; flex-wrap:wrap; gap:var(--space-2); } -.c-scanner__add, .c-scanner__undo { margin:0; } -.c-scanner__add .c-btn, .c-scanner__undo .c-btn, .c-scanner__sitting .c-btn { min-height:44px; min-width:44px; } +/* Scanner confirm flow (spec 009): the sitting's list and its Undo (the candidates grid, evidence, read-text list and printings frame went with spec 088). */ +.c-scanner__undo { margin:0; } +.c-scanner__undo .c-btn, .c-scanner__sitting .c-btn { min-height:44px; min-width:44px; } .c-scanner__sitting .c-list li { flex-wrap:wrap; } .c-scanner__sitting .c-section__title { min-width:0; white-space:normal; overflow-wrap:anywhere; } /* "…, in " wraps (spec 040 AC-7.1) */ .c-scanner__sitting .c-section__title:focus-visible { outline:2px solid var(--focus); outline-offset:2px; border-radius:var(--radius-sm); } /* the heading takes focus after an Undo (spec 047) */ @@ -284,8 +274,6 @@ img.c-table__thumb { height:auto; object-fit:contain; } /* the export stretches: .c-scanner__entry-actions { display:flex; gap:var(--space-2); font-family:var(--font-sans); } .c-scanner__more > summary { list-style:none; width:max-content; margin-top:var(--space-2); } .c-scanner__more > summary::-webkit-details-marker { display:none; } -.c-scanner__printings .c-list li { flex-wrap:wrap; } -.c-scanner__printings .c-list__end { font-family:var(--font-sans); } .c-scanner__hint { margin:0; font:400 14px/20px var(--font-sans); color:var(--ink-muted); } /* Art matching's status under the scanner's controls (spec 011 AC-5.1): quiet, like the hint. */ .c-scanner__art { margin:0; font:400 14px/20px var(--font-sans); color:var(--ink-muted); } @@ -841,10 +829,11 @@ body:has(.c-locations) .c-appbar__back { min-height:var(--touch-floor); min-widt /* `c-scan--result` on the shell shows both. The strip is the frozen capture, shrunk, with what was read beside it: 144px high with a 124px picture, 96px with a 76px one in a shell less than 720px high (the container query at the end), and never taller, so the name read is one line. The pane fills the rest of the stage. Its band never scrolls; its body is a - grid of two rows. The first is the image row, 160px to 288px of image and its own 16px above and 12px below; the second - is the centred card's panel and takes what is left, its add buttons at its foot, so spare room goes above them. When the - two do not fit (a short screen, a wrapped band or set name) the images have already given way to 160px and the body - scrolls inside the pane. The sums are in spec 080 decision 7. + grid of two rows. The first is the image row, `--scan-image-min` to 288px of image (the floor is read from the variable, + 160px without one; spec 088 sets it, below) and its own 16px above and 12px below; the second is the centred card's panel + and takes what is left, its add buttons at its foot, so spare room goes above them. When the two do not fit (a short + screen, a wrapped band or set name) the images have already given way to the floor and the body scrolls inside the pane. + The sums were spec 080 decision 7's; spec 088 decision 13 supersedes its 160px floor and 76px picture. The row is a size container, so an image's width is known from the row's height: the inline padding is half the row less half an image, which lets the first and the last image be centred. The row draws no scrollbar (Previous and Next are the way to move it without a swipe, and a classic scrollbar would take 12px to 15px of the pane), snaps each image to the @@ -870,7 +859,7 @@ body:has(.c-locations) .c-appbar__back { min-height:var(--touch-floor); min-widt @keyframes c-scanpane-rise { from { transform:translateY(100%); } } .c-scanpane > .c-band { flex:none; } .c-scanpane__body { flex:1 1 auto; min-height:0; display:grid; overflow-y:auto; overscroll-behavior:contain; - grid-template-rows:minmax(calc(160px + var(--space-4) + var(--space-3)), calc(288px + var(--space-4) + var(--space-3))) 1fr; } + grid-template-rows:minmax(calc(var(--scan-image-min, 160px) + var(--space-4) + var(--space-3)), calc(288px + var(--space-4) + var(--space-3))) 1fr; } .c-scanpane__rowwrap { container-type:size; position:relative; min-height:0; } .c-scanpane__row { box-sizing:border-box; display:flex; gap:var(--space-3); height:100%; margin:0; list-style:none; padding:var(--space-4) calc(50cqw - (100cqh - var(--space-4) - var(--space-3)) * 63 / 176) var(--space-3); @@ -909,9 +898,8 @@ body:has(.c-locations) .c-appbar__back { min-height:var(--touch-floor); min-widt element an answer replaces, laid out as the top bar's own children. The bars grow by the safe-area insets of a phone with a notch or home indicator (decision 18). The frozen frame is the captured frame on a canvas, cropped as the video is, so the collector sees what was read while it is read (decision 11). A stage notice is centred on the stage (the camera or the - engine unavailable, no script); the catalog-not-loaded line sits at the stage's foot (decision 14). Today's result sits in an - interim scrolling body until #345 replaces it (decision 11, `c-scanpane__results`). Each live region stays rendered while - empty, taking no space (spec 080 decision 8). From 640px the shell is a centred 480px column and the scanner's sheets sit under it (decision + engine unavailable, no script); the catalog-not-loaded line sits at the stage's foot (decision 14). A result sits in the + selection pane (spec 088). Each live region stays rendered while empty, taking no space (spec 080 decision 8). From 640px the shell is a centred 480px column and the scanner's sheets sit under it (decision 17), by a rule scoped to the scanner, so the sheet's own rule is unchanged. */ .c-scan__sitting { display:contents; } .c-scan__top { box-sizing:border-box; height:calc(56px + env(safe-area-inset-top)); padding:env(safe-area-inset-top) calc(var(--space-2) + env(safe-area-inset-right)) 0 calc(var(--space-2) + env(safe-area-inset-left)); } @@ -928,14 +916,11 @@ body:has(.c-locations) .c-appbar__back { min-height:var(--touch-floor); min-widt .c-scan__notice .c-btn { box-sizing:border-box; min-height:44px; min-width:44px; } .c-scan__notice .c-sheet__foot { align-self:stretch; text-align:left; } .c-scan__catalog { box-sizing:border-box; position:absolute; z-index:1; left:var(--space-4); right:var(--space-4); bottom:var(--space-4); } -.c-scanpane__results { box-sizing:border-box; flex:1 1 auto; min-height:0; display:flex; flex-direction:column; gap:var(--space-4); - overflow-y:auto; overscroll-behavior:contain; padding:var(--space-4); } .c-scan__sheet-status { margin-bottom:var(--space-4); } /* An empty live region stays rendered, so a sentence written into it is announced (spec 080 decision 8): `display:none` removes it from the accessibility tree, and one shown in the same moment it is filled is not read out. It takes no space instead: a - sheet's loses its margin, and the pane's, an item of the results' flex column, takes back the column's gap. */ + sheet's loses its margin (the pane's `#status` has none to lose, spec 088). */ .c-scan__sheet-status:empty { margin:0; } -.c-scan__pane-status:empty { margin-bottom:calc(-1 * var(--space-4)); } .c-scan__photo { display:flex; flex-direction:column; align-items:flex-start; gap:var(--space-2); margin-top:var(--space-4); } .c-scan .c-sheet .c-btn { box-sizing:border-box; min-height:44px; min-width:44px; } .c-scan .c-sheet__title:focus { outline:2px solid var(--focus); outline-offset:2px; border-radius:var(--radius-sm); } /* programmatic focus after an Undo (spec 047); Firefox gives it no :focus-visible once the focused Undo is replaced */ @@ -951,6 +936,39 @@ body:has(.c-locations) .c-appbar__back { min-height:var(--touch-floor); min-widt .c-scan__run { box-sizing:border-box; flex:none; min-height:44px; max-width:40%; overflow:hidden; text-overflow:ellipsis; white-space:nowrap; } .c-scan__framing { display:flex; flex-wrap:wrap; gap:var(--space-2); margin-top:var(--space-4); } +/* ---------- Scanner selection pane (spec 088): the band's empty state, the selection frame, the layout numbers ---------- */ +/* The band and the pane's `#status` stay rendered while empty and take no space, so a sentence written into them is announced + (spec 080 decision 8); `#status` has no padding, margin or gap of its own, and the spacing is on the message inside it. The result + container is a flex column that takes the pane's remaining height, so the selection frame (a grid by its class) sizes as spec 080's + body did. */ +.c-band:empty { min-height:0; padding:0; } +.c-scanpane__result { flex:1 1 auto; min-height:0; display:flex; flex-direction:column; } +.c-scanpane__result > .c-scanpane__body { flex:1 1 auto; } +.c-scan__pane-status .c-status__message { margin:var(--space-3) var(--space-4); } +.c-scanpane > .c-scanner__failure { padding:0 var(--space-4) var(--space-3); } +.c-scanpane__missing { box-sizing:border-box; width:100%; height:100%; display:flex; flex-direction:column; align-items:center; justify-content:center; gap:var(--space-1); + padding:var(--space-2); text-align:center; font:600 13px/18px var(--font-sans); color:var(--ink-muted); overflow-wrap:anywhere; } +.c-scanpane__missing span { font:400 13px/18px var(--font-mono); } +.c-scanpane__meta .c-btn { box-sizing:border-box; flex:none; min-height:44px; min-width:44px; } +/* A possible-cards row (spec 088 decision 3): the line holds the tag, the position and the 44px "All printings", which do not fit + on one line at 360px with the year, so the year is left out and the position wraps to two lines, right-aligned, inside the 44px. */ +.c-scanpane__meta--cards .c-scanpane__position { min-width:0; white-space:normal; text-align:right; } + +/* The layout numbers of decision 13, here and nowhere else (the examples that prove them are in spec/system/scanner_selection_spec.rb + and design_system_scanner_components_spec.rb): the images' floor is 96px, not spec 080's 160px, so a not-sure pane (a two-line + band and the 44px "All printings" line) fits a 360 x 640 screen without scrolling and a confident one shows 112px images at + 360 x 560; and in a shell under 600px high (a 360 x 560 screen) the strip is 64px with a 52px picture, after spec 080's 719px + step above (96px with a 76px picture, which this block supersedes under 600px). */ +.c-scanpane { --scan-image-min:96px; } +@container scan (max-height: 599px) { + .c-scan__strip { box-sizing:border-box; height:64px; } + .c-scan__capture { box-sizing:border-box; height:52px; } + /* The label's 16px + 4px gap and the name, the gap and the line (22 + 4 + 18) would fill the 64px strip exactly, the label on its + top edge and the line on the band. Visually hidden, as `.c-sr` is (not `display:none`: assistive technology still reads "Read + from the card"), the name and the line are 44px, centred, 10px above and below. At 144px and 96px the label shows. */ + .c-scan__read-label { position:absolute; width:1px; height:1px; overflow:hidden; clip:rect(0 0 0 0); white-space:nowrap; } +} + /* ---------- Reduced motion, app-wide (spec 080 decision 14) ---------- */ /* The README's Motion rule: under `prefers-reduced-motion: reduce` nothing eases or slides, in any component. A universal selector loses to any class rule (the export's `.c-btn` and `.c-tile__media` set a transition), so these two declarations diff --git a/app/controllers/scanner/printings_controller.rb b/app/controllers/scanner/printings_controller.rb index 20697fb9..27c10f73 100644 --- a/app/controllers/scanner/printings_controller.rb +++ b/app/controllers/scanner/printings_controller.rb @@ -1,13 +1,21 @@ -# "Other printings" for a scanned card (spec 009 Story 2): its printings in a Turbo Frame inside the reading, each with add -# buttons for the same reading, so the scanner and its camera stay on the page. +# The selection frame for one card (spec 088 decisions 3, 4, 15; reworks spec 009 Story 2's Other printings): its printings with +# one first, 20 at a time, for "All printings" from a possible-cards row and "Show 20 more". The same partial the reading's answer +# renders, so the camera and the scanner stay on the page. Params: card (the identity's external key), key (the reading key), +# set, number, finish_hint, artwork (as the reading's candidates carry them), match (the printing that goes first), band (its +# band state, a closed list), limit and current (the printing to centre). class Scanner::PrintingsController < ApplicationController def index - @key = params.expect(:key) - return head(:not_found) unless Scanner::Sitting::KEY_FORMAT.match?(@key) + key = params.expect(:key) + return head(:not_found) unless Scanner::Sitting::KEY_FORMAT.match?(key) - @identity = Catalog::Identity.where(collectible_type: MTG::Reading::COLLECTIBLE_TYPE).find_by!(external_key: params.expect(:card)) - @printings = Scanner::OtherPrintings.new(identity: @identity, set_code: params[:set], number: params[:number], - artwork: params[:artwork].to_s[MTG::Art::Sent::ID]) # an invalid or unknown id is ignored (spec 011 AC-7.5) - @finish_hint = params[:finish_hint].presence + identity = Catalog::Identity.where(collectible_type: MTG::Reading::COLLECTIBLE_TYPE).find_by!(external_key: params.expect(:card)) + artwork = params[:artwork].to_s[MTG::Art::Sent::ID] # an invalid or unknown id is ignored (spec 011 AC-7.5) + match = identity.entries.includes(:set).find_by(external_key: params[:match].to_s) || + Scanner::OtherPrintings.new(identity:, set_code: params[:set], number: params[:number], artwork:).entries.first + return head(:not_found) unless match + + Catalog::Entry.preload_extensions([ match ]) + @selection = Scanner::Selection.for_card(identity:, match:, state: params[:band], key:, set: params[:set].to_s.presence, number: params[:number].to_s.presence, + finish_hint: params[:finish_hint].to_s.presence, artwork:, limit: params[:limit], current: params[:current].to_s.presence) end end diff --git a/app/controllers/scanner/readings_controller.rb b/app/controllers/scanner/readings_controller.rb index 2ebced24..3e622e0a 100644 --- a/app/controllers/scanner/readings_controller.rb +++ b/app/controllers/scanner/readings_controller.rb @@ -1,9 +1,13 @@ -# Turns what was read off a card into what the page shows (spec 007 Story 3): the candidates, each with an add button per -# finish for this reading (spec 009 Story 1). Only the text, the page's reading key and a live capture's nearest artworks -# (ids and distances, spec 011 FR-5) arrive here; the photo and its fingerprint never leave the device. +# Turns what was read off a card into what the page shows (spec 007 Story 3, spec 088 decisions 2 to 4, 11): a selection of the +# card's printings or of the candidate cards, or a sentence on the status line when nothing could be read or no card matches. +# Only the text, the page's reading key and a live capture's nearest artworks (ids and distances, spec 011 FR-5) arrive here; the +# photo and its fingerprint never leave the device. class Scanner::ReadingsController < ApplicationController TOO_LONG = "That reading was too long to use. Line the card up with the guide and capture it again.".freeze NO_KEY = "That reading couldn't be used. Capture the card again.".freeze + NOTHING_READ = "Nothing could be read. Line the card up with the guide and capture it again.".freeze + NO_MATCH = "No card matches what was read. Line the card up with the guide and capture it again.".freeze + CATALOG_NOT_READY = "The card catalog isn't ready yet. Once it has loaded, scan the card again.".freeze def create attributes = params.expect(reading: %i[name_text collector_text key]) @@ -12,13 +16,24 @@ def create key = attributes[:key].to_s if !Scanner::Sitting::KEY_FORMAT.match?(key) then refuse(NO_KEY) elsif reading.valid? - render turbo_stream: turbo_stream.update("scanner_result", partial: "scanners/result", locals: { reading: reading.resolve, key: }) + render turbo_stream: answer(reading.resolve, key) record_measurement(key, reading) else refuse(TOO_LONG) end end private + # The streams that answer a resolved reading. A refusal that opens the pane is a visible alert in its region and leaves + # `#scanner_result` empty; nothing read or no match is the status line's sentence and opens no pane (spec 088 decision 11). + def answer(reading, key) + if !reading.catalog_ready? then [ alert_in_pane(CATALOG_NOT_READY) ] + elsif reading.candidates.empty? + [ turbo_stream.update("scanner_status_text", ERB::Util.html_escape(reading.nothing_read? ? NOTHING_READ : NO_MATCH)) ] + else + [ turbo_stream.update("scanner_result", partial: "scanners/selection", locals: { selection: Scanner::Selection.for_reading(reading, key:) }) ] + end + end + # Development measurement mode only (spec 011 AC-9.3): never fails or changes the answer. def record_measurement(key, reading) Scanner::MeasurementRun.current(framing: Scanner::MeasurementRun.framing_in(session))&.record_reading!(key, reading) @@ -26,8 +41,11 @@ def record_measurement(key, reading) Rails.logger.warn("scanner.measurement reading not recorded: #{error.class}: #{error.message}") end + def alert_in_pane(message) + turbo_stream.update("status", partial: "shared/status_message", locals: { message:, alert: true }) + end + def refuse(message) - render turbo_stream: turbo_stream.update("scanner_result", partial: "shared/status_message", locals: { message:, alert: true }), - status: :unprocessable_content + render turbo_stream: alert_in_pane(message), status: :unprocessable_content end end diff --git a/app/helpers/scanners_helper.rb b/app/helpers/scanners_helper.rb index 54f4a94c..6e6a4fee 100644 --- a/app/helpers/scanners_helper.rb +++ b/app/helpers/scanners_helper.rb @@ -1,20 +1,27 @@ module ScannersHelper - COLLECTOR_OUTCOMES = { one: "Matched one printing", none: "No printing", several: "Several printings", unread: "Not read" }.freeze - - def collector_outcome(reading) = COLLECTOR_OUTCOMES.fetch(reading.collector_status) - - ART_OUTCOMES = { matched: "Matched", similar: "Looks similar", no_match: "No match" }.freeze - - # The Artwork row of "What the scanner read" (spec 011 AC-7.1): nil when no artworks were sent. - def scanner_art_outcome(reading) = reading.art_status && ART_OUTCOMES.fetch(reading.art_status) + # The visually hidden tail of a band sentence, naming the centred card so the band's text changes with every swipe and is + # announced each time (spec 088 decision 8): "Printing 2 of 14: Lightning Bolt, Double Masters 2022, 2X2 · 117." The more item + # (index past the items) says what it offers instead. + # + # @param selection [Scanner::Selection] + # @param index [Integer] the item's place in the row + # @return [String] + def selection_tail(selection, index) + return "#{pluralize(selection.more, "more printing")}. Show 20 more." if index >= selection.items.size - # One sentence when confident art overruled what the text suggested (spec 011 AC-7.4). - def scanner_overrule_note(reading) - return unless reading.overruled + entry = selection.items[index].entry + noun = selection.kind == :cards ? "Card" : "Printing" + "#{noun} #{index + 1} of #{selection.total}: #{entry.display_name}, #{entry.set.name}, #{set_number(entry)}." + end - what = reading.overruled_scope == :card ? "card" : "printing" - by = reading.overruled == :collector_line ? "collector line" : "name" - "The artwork matches a different #{what} from the one the #{by} suggests. The artwork's match is first." + # "2 of 14 printings", "1 of 3 possible cards": the details block's position (spec 088, design decision 15). + # + # @param selection [Scanner::Selection] + # @param index [Integer] the item's place in the row + # @return [String] + def selection_position(selection, index) + noun = selection.kind == :cards ? "possible card" : "printing" + "#{index + 1} of #{pluralize(selection.total, noun)}" end def ocr_engine_path = "/ocr/#{::Collector::OcrEngine::VERSION}" diff --git a/app/javascript/controllers/card_reader_controller.js b/app/javascript/controllers/card_reader_controller.js index 153454fb..06a71700 100644 --- a/app/javascript/controllers/card_reader_controller.js +++ b/app/javascript/controllers/card_reader_controller.js @@ -13,8 +13,10 @@ import { loadArtIndex, matchArtwork } from "scanner/art" // collectorText, ms, key, outline, detectMs, warpMs, artworks, artMs, art, readyMs, strips, frame }) for measurement mode, // where outline is "live", "found" or "not_found". frame is { image, guide } for live captures, else null; sizes is the video's // and the stage's sizes for a live capture, else null. On the one-screen page (spec 082) a capture freezes on its frame (a -// canvas, decision 11), the answer opens the pane, and the main button reads "Scan again" while a result shows (decision 13); a -// sentence about a result is written to the pane's region once the pane shows (decision 10). +// canvas, decision 11), the answer opens the pane, and the main button reads "Scan again" while a result shows (decision 13). +// The capture's box is drawn on the strip beside what was read from it (spec 088 decisions 8, 9); once the pane shows, the +// reader says "Done." and the band's sentence once, as hidden text in the pane's region, and a refusal or the no-edge sentence +// is written there too (spec 082 decision 10, spec 088 decision 11). It never writes the band: the selection controller does. const REASONS = { insecure: "The live camera needs this page to be served over HTTPS. You can use a photo instead.", denied: "The camera is blocked for this site. Allow it in your browser's settings, or use a photo instead.", @@ -23,13 +25,13 @@ const REASONS = { lost: "The camera stopped. Try again, or use a photo instead." } const READY_LIVE = "Ready. Line the card up with the guide, then capture." -const SHOWING = "Done. What the scanner read and its candidates are showing." +const READING = "Reading the card…" const COULD_NOT_READ = "The card couldn't be read. Line it up with the guide and try again." const NO_EDGE = "No card edge was found, so the photo was read as if framed like the guide. For a better reading, photograph the whole card, upright, filling most of the photo, on a plain background." export default class extends Controller { static targets = [ "status", "main", "picker", "result", "unavailable", "reason", "retry", "engineRetry", "engineFailed", - "frozen", "paneStatus", "added", "addedText", "failure", "failureName", "failureCollector", "signIn", "resend", "artStatus" ] + "frozen", "paneStatus", "added", "addedText", "failure", "signIn", "resend", "artStatus", "strip", "capture", "readName", "readLine", "band" ] static values = { readingsUrl: String, enginePath: String, artIndexUrl: String, artSettings: Object } connect() { @@ -166,7 +168,11 @@ export default class extends Controller { // The pane closed (an add cleared the result) or opened: the frozen frame goes with the result, and the main button follows. shellChanged({ detail: { result } }) { - if (!result) this.paneSaid += 1 + if (!result) { + this.paneSaid += 1 + this.readNameTarget.textContent = "" + this.readLineTarget.textContent = "" + } if (!result && !this.busy) this.frozenTarget.hidden = true this.render() } @@ -195,7 +201,7 @@ export default class extends Controller { this.addedTextTarget.textContent = "" this.addedTarget.classList.remove("c-scan__line--alert") // a page-load alert flash styled the line; the next sentence is not one this.render() - this.say("Reading the card…") + this.say(READING) try { const { image, card, layout, ...source } = await prepare() // Spec 011: art runs on live captures only (AC-5.4), before OCR, about 35 ms on the phone. @@ -205,9 +211,11 @@ export default class extends Controller { if (!this.element.isConnected) return // Spec 010: a live capture's frame and guide rect travel with the event, in memory, for measurement mode only. const frame = source.outline === "live" ? { image, guide: card } : null + this.drawCapture(image, card, source.outline) + this.writeRead(reading.nameText, reading.collectorText) this.dispatch("read", { detail: { ...reading, sizes: source.sizes || null, strips, frame, art: this.artTimings || null, readyMs: this.readyMs } }) this.lastReading = reading - if (await this.send(reading) && reading.outline === "not_found") this.paneSay(NO_EDGE) + if (await this.send(reading) && reading.outline === "not_found") this.paneSay(NO_EDGE, { append: true }) } catch { this.frozenTarget.hidden = true this.say(COULD_NOT_READ) @@ -218,13 +226,19 @@ export default class extends Controller { } resend() { - if (this.lastReading) this.send(this.lastReading) + if (!this.lastReading) return + // The old failure alert goes before the new answer: a nothing-read or no-match answer opens no pane to replace it. + this.paneStatusTarget.replaceChildren(...[ ...this.paneStatusTarget.children ].filter((child) => child.hasAttribute("data-pane-keep"))) + this.send(this.lastReading) } // Sends the text, the reading key (spec 009 FR-5) and a live capture's nearest artworks (spec 011 FR-5), never the // outline, timings or a fingerprint; true when the answer was shown. + // + // The failure block (and so the pane) stays shown while a resend is in flight and is hidden in the same render as the answer + // (`hideFailureWith`): hiding it first would empty the shell's result state for the answer's two frames, and `shellChanged` + // would clear the strip's texts and close the pane under an answer that is about to reopen it (spec 088 decisions 9, 11). async send({ nameText, collectorText, key, artworks }) { - this.failureTarget.hidden = true const body = new FormData() body.append("reading[name_text]", nameText) body.append("reading[collector_text]", collectorText) @@ -242,14 +256,67 @@ export default class extends Controller { } if (response.type === "opaqueredirect") return this.failed("Your session has ended. Sign in again, then capture the card again.", { signedOut: true }) if (!response.ok && response.status !== 422) return this.failed("The scanner had a problem with that card. Send it again.", { signedOut: false }) - Turbo.renderStreamMessage(await response.text()) - this.paneSay(SHOWING) + const text = await response.text() + const hideFailure = this.hideFailureWith() + document.addEventListener("turbo:before-stream-render", hideFailure) + try { + Turbo.renderStreamMessage(text) + return await this.opened() + } finally { + document.removeEventListener("turbo:before-stream-render", hideFailure) + this.failureTarget.hidden = true + } + } + + // A `turbo:before-stream-render` listener that wraps the stream's render so the failure block hides in the same task as the + // answer's DOM change: the shell's observer then sees one state, never "no result" in between. + hideFailureWith() { + return (event) => { + const render = event.detail.render + event.detail.render = (stream) => { + this.failureTarget.hidden = true + return render(stream) + } + } + } + + // Waits for the answer to land and says what it did (spec 088 decision 11): a selection opens the pane (the band holds its + // sentence) and says "Done." once, as hidden text; a refusal in the pane is its own alert; "nothing read" or "no match" is the + // status line's sentence, so the frozen frame goes and no pane opens. Turbo applies a stream after a repaint, so wait for the + // pane or for the status line to change, at most about a second; after a failure, for the failure block to hide with the answer. + async opened() { + for (let frame = 0; frame < 60 && (!this.failureTarget.hidden || (!this.resultShowing && this.statusTarget.textContent === READING)); frame++) await nextFrame() + await nextFrame() + if (!this.resultShowing) { + this.frozenTarget.hidden = true + return false + } + const sentence = this.bandTarget.textContent.replace(/\s+/g, " ").trim() + if (!sentence) return false + this.paneSay(`Done. ${sentence}`, { hidden: true }) return true } + // The card's box drawn on the strip's canvas at twice its size, so the strip shows the card and not the whole frame (spec 088 + // decision 9); a live capture's shrink starts from the guide's height over the picture's (spec 080's `--from`). + drawCapture(image, card, outline) { + const canvas = this.captureTarget + canvas.height = 248 + canvas.width = Math.round(248 * 63 / 88) + canvas.getContext("2d").drawImage(image, card.x, card.y, card.width, card.height, 0, 0, canvas.width, canvas.height) + const guide = outline === "live" && this.element.querySelector(".c-scan__guide")?.getBoundingClientRect().height + if (guide) canvas.style.setProperty("--from", (guide / parseFloat(getComputedStyle(canvas).height || 124)).toFixed(2)) + else canvas.style.removeProperty("--from") // a photo has no guide on the stage: spec 080's default + } + + // What the reader read, on one line each (escaped by textContent; "nothing read" for a blank). + writeRead(nameText, collectorText) { + const line = (text) => text.replace(/\s+/g, " ").trim() || "nothing read" + this.readNameTarget.textContent = line(nameText) + this.readLineTarget.textContent = line(collectorText) + } + failed(message, { signedOut }) { - this.failureNameTarget.textContent = this.lastReading?.nameText || "Nothing read" - this.failureCollectorTarget.textContent = this.lastReading?.collectorText || "Nothing read" this.resendTarget.hidden = signedOut this.signInTarget.hidden = !signedOut this.failureTarget.hidden = false @@ -274,16 +341,19 @@ export default class extends Controller { // Writes a sentence to the pane's live region once the pane shows: a region filled while hidden and then shown is not // announced (spec 080 decision 8; spec 082 decision 10). Waits for `c-scan--result` (set by scan-shell from the answer), at // most about two seconds, then one more frame. - // It replaces only its own sentence: a child marked `data-pane-keep` (measurement mode's "capture wasn't stored") stays. + // `hidden` writes it as visually hidden text ("Done." for the screen reader); `append` keeps the paragraphs already there + // (the no-edge sentence follows "Done."), else it replaces its own sentence: a child marked `data-pane-keep` (measurement + // mode's "capture wasn't stored") stays either way. // A sentence for a reading the collector has since discarded ("Scan again", an add, a new reading bump `paneSaid`) is dropped, - // so a late write can never reopen the pane. - async paneSay(message, { alert = false } = {}) { + // so a late write can never reopen the pane, and so is one for a pane that never opened (spec 088 decision 11). + async paneSay(message, { alert = false, hidden = false, append = false } = {}) { const said = this.paneSaid for (let frame = 0; frame < 120 && !this.resultShowing; frame++) await nextFrame() await nextFrame() - if (said !== this.paneSaid) return - const paragraph = Object.assign(document.createElement("p"), { className: `c-status__message${alert ? " c-status__message--alert" : ""}`, textContent: message }) - const kept = [ ...this.paneStatusTarget.children ].filter((child) => child.hasAttribute("data-pane-keep")) + if (said !== this.paneSaid || !this.resultShowing) return + const className = hidden ? "c-sr" : `c-status__message${alert ? " c-status__message--alert" : ""}` + const paragraph = Object.assign(document.createElement("p"), { className, textContent: message }) + const kept = [ ...this.paneStatusTarget.children ].filter((child) => append || child.hasAttribute("data-pane-keep")) this.paneStatusTarget.replaceChildren(...kept, paragraph) } diff --git a/app/javascript/controllers/scanner_reading_controller.js b/app/javascript/controllers/scanner_reading_controller.js index 086f32e5..8c71e54d 100644 --- a/app/javascript/controllers/scanner_reading_controller.js +++ b/app/javascript/controllers/scanner_reading_controller.js @@ -1,65 +1,77 @@ import { Controller } from "@hotwired/stimulus" -// Holds a reading's add buttons while one of its adds is in flight, so one reading adds one copy (spec 009 AC-1.4). The -// answer clears the reading when the add goes through; a refusal hands the buttons back. An add that fails on the network -// or the server keeps the reading and says so, and its button is the retry: the same reading key makes a retry safe -// (AC-1.5, Error Scenarios). Other printings that fail to load say so in place, and opening them is announced (NFR -// Accessibility). +// Holds a reading's add buttons while one of its adds is in flight, so one reading adds one copy (spec 009 AC-1.4). The answer +// clears the reading when the add goes through; a refusal hands the buttons back. An add that fails on the network or the server +// keeps the reading and says so, and its button is the retry: the same reading key makes a retry safe (AC-1.5, Error Scenarios). +// The selection frame's links ("All printings", "Show 20 more") that fail say so in the pane's region and leave the row as it was +// (spec 088 AC-3.11); `remember` notes which link was used, since a failed frame request does not say. const ADD_FAILED = "That card wasn't added. Check your connection, then tap its add button again." -const PRINTINGS_FAILED = "Other printings couldn't be loaded. Tap “Other printings” again." +const PRINTINGS_FAILED = "The card's printings couldn't be loaded. Tap “All printings” again." const SIGN_IN_PATH = "/session/new" +const ADD = ".c-scanpane__add" export default class extends Controller { lock({ target }) { - if (target.matches(".c-scanner__add")) this.buttons.forEach((button) => { button.disabled = true }) + if (target.matches(ADD)) this.buttons.forEach((button) => { button.disabled = true }) } // success is undefined when Turbo cancelled this submission for a newer one, which is still in flight: keep the lock. unlock({ target, detail: { success, error } }) { - if (!target.matches(".c-scanner__add")) return + if (!target.matches(ADD)) return if (error) this.announce(ADD_FAILED, { alert: true }) if (success === false) this.buttons.forEach((button) => { button.disabled = false }) } - // A server error's HTML page would otherwise replace the scanner, camera and all. For Other printings, a server error - // whose body isn't HTML would otherwise leave the frame silently empty. + // The link the collector used, so a failure can name its button. + remember({ currentTarget }) { + this.failureMessage = currentTarget.dataset.failureMessage + } + + // A server error's HTML page would otherwise replace the scanner, camera and all; for the selection frame, one whose body isn't + // HTML would leave the frame silently unchanged. inspect(event) { if (!event.detail.fetchResponse.serverError) return - if (event.target.matches?.(".c-scanner__add")) { + if (event.target.matches?.(ADD)) { event.preventDefault() this.announce(ADD_FAILED, { alert: true }) - } else if (event.target.id === "scanner_printings") { + } else if (event.target.id === "scanner_selection") { event.preventDefault() - this.printingsFailed(event.target) + this.printingsFailed() } } - // An ended session answers Other printings with the sign-in page, which has no frame: show it, as an add would (Error - // Scenarios, AC-3.3). + // An ended session answers the selection frame with the sign-in page, which has no frame: show it, as an add would. printingsMissing(event) { - if (event.target.id !== "scanner_printings") return + if (event.target.id !== "scanner_selection") return const { response, visit } = event.detail event.preventDefault() if (response.redirected && new URL(response.url).pathname === SIGN_IN_PATH) visit(response) - else this.printingsFailed(event.target) + else this.printingsFailed() } printingsErrored({ target }) { - if (target.id === "scanner_printings") this.printingsFailed(target) + if (target.id === "scanner_selection") this.printingsFailed() } - printingsLoaded({ target }) { - const heading = target.id === "scanner_printings" && target.querySelector("h2") - if (heading) this.announce(`${heading.textContent} are below.`) + printingsFailed() { + this.announce(this.failureMessage || PRINTINGS_FAILED, { alert: true, printings: true }) } - printingsFailed(frame) { - frame.replaceChildren(this.message(PRINTINGS_FAILED, { alert: true })) - this.announce(PRINTINGS_FAILED, { alert: true }) + // A retry that loads: the "couldn't be loaded" alert is stale, and only that one (an add's failure alert stays until it is + // answered). The frame event bubbles from the frame, so the action sits on `#scanner_result`. + printingsLoaded({ target }) { + if (target.id === "scanner_selection") document.querySelectorAll("#status [data-printings-failed]").forEach((alert) => alert.remove()) } - announce(text, { alert = false } = {}) { - document.getElementById("status")?.replaceChildren(this.message(text, { alert })) + // Replaces the region's sentence, but not a child marked `data-pane-keep` (measurement mode's "capture wasn't stored" note), + // as `card-reader#paneSay` does. + announce(text, { alert = false, printings = false } = {}) { + const region = document.getElementById("status") + if (!region) return + const kept = [ ...region.children ].filter((child) => child.hasAttribute("data-pane-keep")) + const paragraph = this.message(text, { alert }) + if (printings) paragraph.setAttribute("data-printings-failed", "") + region.replaceChildren(...kept, paragraph) } message(text, { alert }) { @@ -67,6 +79,6 @@ export default class extends Controller { } get buttons() { - return this.element.querySelectorAll(".c-scanner__add button") + return this.element.querySelectorAll(`${ADD} button`) } } diff --git a/app/javascript/controllers/selection_controller.js b/app/javascript/controllers/selection_controller.js new file mode 100644 index 00000000..79996c5b --- /dev/null +++ b/app/javascript/controllers/selection_controller.js @@ -0,0 +1,104 @@ +import { Controller } from "@hotwired/stimulus" + +// The selection pane's one small controller (spec 088 decision 6): it notes which card of the swipe row is centred and shows what +// that means. The server rendered everything else: every item, a panel for each (hidden unless centred) and, inside each panel, a +//