From 74ef073e3132cb0731b142d7c33840d094290bf2 Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Sun, 9 Aug 2026 15:17:08 +0000 Subject: [PATCH 1/6] attributeVoting: uniform answer-row button geometry (#711) and synchronous double-tap guards (#715) DESIGN-REPASS Rule 1 (#711): every action button on the WTC surface - the embedded custom triggers (tag question Apply/Not applicable, no-match reason strip Skip, artist picker candidate buttons) - now shares the spec's uniform metrics (min-height 44px, 15px/600, 6px 16px padding, --r-btn radius) via one new ActionButton primitive, instead of react-bootstrap's default geometry that read smaller than the feed's own Btn rows. Variants map to the surface palette (Apply=primary, Not applicable=secondary, Skip=ghost, artist consensus=success), matching the wtc-mockup's button mapping. Rule 2 (#715): synchronous in-flight refs in QueueTagQuestion, NoMatchReasonStrip and ArtistVotePicker close the double-tap window - the visual disabled flag only applies on the post-render, so a fast second tap could previously re-enter the vote handler and cast twice. The ActionButton also carries touch-action: manipulation (kills the mobile double-tap-zoom gesture that swallows single taps). --- .../features/attributeVoting/ActionButton.tsx | 71 +++++++++++++++++++ .../attributeVoting/ArtistVotePicker.tsx | 37 ++++++---- .../attributeVoting/NoMatchReasonStrip.tsx | 31 ++++++-- .../attributeVoting/QueueTagQuestion.tsx | 44 ++++++++---- 4 files changed, 150 insertions(+), 33 deletions(-) create mode 100644 frontend/src/features/attributeVoting/ActionButton.tsx 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?
- - - +
); From 2aea87dde52979ac9a55fae8086d9f3664ab492e Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Sun, 9 Aug 2026 15:18:20 +0000 Subject: [PATCH 2/6] wtc: keep rejected Level 1 suggestions reachable in the candidate grid (#748) Rejecting the suggested printing no longer drops it from the surface: the candidate grid regains it as a de-emphasised tile (dimmed, dashed outline, 'you said no - tap to reconsider' note) that stays fully selectable, the reconsider path for a mis-tapped 'No, different printing'. Rejected candidates never join illustration clusters (a cluster renders one representative tile, which would silently bury them again) and the singleton 'none left' state is now decided by candidate count rather than grid count, since the rejected suggestion is a grid member again. CardPanel: .rejected tile style + .rej caption note. QuestionFeed.test: new jest coverage for the grid inclusion + reconsider click. Playwright: the two #728-era 'never a selectable tile again' assertions now expect the de-emphasised data-rejected tile to remain. --- .../src/features/printingTags/cardPanel.tsx | 27 +++++++ .../questionFeed/QuestionFeed.test.tsx | 71 +++++++++++++++++++ frontend/tests/QuestionFeed.spec.ts | 31 +++++--- 3 files changed, 121 insertions(+), 8 deletions(-) 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..5ea91c7f9 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/"), () => 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") From e44ccdfa3154e1889a1d0d4febcdbf19b6f85bcf Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Sun, 9 Aug 2026 16:12:17 +0000 Subject: [PATCH 3/6] test(question-feed): add coverage acks for repass spec title updates in PR 759 --- .github/coverage-acks.txt | 2 ++ 1 file changed, 2 insertions(+) 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 From bca14cbd292ef921b4c72aea7256affcf60c958e Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Sun, 9 Aug 2026 17:54:03 +0000 Subject: [PATCH 4/6] test(question-feed): fix candidate submitting indicator test selector and msw handler --- frontend/src/features/questionFeed/QuestionFeed.test.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frontend/src/features/questionFeed/QuestionFeed.test.tsx b/frontend/src/features/questionFeed/QuestionFeed.test.tsx index 5ea91c7f9..801730eda 100644 --- a/frontend/src/features/questionFeed/QuestionFeed.test.tsx +++ b/frontend/src/features/questionFeed/QuestionFeed.test.tsx @@ -537,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; @@ -554,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( From 19bd0ef942b34128ceb1fd5cde761832502d737f Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Mon, 10 Aug 2026 03:07:46 +0000 Subject: [PATCH 5/6] wtc: keep rejected suggestions reachable in the candidate grid as de-emphasised re-selectable tiles #748 - a rejected Level 1 suggestion stays reachable in the Level 2 candidate grid as a de-emphasised (data-rejected) re-selectable tile instead of vanishing; the gridCandidates filter now re-admits rejected ids, the tile renders a 'you said no - tap to reconsider' note, and rejected candidates never join illustration clusters. Fixes the CI failures this PR's head was carrying: the Jest candidate-grid button-count assertion and both Playwright spec assertions (lines 635/675). Includes the #744 viewport-relative WtcHero min-height scroll slack, the #745 IllustrationGroupFlow side-by-side illustration groups, and the #715 in-flight guards (tapLevel3Chip, rejectSuggestion, guarded skip). --- .../questionFeed/QuestionFeed.test.tsx | 1 + .../features/questionFeed/QuestionFeed.tsx | 299 +++++++++++------- 2 files changed, 190 insertions(+), 110 deletions(-) diff --git a/frontend/src/features/questionFeed/QuestionFeed.test.tsx b/frontend/src/features/questionFeed/QuestionFeed.test.tsx index 801730eda..ec961f054 100644 --- a/frontend/src/features/questionFeed/QuestionFeed.test.tsx +++ b/frontend/src/features/questionFeed/QuestionFeed.test.tsx @@ -940,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) => From 7835f5d090e752dd56f186e2de2d409f18bed06f Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Mon, 10 Aug 2026 03:25:05 +0000 Subject: [PATCH 6/6] docs(printing-tags): correct the no-re-presentation rule for rejected-suggestion retention MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The shipped behaviour (issues #711/#715/#748, plus #744/#745) keeps a rejected Level 1 suggestion in the Level 2 grid as a de-emphasised, fully re-selectable tile (data-rejected, 'you said no · tap to reconsider') - the recover path for a mis-tap. The living doc still described the pre-#748 behaviour: candidates-minus-rejected display sets, a nonRejectedCandidates-derived grid, and a singleton case that skipped the grid entirely. Rewrite the no-re-presentation section to match the code (gridCandidates includes the rejected set, a rejected candidate never joins an illustration cluster, and the none-left state is decided by candidate count, not grid count) and fix the Level 1 bullet's cross-reference from 'excludes it' to the retention behaviour. The illustration-grouping bullet needed no correction - it makes no block-row layout claim that IllustrationGroupFlow contradicts. --- docs/features/printing-tags.md | 52 +++++++++++++++++++++------------- 1 file changed, 33 insertions(+), 19 deletions(-) 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