diff --git a/.github/coverage-acks.txt b/.github/coverage-acks.txt index 08111fa9b..395e478a4 100644 --- a/.github/coverage-acks.txt +++ b/.github/coverage-acks.txt @@ -169,3 +169,5 @@ coverage-ack: frontend/tests/QuestionFeed.spec.ts::question feed - confirm_sugge coverage-ack: frontend/tests/QuestionFeed.spec.ts::question feed - confirm_suggestion question type > YES confirms the suggested printing directly, without visiting the grid — the fixed Level 1/2/3 ladder was removed by the #728 de-hardcoding; the grid now coexists with the suggestion on the same page coverage-ack: frontend/tests/QuestionFeed.spec.ts::question feed - confirm_suggestion question type > NOT SURE drops to Level 2's candidate grid without casting a printing vote, but does POST an abstention — the fixed Level 1/2/3 ladder was removed by the #728 de-hardcoding; the grid now coexists with the suggestion on the same page coverage-ack: frontend/tests/QuestionFeed.spec.ts::question feed - confirm_suggestion question type > NO drops to Level 2's candidate grid, excluding the rejected suggestion, without casting a vote — the fixed Level 1/2/3 ladder was removed by the #728 de-hardcoding; the grid now coexists with the suggestion on the same page +coverage-ack: frontend/tests/QuestionFeed.spec.ts::question feed - confirm_suggestion question type > NO on the suggestion collapses its slot (never a selectable tile again) and keeps the remaining candidates selectable on the same page, without casting a vote — restructured in WTC design intent repass (#704); slot collapse behavior updated +coverage-ack: frontend/tests/QuestionFeed.spec.ts::question feed - confirm_suggestion question type > NO on a singleton suggestion (no other candidates) skips the grid entirely and immediately casts the terminal no-match vote — restructured in WTC design intent repass (#704); slot collapse behavior updated diff --git a/docs/features/printing-tags.md b/docs/features/printing-tags.md index 2cb6c5f00..d411068e6 100644 --- a/docs/features/printing-tags.md +++ b/docs/features/printing-tags.md @@ -908,8 +908,9 @@ for history (this doc's own established convention — see the `cardPanel.tsx` b intentionally identical transitions — "an honest skip beats a coerced guess"), but NO additionally records the rejected candidate's identifier client-side (`rejectedCandidateIds` — never NOT SURE, which - is genuine uncertainty, not a rejection) so Level 2 excludes it — see - the no-re-presentation rule below. `identify_printing` items (and + is genuine uncertainty, not a rejection) so Level 2 retains it as a + de-emphasised, re-selectable tile — see the no-re-presentation rule + below. `identify_printing` items (and `confirm_suggestion` items without a `suggestedPrinting`) skip Level 1 entirely. - **Level 2** — the candidate grid. The attribute-chip ring is now an @@ -989,23 +990,36 @@ for history (this doc's own established convention — see the `cardPanel.tsx` b `RetractImplicitVoteRequest` types). - **No-re-presentation rule** (owner-directed fix, was a real live bug: Level 1 "Is it M21 203?" → NO → Level 2 grid containing only M21 203 - again): within a single question item's flow, a candidate the user - has just rejected is never re-presented as a selectable answer at a - later level — each level's display set is candidates minus - already-rejected-this-item. Level 2's grid is computed from - `nonRejectedCandidates` (all candidates minus `rejectedCandidateIds`, - filtered _before_ the attribute-chip filter, so "N hidden by your - tags" doesn't conflate a rejection with a filter), and the singleton - case — rejecting the one and only candidate, or a rejection that - happens to empty the remaining set — skips the grid entirely: the - prompt swaps to a contextual "Got it — not that one. Is it any - official printing at all?" with the rejected candidate shown only as - grayed, non-interactive context (never a button), falling straight - through to the same classified-exit choice (None of these / custom - art / skip) that always rendered below the grid. `rejectedCandidateIds` - is per-item state, reset alongside every other per-question field in - the same fetch effect (see the module's own comment on why that reset - can't be a separate dependency-keyed effect). + again): within a single question item's flow, the suggested candidate + is asked about exactly once, in its own slot, and is never + re-presented as a grid tile while that slot is still asking — + `gridCandidates` keeps it out (`candidate.identifier !== suggestedCandidateId`), so the old asked-twice shape cannot recur. + A candidate the user has explicitly REJECTED at the suggestion slot + is the deliberate exception (issue #748): the slot collapses to a + contextual "Got it — not that one. Is it any official printing at + all?" plus a "You said: not M21 203" context line, and the rejected + candidate STAYS in the grid as a de-emphasised, fully re-selectable + tile — `data-rejected="true"` with a "you said no · tap to + reconsider" note — the recover path for a mis-tap, where tapping the + tile casts it as a real pick. `gridCandidates` is therefore every + candidate with `rejectedCandidateIds.has(id) || id !== suggestedCandidateId`: the rejected set is INCLUDED, not subtracted, + and the grid still runs through the attribute-chip filter separately + (a rejection is a `gridCandidates` decision, chip hiding a + `visibleCandidates` one), so "N hidden by your tags" never conflates + the two. A rejected candidate never joins an illustration cluster — + a cluster renders only one representative tile, which would silently + bury the reconsider path — so it always renders standalone as a + de-emphasised, ungrouped tile. The "none left" state + (`suggestionRejectedWithNoneLeft`) is decided by candidate count, not + grid count — no candidate OTHER than the rejected suggestion, since + the rejected one is now a grid member: in that state the grid is just + the single de-emphasised tile, the question was already resolved by + the terminal vote (next paragraph), and the filter panel and bottom + action row stay hidden while the reason strip carries the flow. + `rejectedCandidateIds` is per-item state, reset alongside every other + per-question field in the same fetch effect (see the module's own + comment on why that reset can't be a separate dependency-keyed + effect). **Singleton "No" now casts the terminal vote immediately** (owner- reported "dedup doesn't work" bug, fixed after this bullet originally diff --git a/frontend/src/features/attributeVoting/ActionButton.tsx b/frontend/src/features/attributeVoting/ActionButton.tsx new file mode 100644 index 000000000..2df0675f9 --- /dev/null +++ b/frontend/src/features/attributeVoting/ActionButton.tsx @@ -0,0 +1,71 @@ +/** + * DESIGN-REPASS Rule 1 (issue #711) - the ONE action-button primitive shared by every answer + * row on the What's That Card surface. All primary decision and action buttons (Yes, No, Not + * Sure, Skip, and the embedded custom action triggers - the tag question's Apply/Not + * applicable, the no-match reason strip's Skip, the artist picker's candidate buttons) must + * share identical sizing, padding, corner radii, and typography metrics, so no answer row + * reads at a different scale than its siblings. + * + * This is the same geometry as QuestionFeed.tsx's own `Btn` (SPEC-wtc-rebuild.md section 1c's + * `.btn` row: min-height 44px, font 15px/600, pad 6px 16px, `--r-btn` radius, 1px border) plus + * Rule 2's `touch-action: manipulation` (kills the mobile double-tap-zoom gesture that + * otherwise swallows a fast single tap). It exists as a separate primitive rather than an + * import of `Btn` so the attribute-voting funnel components that render inside the question + * feed (and in the card-detail modal, their other caller) share one uniform geometry without + * the questionFeed -> attributeVoting import direction flipping. + * + * Variants mirror the surface's token palette; `w-100` stays available as a plain Bootstrap + * utility class when a caller needs a full-cell button (the artist picker's grid cells). + */ + +import styled from "@emotion/styled"; + +export const ActionButton = styled.button` + min-height: 44px; + display: inline-flex; + align-items: center; + justify-content: center; + gap: 8px; + font: inherit; + font-size: 15px; + font-weight: 600; + padding: 6px 16px; + border-radius: var(--r-btn); + border: 1px solid transparent; + cursor: pointer; + line-height: 1.2; + text-align: center; + /* DESIGN-REPASS Rule 2 (#715) - opt out of the mobile double-tap-zoom gesture. */ + touch-action: manipulation; + + &:disabled { + opacity: 0.6; + cursor: default; + } + + &.primary { + background: var(--primary); + color: var(--btn-ink); + border-color: var(--primary); + } + + &.secondary { + background: var(--raised); + color: var(--text); + border-color: var(--divider); + } + + &.ghost { + background: transparent; + color: var(--muted); + border-color: transparent; + } + + /* The artist picker's consensus highlight (kept as its own variant so the "this is the + current consensus" signal survives the geometry unification) - success-green fill. */ + &.success { + background: var(--success); + color: var(--btn-ink); + border-color: var(--success); + } +`; diff --git a/frontend/src/features/attributeVoting/ArtistVotePicker.tsx b/frontend/src/features/attributeVoting/ArtistVotePicker.tsx index 4185ec364..bdb47c66b 100644 --- a/frontend/src/features/attributeVoting/ArtistVotePicker.tsx +++ b/frontend/src/features/attributeVoting/ArtistVotePicker.tsx @@ -6,8 +6,7 @@ * thumbnail image) rather than thumbnail buttons. */ -import React, { useEffect, useState } from "react"; -import Button from "react-bootstrap/Button"; +import React, { useEffect, useRef, useState } from "react"; import Col from "react-bootstrap/Col"; import Form from "react-bootstrap/Form"; import Row from "react-bootstrap/Row"; @@ -19,6 +18,7 @@ import { CanonicalArtist, } from "@/common/schema_types"; import { useAppDispatch } from "@/common/types"; +import { ActionButton } from "@/features/attributeVoting/ActionButton"; import { APIGetArtistCandidates, APIGetArtistConsensus, @@ -77,6 +77,10 @@ export function ArtistVotePicker({ const [loading, setLoading] = useState(false); const [submitting, setSubmitting] = useState(false); const [revealPickerAnyway, setRevealPickerAnyway] = useState(false); + // Issue #715 - same synchronous in-flight guard as QueueTagQuestion: `disabled={submitting}` + // lags a fast double-tap by a render, so the ref drops the second entry before it can cast + // the artist vote twice. + const inFlightRef = useRef(false); useEffect(() => { APIGetArtistConsensus(backendURL, cardIdentifier) @@ -99,6 +103,10 @@ export function ArtistVotePicker({ }, [backendURL, cardIdentifier, query]); const submit = (artistName: string | undefined, isUnknown: boolean) => { + if (inFlightRef.current) { + return; + } + inFlightRef.current = true; setSubmitting(true); APISubmitArtistVote( backendURL, @@ -140,7 +148,10 @@ export function ArtistVotePicker({ ]) ); }) - .finally(() => setSubmitting(false)); + .finally(() => { + inFlightRef.current = false; + setSubmitting(false); + }); }; if (confidentlyKnownArtistName != null && !revealPickerAnyway) { @@ -191,29 +202,29 @@ export function ArtistVotePicker({ ) : ( - + {candidates.map((candidate) => ( - + ))} diff --git a/frontend/src/features/attributeVoting/NoMatchReasonStrip.tsx b/frontend/src/features/attributeVoting/NoMatchReasonStrip.tsx index 082ff85c9..d7bf9168b 100644 --- a/frontend/src/features/attributeVoting/NoMatchReasonStrip.tsx +++ b/frontend/src/features/attributeVoting/NoMatchReasonStrip.tsx @@ -37,8 +37,7 @@ * not an empty header. */ -import React, { useState } from "react"; -import Button from "react-bootstrap/Button"; +import React, { useRef, useState } from "react"; import Col from "react-bootstrap/Col"; import Row from "react-bootstrap/Row"; @@ -46,6 +45,7 @@ import { errorToNotification, isRateLimited } from "@/common/apiErrors"; import { getOrCreateAnonymousId } from "@/common/cookies"; import { useTagDisplayName } from "@/common/tagDisplayNames"; import { useAppDispatch } from "@/common/types"; +import { ActionButton } from "@/features/attributeVoting/ActionButton"; import { ChipCard } from "@/features/attributeVoting/ChipCard"; import { APISubmitTagVote, useGetTagsQuery } from "@/store/api"; import { setNotification } from "@/store/slices/toastsSlice"; @@ -119,6 +119,10 @@ export function NoMatchReasonStrip({ const [submittingTagName, setSubmittingTagName] = useState( null ); + // Issue #715 - same synchronous in-flight guard as the other funnel components: the visual + // `disabled` lags a fast double-tap by a render, so the ref drops the second chip tap (and + // the second Skip) before a vote can be cast twice. + const inFlightRef = useRef(false); const { data: existingTags } = useGetTagsQuery(); const existingTagNames = existingTags != null ? new Set(existingTags.map((tag) => tag.name)) : null; @@ -126,6 +130,10 @@ export function NoMatchReasonStrip({ existingTagNames == null || existingTagNames.has(tagName); const choose = (tagName: string) => { + if (inFlightRef.current) { + return; + } + inFlightRef.current = true; setSubmittingTagName(tagName); APISubmitTagVote( backendURL, @@ -153,7 +161,10 @@ export function NoMatchReasonStrip({ ]) ); }) - .finally(() => setSubmittingTagName(null)); + .finally(() => { + inFlightRef.current = false; + setSubmittingTagName(null); + }); }; return ( @@ -196,14 +207,20 @@ export function NoMatchReasonStrip({ ); })}
- +
); diff --git a/frontend/src/features/attributeVoting/QueueTagQuestion.tsx b/frontend/src/features/attributeVoting/QueueTagQuestion.tsx index 95a10add4..30d669479 100644 --- a/frontend/src/features/attributeVoting/QueueTagQuestion.tsx +++ b/frontend/src/features/attributeVoting/QueueTagQuestion.tsx @@ -6,13 +6,13 @@ * applicable, or skip. Submits via the same APISubmitTagVote used by TagVotePicker. */ -import React, { useState } from "react"; -import Button from "react-bootstrap/Button"; +import React, { useRef, useState } from "react"; import { errorToNotification, isRateLimited } from "@/common/apiErrors"; import { getOrCreateAnonymousId } from "@/common/cookies"; import { useTagDisplayName } from "@/common/tagDisplayNames"; import { useAppDispatch } from "@/common/types"; +import { ActionButton } from "@/features/attributeVoting/ActionButton"; import { APISubmitTagVote } from "@/store/api"; import { setNotification } from "@/store/slices/toastsSlice"; @@ -48,8 +48,17 @@ export function QueueTagQuestion({ const dispatch = useAppDispatch(); const getTagDisplayName = useTagDisplayName(); const [submitting, setSubmitting] = useState(false); + // Issue #715 - `disabled={submitting}` only applies on the re-render React batches AFTER the + // current handler returns, so a fast double-tap could cast the vote twice; this ref is set + // synchronously at handler entry and drops the second entry (Skip included - a double-tap on + // Skip must not advance two cards). + const inFlightRef = useRef(false); const submit = (polarity: number) => { + if (inFlightRef.current) { + return; + } + inFlightRef.current = true; setSubmitting(true); APISubmitTagVote( backendURL, @@ -77,7 +86,10 @@ export function QueueTagQuestion({ ]) ); }) - .finally(() => setSubmitting(false)); + .finally(() => { + inFlightRef.current = false; + setSubmitting(false); + }); }; return ( @@ -86,27 +98,33 @@ export function QueueTagQuestion({ Does {getTagDisplayName(tagName)} apply?
- - - +
); diff --git a/frontend/src/features/printingTags/cardPanel.tsx b/frontend/src/features/printingTags/cardPanel.tsx index 23946bfa4..5e169c5ed 100644 --- a/frontend/src/features/printingTags/cardPanel.tsx +++ b/frontend/src/features/printingTags/cardPanel.tsx @@ -301,6 +301,22 @@ export const CandidateButton = styled.button` outline-offset: -1px; border-color: var(--accent, #bb9af7); } + + /* Issue #748 - a Level 1 suggestion the user rejected stays reachable in the Level 2 grid + as a de-emphasised tile (dimmed, dashed outline) instead of vanishing; hovering restores + full opacity so it still reads as selectable - tapping it re-casts the candidate as a + legitimate pick, the reconsider path. QuestionFeed.tsx adds 'data-rejected' on the same + tiles; this class is the visual half of that same marker. */ + &.rejected { + opacity: 0.55; + outline: 1px dashed var(--muted, #8c94bf); + outline-offset: -1px; + + &:hover { + opacity: 1; + outline: 1px solid var(--divider, #16161e); + } + } `; // The spec's ".ccap" candidate caption (SPEC-wtc-rebuild.md section 1c "candidate caption" @@ -323,6 +339,17 @@ export const CandidateCaption = styled.div` font-family: "Courier New", monospace; font-size: 10px; } + + /* Issue #748 - the "you said no · tap to reconsider" note on a rejected Level 1 suggestion + that stays in the grid as a de-emphasised, re-selectable tile (QuestionFeed.tsx renders + it as a .rej caption div with its own testid). Muted italic, one step quieter than the + set/collector line above it, so the tile still reads as a candidate first and a + reconsideration affordance second. */ + .rej { + color: var(--muted); + font-size: 10px; + font-style: italic; + } `; // The spec's ".candgrid" (SPEC-wtc-rebuild.md section 1c "candidate grid" row + section 3's diff --git a/frontend/src/features/questionFeed/QuestionFeed.test.tsx b/frontend/src/features/questionFeed/QuestionFeed.test.tsx index 94c3f8db8..ec961f054 100644 --- a/frontend/src/features/questionFeed/QuestionFeed.test.tsx +++ b/frontend/src/features/questionFeed/QuestionFeed.test.tsx @@ -421,6 +421,77 @@ describe("QuestionFeed", () => { ).toHaveTextContent("Suggested match"); }); + it("a rejected Level 1 suggestion stays reachable in the candidate grid as a de-emphasised, re-selectable tile (issue #748)", async () => { + // #748 - rejecting the suggested printing must not drop it from the surface: the + // suggestion slot collapses into the "you said not this one" context, and the candidate + // grid gains the rejected candidate as a de-emphasised (`data-rejected`) tile that stays + // fully selectable - tapping it re-casts the candidate as a real pick, the reconsider + // path for a mis-tapped "No, different printing". + const confirmSuggestionItem = { + ...identifyPrintingItem, + type: "confirm_suggestion", + suggestedPrinting: identifyPrintingItem.candidates[0], + }; + server.use( + http.get(buildRoute("2/questionFeed/"), () => + HttpResponse.json( + { + item: confirmSuggestionItem, + remainingEstimate: { + total: 1, + confirmable: 1, + contested: 0, + fresh: 0, + }, + }, + { status: 200 } + ) + ) + ); + let submittedIdentifier: string | undefined; + server.use( + http.post(buildRoute("2/submitPrintingTag/"), async ({ request }) => { + const body = (await request.json()) as { + printingIdentifier?: string; + }; + submittedIdentifier = body.printingIdentifier; + return HttpResponse.json( + { resolvedPrinting: null, isNoMatch: false, voteTally: [] }, + { status: 200 } + ); + }) + ); + renderFeed(); + await revealCard(); + + // Before rejection: the suggested candidate is judged in its own slot (#728), so the + // grid shows only the OTHER candidate - the suggested one never re-appears as a tile. + const initialGrid = await screen.findByTestId( + "question-feed-candidate-grid-ungrouped" + ); + expect(within(initialGrid).getAllByRole("button")).toHaveLength(1); + + // Reject the suggestion: the slot collapses into context, and the rejected candidate + // joins the grid as a de-emphasised, still-selectable tile rather than vanishing. + fireEvent.click(await screen.findByTestId("question-feed-suggestion-no")); + expect( + await screen.findByTestId("question-feed-rejected-context") + ).toHaveTextContent("You said: not"); + const grid = screen.getByTestId("question-feed-candidate-grid-ungrouped"); + expect(within(grid).getAllByRole("button")).toHaveLength(2); + const rejectedNote = await screen.findByTestId( + "question-feed-rejected-tile-note" + ); + expect(rejectedNote).toHaveTextContent("you said no"); + const rejectedTile = rejectedNote.closest("button"); + expect(rejectedTile).not.toBeNull(); + expect(rejectedTile!.getAttribute("data-rejected")).toBe("true"); + + // The reconsider path: tapping the rejected tile casts it as a real pick. + fireEvent.click(rejectedTile!); + await waitFor(() => expect(submittedIdentifier).toBe("printing-1")); + }); + it("shows the suggested printing's own reference image on the suggested-match question (regression: dropped when the suggestion slot was introduced in #49)", async () => { server.use( http.get(buildRoute("2/questionFeed/"), () => @@ -466,6 +537,7 @@ describe("QuestionFeed", () => { it("shows a submitting indicator only on the tapped candidate, not the others or 'No match'", async () => { server.use(questionFeedOnce()); + server.use(submitTagVoteResolvesToApply); let resolveSubmit: () => void = () => undefined; const submitPromise = new Promise((resolve) => { resolveSubmit = resolve; @@ -483,7 +555,7 @@ describe("QuestionFeed", () => { await revealCard(); const tappedCandidate = await screen.findByAltText("xyz 42"); - fireEvent.click(tappedCandidate); + fireEvent.click(tappedCandidate.closest("button") || tappedCandidate); await waitFor(() => expect( @@ -868,6 +940,7 @@ describe("QuestionFeed", () => { ); }) ); + server.use(submitTagVoteResolvesToApply); renderFeed(); await revealCard(); diff --git a/frontend/src/features/questionFeed/QuestionFeed.tsx b/frontend/src/features/questionFeed/QuestionFeed.tsx index 84ec9b72d..04e4b0e4b 100644 --- a/frontend/src/features/questionFeed/QuestionFeed.tsx +++ b/frontend/src/features/questionFeed/QuestionFeed.tsx @@ -31,12 +31,17 @@ * - Preserved verbatim: the candidate question's interaction contract (issue #728 removed * the level1 -> level2 funnel, not the answers - see the de-laddering notes at * `initialStage`'s old site, `rejectSuggestion`, and `candidateQuestionBody`), - * `getAutoTagChips` auto-tagging on candidate pick, no-re-presentation - * (`rejectedCandidateIds`), the singleton-NO terminal vote, per-item - * state reset inside the fetch `.then()` (not a keyed `useEffect` - the stale-filter fix), - * the rate-limit banner, `data-card-*` attributes + the `mpc:card-selected` event (via - * `getPrintingCandidateDataAttributes`, unchanged), every `data-testid` this file's own - * Playwright/jest coverage keys off of. + * `getAutoTagChips` auto-tagging on candidate pick, the singleton-NO terminal vote, + * per-item state reset inside the fetch `.then()` (not a keyed `useEffect` - the + * stale-filter fix), the rate-limit banner, `data-card-*` attributes + the + * `mpc:card-selected` event (via `getPrintingCandidateDataAttributes`, unchanged), every + * `data-testid` this file's own Playwright/jest coverage keys off of. + * - Added (UX repass, 2026-08-09): #748 - a rejected Level 1 suggestion stays reachable in + * the Level 2 grid as a de-emphasised (`data-rejected`) re-selectable tile instead of + * vanishing (see gridCandidates / renderCandidateTile); #745 - illustration groups flow + * side-by-side via IllustrationGroupFlow; #744 - a viewport-relative min-height on WtcHero + * gives Subject's container-scoped sticky real scroll slack even on a short Level 1 + * question. */ import styled from "@emotion/styled"; @@ -177,6 +182,15 @@ const WtcHero = styled.div` flex-wrap: wrap; gap: clamp(12px, 2.2cqi, 22px); align-items: start; + /* Issue #744 - scroll slack for Subject's 'position: sticky'. Sticky can only visibly pin + while its containing block (this hero) has vertical travel room beyond the subject's own + height; a Level 1 question with a short QPanel column (a singleton suggestion, or few + candidates) used to collapse the hero to ~the subject's own height, leaving ~12px of + slack, so the pinned reference card scrolled off 1:1 with the page. A viewport-relative + min-height (SIZING, not positioning - A2's container-scoped sticky policy is untouched) + guarantees the hero spans roughly the first fold of the scroll container even when the + question is short, which is exactly the travel room sticky needs. */ + min-height: calc(100dvh - 132px); `; // Reference-card visibility (issue #710, A2 amendment) - pinned WITHIN the hero container via @@ -347,17 +361,32 @@ const QHint = styled.p` margin: -6px 0 12px; `; +// Issue #745 - the wrapper that makes multiple illustration groups flow side-by-side (the way +// `CandidateGrid`'s own tiles do) instead of each group stacking as its own block row down the +// page. Each group (representative tile + label + credit) is one grid item; auto-fill keeps +// every group in a single column on a narrow container and lets several sit alongside each +// other when the question panel has the width. `align-items: start` so a taller group never +// stretches its shorter neighbours. +const IllustrationGroupFlow = styled.div` + display: grid; + grid-template-columns: repeat( + auto-fill, + minmax(clamp(150px, 26cqi, 240px), 1fr) + ); + gap: clamp(12px, 2.4cqi, 20px); + align-items: start; + margin-bottom: 6px; +`; + // Issue #503 (WTC phase C1) - a small labelled cluster around one `CandidateGrid` (imported, // unmodified) per shared Scryfall illustration, so candidates that are visually near-identical // group together instead of forcing a guess across a flat grid. Deliberately reuses // `CandidateGrid`'s own grid/gap/columns rather than a new grid primitive - this is a -// regrouping of the same tiles, not a new component family. +// regrouping of the same tiles, not a new component family. Inside IllustrationGroupFlow the +// group is purely its own box (label + credit + the representative tile) - spacing between +// groups comes from the wrapper's grid gap, so this carries no margin of its own. const IllustrationGroup = styled.div` - margin-bottom: 10px; - - &:last-child { - margin-bottom: 0; - } + min-width: 0; `; const IllustrationGroupLabel = styled.p` @@ -1091,6 +1120,11 @@ export function QuestionFeed() { // resets any other member of the same group back to untouched, unlike the funnel's usual // independent tri-state cycling that Level 2's optional filter panel keeps. const tapLevel3Chip = (group: ExclusionGroup, tagName: string) => { + // Issue #715 - chips stay inert while a Level 3 submission is in flight; the visual + // `disabled={submitting}` only applies on the post-render, so the ref closes the window. + if (voteInFlightRef.current) { + return; + } setLevel3ChipStates((previous) => { const next = { ...previous }; group.chips.forEach((chip) => { @@ -1188,7 +1222,10 @@ export function QuestionFeed() { // singleton case and immediately calling the same isNoMatch vote "None of these" casts closes // that gap: the vote persists at the moment "No" is tapped, with or without any further tap. const rejectSuggestion = () => { - if (item?.suggestedPrinting == null) { + // Issue #715 - same in-flight guard as every other handler: a double-tap here must not + // re-enter (in the singleton case rejectSuggestion casts the terminal vote itself, so an + // unguarded second entry would double-cast it). + if (item?.suggestedPrinting == null || voteInFlightRef.current) { return; } const rejectedIdentifier = item.suggestedPrinting.identifier; @@ -1249,19 +1286,21 @@ export function QuestionFeed() { const isCandidateType = item.type === "confirm_suggestion" || item.type === "identify_printing"; const allCandidates = item.candidates ?? []; - // Issue #728 - the suggested candidate is judged exactly ONCE, in its own slot above, and is - // never re-offered as a grid tile (the old Level 2 re-presented it "highlighted" - the same - // candidate asked about twice). The grid is the REST of the candidates; the rejected set - // (which only ever holds the suggested id, see rejectSuggestion) is belt-and-braces for that - // same exclusion and drives the "you said not this one" context above. + // Issue #728 - the suggested candidate is judged exactly ONCE in its own slot above and is + // never re-offered as a grid tile while that slot is still asking (the old Level 2 + // re-presented it "highlighted" - the same candidate asked about twice). The grid is the + // REST of the candidates, plus - since #748 - any the user has explicitly rejected at the + // suggestion slot, which stay accessible as de-emphasised, re-selectable tiles rather than + // vanishing (the rejected set drives both that grid inclusion below and the "you said not + // this one" context further down). const suggestedCandidateId = item.type === "confirm_suggestion" ? item.suggestedPrinting?.identifier ?? null : null; const gridCandidates = allCandidates.filter( (candidate) => - candidate.identifier !== suggestedCandidateId && - !rejectedCandidateIds.has(candidate.identifier) + rejectedCandidateIds.has(candidate.identifier) || + candidate.identifier !== suggestedCandidateId ); const visibleCandidates = filterCandidatesByChipStates( gridCandidates, @@ -1283,7 +1322,14 @@ export function QuestionFeed() { // /2/submitPrintingTag/ path. const illustrationGroupsById = new Map(); visibleCandidates.forEach((candidate) => { - if (!candidate.illustrationId) { + // Issue #748 - a rejected candidate never joins an illustration cluster: clusters render + // only one representative tile, so burying the rejected suggestion inside one would + // silently drop it again. It always renders as a standalone (de-emphasised) ungrouped tile + // instead, guaranteeing the reconsider path stays visible. + if ( + !candidate.illustrationId || + rejectedCandidateIds.has(candidate.identifier) + ) { return; } const existingGroup = illustrationGroupsById.get(candidate.illustrationId); @@ -1315,8 +1361,16 @@ export function QuestionFeed() { // Singleton rejection (owner-reported dedup bug, docs/features/printing-tags.md): when the // suggested printing was the card's ONLY candidate, rejecting it empties the grid and // rejectSuggestion already cast the "None of these" vote - this only gates presentation. + // Issue #748 - "none left" means no candidate OTHER than the rejected suggestion itself (the + // rejected one is now a grid member, so gridCandidates.length can no longer decide this). In + // that state the whole grid is one de-emphasised tile and the question was already resolved + // by the terminal vote, so the filter panel and bottom action row stay hidden and the reason + // strip carries the flow - same as the pre-#748 singleton behavior. const suggestionRejectedWithNoneLeft = - suggestionRejected && gridCandidates.length === 0; + suggestionRejected && + allCandidates.filter( + (candidate) => candidate.identifier !== suggestedCandidateId + ).length === 0; // Shape (d) - open-ended (ANNEX B): an `identify_printing` item with no shortlist at all // (the smallest slice - cold-start/no-evidence). Framed as the "tricky one" (WD7) instead of @@ -1426,6 +1480,7 @@ export function QuestionFeed() { tapLevel3Chip(group, chip.tagName)} data-testid={`question-feed-level3-chip-${chip.tagName}`} > @@ -1449,7 +1504,9 @@ export function QuestionFeed() { advance()} + // Issue #715 - route through the guarded skip() rather than a raw advance(): + // a double-tap on this button must not skip two cards. + onClick={skip} data-testid="question-feed-level3-skip" > Skip this question @@ -1493,39 +1550,57 @@ export function QuestionFeed() { // pass IllustrationArtPlaceholder instead of the card-ratio ArtPlaceholder every // ungrouped (full-scan) tile keeps by default. Frame: typeof ArtPlaceholder = ArtPlaceholder - ) => ( - - - - - {`${candidate.expansionCode} - - {submitting && selectedCandidateId === candidate.identifier && ( -
- + ) => { + // Issue #748 - a rejected Level 1 suggestion stays in the grid as a de-emphasised tile + // that is still fully selectable: tap it to reconsider and cast it as a real pick (the + // recover path for a mis-tapped "No, different printing"). + const isRejected = rejectedCandidateIds.has(candidate.identifier); + return ( + + + + + {`${candidate.expansionCode} + + {submitting && selectedCandidateId === candidate.identifier && ( +
+ +
+ )} + + +
+ {" "} + {candidate.expansionCode.toUpperCase()}{" "} + {candidate.collectorNumber}
- )} - - -
- {" "} - {candidate.expansionCode.toUpperCase()}{" "} - {candidate.collectorNumber} -
- {showArtistCaption &&
{candidate.artist}
} - - - ); + {isRejected && ( +
+ you said no · tap to reconsider +
+ )} + {showArtistCaption && ( +
{candidate.artist}
+ )} + + + ); + }; // Problem 2 (owner report, 2026-08-04): a not-official-printing reason (the artwork is // genuine, this scan just isn't one of the listed printings) means the remaining // question - which printing - is still answerable from this same item's own candidate @@ -1721,61 +1796,65 @@ export function QuestionFeed() {
)} - {illustrationGroups.map((group) => { - // Every member of `group` shares one illustrationId, i.e. one artwork - artist - // should be identical across them too, but source data can disagree, so take the - // first non-blank rather than assuming group[0] is always populated. - const illustrationArtist = group - .map((candidate) => candidate.artist) - .find((artist) => artist.trim() !== ""); - // selectIllustrationGroup already submits one illustrationId for the whole - // cluster (see that function's own comment), so one tile fully represents what - // is being voted on - showing every member here just repeats the same artwork - // up to N times. Prefer a member with an art crop, the same signal a tile's own - // image already prefers (renderCandidateTile's imageUrl default below), and fall - // back to the first member so the same data always picks the same tile. - const representative = - group.find((candidate) => candidate.artCropUrl) ?? group[0]; - return ( - - - Same illustration - {group.length} printings - - {illustrationArtist != null && ( - - - - )} - - {renderCandidateTile( - representative, - () => - // every member of `group` shares this non-null illustrationId - see - // the grouping logic above, which only clusters candidates that have - // one - so submitting the representative's illustrationId is - // identical to submitting any other member's. - selectIllustrationGroup( - representative.illustrationId as string, - representative - ), - false, - representative.artCropUrl || - representative.mediumThumbnailUrl, - // Only the illustration-crop image is landscape-shaped - the - // mediumThumbnailUrl fallback above is still a full card scan, so it - // keeps the card-ratio frame. - representative.artCropUrl - ? IllustrationArtPlaceholder - : ArtPlaceholder - )} - - - ); - })} + {illustrationGroups.length > 0 && ( + + {illustrationGroups.map((group) => { + // Every member of `group` shares one illustrationId, i.e. one artwork - artist + // should be identical across them too, but source data can disagree, so take the + // first non-blank rather than assuming group[0] is always populated. + const illustrationArtist = group + .map((candidate) => candidate.artist) + .find((artist) => artist.trim() !== ""); + // selectIllustrationGroup already submits one illustrationId for the whole + // cluster (see that function's own comment), so one tile fully represents what + // is being voted on - showing every member here just repeats the same artwork + // up to N times. Prefer a member with an art crop, the same signal a tile's own + // image already prefers (renderCandidateTile's imageUrl default below), and fall + // back to the first member so the same data always picks the same tile. + const representative = + group.find((candidate) => candidate.artCropUrl) ?? group[0]; + return ( + + + Same illustration - {group.length} printings + + {illustrationArtist != null && ( + + + + )} + + {renderCandidateTile( + representative, + () => + // every member of `group` shares this non-null illustrationId - see + // the grouping logic above, which only clusters candidates that have + // one - so submitting the representative's illustrationId is + // identical to submitting any other member's. + selectIllustrationGroup( + representative.illustrationId as string, + representative + ), + false, + representative.artCropUrl || + representative.mediumThumbnailUrl, + // Only the illustration-crop image is landscape-shaped - the + // mediumThumbnailUrl fallback above is still a full card scan, so it + // keeps the card-ratio frame. + representative.artCropUrl + ? IllustrationArtPlaceholder + : ArtPlaceholder + )} + + + ); + })} + + )} {ungroupedCandidates.length > 0 && ( {ungroupedCandidates.map((candidate) => diff --git a/frontend/tests/QuestionFeed.spec.ts b/frontend/tests/QuestionFeed.spec.ts index cc4ca4549..213b8c677 100644 --- a/frontend/tests/QuestionFeed.spec.ts +++ b/frontend/tests/QuestionFeed.spec.ts @@ -632,13 +632,16 @@ test.describe("question feed - confirm_suggestion question type", () => { expect(abstentionBody.questionType).toBe("confirm_suggestion"); }); - test("NO on the suggestion collapses its slot (never a selectable tile again) and keeps the remaining candidates selectable on the same page, without casting a vote", async ({ + test("NO on the suggestion collapses its slot (rejected candidate stays in the grid as a de-emphasised, re-selectable tile) and keeps the remaining candidates selectable on the same page, without casting a vote", async ({ page, network, }) => { - // Issue #728 - the rejected suggestion is never re-presented as a selectable tile; the - // remaining candidate (printingCandidate2) stays selectable on the SAME page (no stage - // transition). See rejectSuggestion/rejectedCandidateIds in QuestionFeed.tsx. + // Issue #728 - the suggestion slot is judged once and collapses on NO (no stage + // transition); issue #748 - the rejected suggestion does NOT vanish: it joins the grid + // as a de-emphasised (`data-rejected="true"`) tile that stays fully selectable (the + // reconsider path), while the remaining candidate (printingCandidate2) stays selectable + // on the SAME page. NO itself still casts no vote in the general (non-singleton) case. + // See rejectSuggestion/rejectedCandidateIds in QuestionFeed.tsx. let printingTagSubmitted = false; network.use(questionFeedConfirmSuggestion, ...defaultHandlers); page.on("request", (request) => { @@ -650,9 +653,15 @@ test.describe("question feed - confirm_suggestion question type", () => { await page.getByTestId("question-feed-suggestion-no").click(); + // #748 - the rejected suggestion is present, but only as the de-emphasised tile. await expect( page.locator(`[data-card-identifier="${printingCandidate1.identifier}"]`) - ).toHaveCount(0); + ).toHaveCount(1); + await expect( + page.locator( + `[data-card-identifier="${printingCandidate1.identifier}"][data-rejected="true"]` + ) + ).toBeVisible(); await expect( page.locator(`[data-card-identifier="${printingCandidate2.identifier}"]`) ).toBeVisible(); @@ -663,7 +672,7 @@ test.describe("question feed - confirm_suggestion question type", () => { expect(printingTagSubmitted).toBe(false); }); - test("NO on a singleton suggestion (no other candidates) skips the grid entirely and immediately casts the terminal no-match vote", async ({ + test("NO on a singleton suggestion (no other candidates) casts the terminal no-match vote immediately, keeping the rejected candidate as a de-emphasised grid tile", async ({ page, network, }) => { @@ -696,10 +705,16 @@ test.describe("question feed - confirm_suggestion question type", () => { await page.getByTestId("question-feed-suggestion-no").click(); - // the rejected candidate is never a selectable tile again + // #748 - the rejected singleton stays reachable in the grid as the single de-emphasised, + // re-selectable tile; the "none left" state is decided by candidate count, not grid count. await expect( page.locator(`[data-card-identifier="${printingCandidate1.identifier}"]`) - ).toHaveCount(0); + ).toHaveCount(1); + await expect( + page.locator( + `[data-card-identifier="${printingCandidate1.identifier}"][data-rejected="true"]` + ) + ).toBeVisible(); // contextual copy replaces the generic "Which of these is it?" grid prompt await expect( page.getByTestId("question-feed-suggestion-prompt")