diff --git a/docs/features/printing-tags.md b/docs/features/printing-tags.md index e47803a4e..88f899778 100644 --- a/docs/features/printing-tags.md +++ b/docs/features/printing-tags.md @@ -1944,6 +1944,22 @@ for history (this doc's own established convention — see the `cardPanel.tsx` b ## Known gaps +- **Illustration-grouped tiles still show printing scans, not an art crop** + (owner report, 2026-08-04): `selectIllustrationGroup` casts an + illustration-level vote, but `IllustrationGroup`'s tiles in + `QuestionFeed.tsx` still render each candidate's own + `mediumThumbnailUrl` (a per-printing scan) via the same + `renderCandidateTile` markup the ungrouped grid uses — asking the voter + to discriminate on border/frame/language differences the vote itself + doesn't record. A shared art crop isn't available to plumb in instead: + `QuestionFeedItem.scryfallIllustrationUrl` (`schema_types.ts`) is a + single, item-level field consumed only by the unrelated `artist` + question type's subject-card re-frame, and `PrintingCandidate` carries + no art-crop field of its own — only `mediumThumbnailUrl`/ + `smallThumbnailUrl`. Making this fix possible needs a backend change + (a per-candidate or, more efficiently, a per-`illustrationId` art-crop + URL on the questionFeed payload) that is out of scope for a + frontend-only change and was not attempted here. - Client-side (local-folder/Google Drive) search gets no re-rank/filter/ match-indicator parity — no ES/DB access on that path. - The starburst/card/chip-ring layout was hand-tuned via iterative diff --git a/frontend/src/features/attributeChips/AttributeChipPanel.test.tsx b/frontend/src/features/attributeChips/AttributeChipPanel.test.tsx index 9605bff17..43a970810 100644 --- a/frontend/src/features/attributeChips/AttributeChipPanel.test.tsx +++ b/frontend/src/features/attributeChips/AttributeChipPanel.test.tsx @@ -41,7 +41,7 @@ function Wrapper({ } describe("AttributeChipPanel", () => { - it("cycles a chip untouched -> positive -> negative -> untouched, casting one vote per tap", async () => { + it("reaches positive, negative, and back to untouched in exactly one tap each - no cycle", async () => { server.use( http.post(buildRoute("2/submitTagVote/"), async ({ request }) => { const body = (await request.json()) as { @@ -61,8 +61,8 @@ describe("AttributeChipPanel", () => { ); render(); - const chip = screen.getByTestId("attribute-chip-Full Art"); - expect(chip.getAttribute("data-chip-state")).toBe("untouched"); + const group = screen.getByTestId("attribute-chip-Full Art"); + expect(group.getAttribute("data-chip-state")).toBe("untouched"); // each click's optimistic state update lands synchronously, but the button stays // `disabled` (submitting) until the mocked request's promise resolves - wait for it to @@ -72,17 +72,23 @@ describe("AttributeChipPanel", () => { await waitFor(() => { const el = screen.getByTestId("attribute-chip-Full Art"); expect(el.getAttribute("data-chip-state")).toBe(expectedState); - expect(el).not.toBeDisabled(); + expect( + screen.getByTestId("attribute-chip-Full Art-yes") + ).not.toBeDisabled(); }); }; - fireEvent.click(chip); - await waitForSettled("positive"); - - fireEvent.click(screen.getByTestId("attribute-chip-Full Art")); + // one tap on "no" reaches negative directly from untouched - no need to pass through + // positive first. + fireEvent.click(screen.getByTestId("attribute-chip-Full Art-no")); await waitForSettled("negative"); - fireEvent.click(screen.getByTestId("attribute-chip-Full Art")); + // one tap on "yes" switches straight to positive from negative - still one tap, no cycle. + fireEvent.click(screen.getByTestId("attribute-chip-Full Art-yes")); + await waitForSettled("positive"); + + // tapping the already-active button retracts to untouched. + fireEvent.click(screen.getByTestId("attribute-chip-Full Art-yes")); await waitForSettled("untouched"); }); @@ -108,7 +114,7 @@ describe("AttributeChipPanel", () => { ); render(); - fireEvent.click(screen.getByTestId("attribute-chip-Black Border")); + fireEvent.click(screen.getByTestId("attribute-chip-Black Border-yes")); await waitFor(() => expect(submittedTagNames).toEqual(["Black Border"])); // sibling should render implied-negative (dimmed) without ever being submitted @@ -126,7 +132,7 @@ describe("AttributeChipPanel", () => { ); render(); - fireEvent.click(screen.getByTestId("attribute-chip-Full Art")); + fireEvent.click(screen.getByTestId("attribute-chip-Full Art-yes")); await waitFor(() => expect( screen @@ -159,7 +165,7 @@ describe("AttributeChipPanel", () => { /> ); - fireEvent.click(screen.getByTestId("attribute-chip-Full Art")); + fireEvent.click(screen.getByTestId("attribute-chip-Full Art-yes")); await waitFor(() => expect(rateLimitedCallCount).toBe(1)); expect( @@ -192,7 +198,7 @@ describe("AttributeChipPanel", () => { /> ); - fireEvent.click(screen.getByTestId("attribute-chip-Full Art")); + fireEvent.click(screen.getByTestId("attribute-chip-Full Art-yes")); await waitFor(() => { const notifications = Object.values( store.getState().toasts.notifications diff --git a/frontend/src/features/attributeChips/AttributeChipPanel.tsx b/frontend/src/features/attributeChips/AttributeChipPanel.tsx index 6c7900393..a28b04afe 100644 --- a/frontend/src/features/attributeChips/AttributeChipPanel.tsx +++ b/frontend/src/features/attributeChips/AttributeChipPanel.tsx @@ -1,9 +1,9 @@ /** * Tri-state attribute chips surrounding the subject card in the unified question feed (see - * QuestionFeed.tsx and docs/features/printing-tags.md's questionFeed section). Each chip - * cycles untouched -> positive -> negative -> untouched on tap, casting a real CardTagVote - * each time (including the retraction on cycling back to untouched - see - * cardpicker.views.RETRACT_POLARITY). Fill color/intensity renders the tag's current + * QuestionFeed.tsx and docs/features/printing-tags.md's questionFeed section). Each chip is a + * Yes/No button pair, either directly reachable in one tap, casting a real CardTagVote each + * time (including the retraction when tapping the already-active button back to untouched - + * see cardpicker.views.RETRACT_POLARITY). Fill color/intensity renders the tag's current * weighted net polarity (confidence), independent of - though usually correlated with - this * voter's own explicit state; exclusion-group siblings of an explicitly-positive chip render * a separate "implied-negative" dimmed style without casting a vote of their own. diff --git a/frontend/src/features/attributeChips/attributeChipRender.tsx b/frontend/src/features/attributeChips/attributeChipRender.tsx index d9f878215..187d57c68 100644 --- a/frontend/src/features/attributeChips/attributeChipRender.tsx +++ b/frontend/src/features/attributeChips/attributeChipRender.tsx @@ -1,13 +1,24 @@ /** - * The chip button styling + single chip-button render (fill color, implied-negative dimming, + * The chip group styling + single chip-group render (fill color, implied-negative dimming, * lean tooltip, data-chip-state) shared by every attribute-chip layout. Extracted out of * AttributeChipPanel.tsx (Proposal H pane migration, left-panel unification) so the display * page's rail Attributes section (features/display/AttributesSection.tsx) renders byte-for-byte * the same chip a caller sees in the question feed's ring - only the surrounding arrangement * (ring around a card vs. a plain vertical stack) differs between the two, per the design doc's - * §5 component-mapping table. AttributeChipPanel.tsx imports Chip/ChipRow/renderAttributeChip - * from here (one-directional) rather than the other way around, so there's no import cycle - * between this file and its own ring-layout caller. + * §5 component-mapping table. AttributeChipPanel.tsx imports ChipRow/renderAttributeChip from + * here (one-directional) rather than the other way around, so there's no import cycle between + * this file and its own ring-layout caller. + * + * Direct-access Yes/No (owner-reported defect, 2026-08-04): a single cycling button + * (untouched -> positive -> negative -> untouched) made a "No" answer cost two taps, and an + * untouched chip whose exclusion-group sibling was explicitly positive rendered dimmed + * ("implied negative") while having cast no vote of its own - a voter reading that dimming as + * "I already said no" was looking at a UI that disagreed with the data. Each chip is now a + * labelled group of two always-visible buttons (Yes / No), either one reachable in exactly one + * tap from any prior state - tapping the button matching the chip's current explicit state + * retracts it back to untouched (the vote-level RETRACT_POLARITY behavior is unchanged, only how + * many taps it takes to reach any of the three states changed). This still can't be forked per + * surface: the rail and the question feed's ring render the exact same group. */ import styled from "@emotion/styled"; import React from "react"; @@ -19,20 +30,51 @@ import { // Mobile funnel pass (thumb-native tap targets): measured at ~30px tall against the previous // 0.35rem/0.6rem padding - short of the 44px minimum both Apple's HIG and WCAG 2.5.5 (Target -// Size, AA) call for, on the ring's own answer controls. min-height/min-width guarantee the real -// hit area regardless of label length; flex centering keeps the (unchanged, still compact) text -// centered in the now-taller box rather than pinned to its old top-padding baseline. -export const Chip = styled.button<{ fill: string; impliedNegative: boolean }>` +// Size, AA) call for, on the ring's own answer controls. min-height guarantees the real hit +// area of the group as a whole regardless of label length; flex centering keeps the label +// vertically centered against the taller Yes/No buttons beside it. +export const ChipGroup = styled.div<{ impliedNegative: boolean }>` border: 2px solid rgba(0, 0, 0, 0.25); border-radius: 0.5rem; - background-color: ${(props) => props.fill}; opacity: ${(props) => (props.impliedNegative ? 0.45 : 1)}; color: inherit; - padding: 0.35rem 0.6rem; font-size: 0.85rem; white-space: nowrap; min-height: 44px; - min-width: 44px; + display: inline-flex; + align-items: stretch; + overflow: hidden; +`; + +export const ChipLabel = styled.span<{ fill: string }>` + background-color: ${(props) => props.fill}; + padding: 0.35rem 0.5rem; + display: inline-flex; + align-items: center; +`; + +// Yes/No are separate, always-visible buttons rather than a single cycling one so each of the +// chip's three meaningful answers (yes / no / no opinion) is reachable in exactly one tap - see +// this file's header comment. `$active` reflects this voter's own explicit tap (independent of +// `fill`'s fluctuating community-plus-machine lean, which is what let a just-cast vote appear to +// "not have registered" once the server's real weighted polarity replaced the optimistic one). +export const ChipStateButton = styled.button<{ + $active: boolean; + $polarity: "positive" | "negative"; +}>` + border: none; + border-left: 1px solid rgba(0, 0, 0, 0.25); + background-color: ${(props) => + props.$active + ? props.$polarity === "positive" + ? "rgba(40, 167, 69, 0.85)" + : "rgba(220, 53, 69, 0.85)" + : "transparent"}; + color: ${(props) => (props.$active ? "#fff" : "inherit")}; + font-weight: ${(props) => (props.$active ? 700 : 400)}; + min-height: 44px; + min-width: 32px; + padding: 0.35rem 0.4rem; display: inline-flex; align-items: center; justify-content: center; @@ -78,7 +120,10 @@ export interface RenderAttributeChipArgs { confidence: Record; chipStates: Record; submittingTagName: string | null; - tap: (tagName: string) => void; + /** Sets tagName's explicit state directly to `desired` ("positive" or "negative") in one + * call - tapping the button that's already active is what retracts to "untouched", handled + * by the caller before invoking this (see the two `onClick`s below). */ + tap: (tagName: string, desired: ChipVoteState) => void; getTagDisplayName: (label: string) => string; } @@ -94,6 +139,8 @@ export function renderAttributeChip( label: string ): React.ReactElement { const explicitState = chipStates[tagName] ?? "untouched"; + const isPositive = explicitState === "positive"; + const isNegative = explicitState === "negative"; const group = findExclusionGroup(tagName); const impliedNegative = explicitState === "untouched" && @@ -104,26 +151,53 @@ export function renderAttributeChip( (chipStates[sibling.tagName] ?? "untouched") === "positive" ); const lean = leanTooltip(confidence[tagName] ?? 0); - const title = - explicitState === "positive" - ? "Yes" - : explicitState === "negative" - ? "No" - : lean ?? "Tap to describe what you see"; + const disabled = submittingTagName != null; + const setState = (desired: "positive" | "negative") => + tap(tagName, explicitState === desired ? "untouched" : desired); + const noTitle = impliedNegative + ? "Implied by another answer above - no vote cast yet. Tap to answer this directly." + : isNegative + ? "Tap to clear your No" + : "No"; + return ( - tap(tagName)} data-testid={`attribute-chip-${tagName}`} data-chip-state={explicitState} - title={title} + title={lean ?? undefined} > - {getTagDisplayName(label)} - + + {getTagDisplayName(label)} + + setState("positive")} + data-testid={`attribute-chip-${tagName}-yes`} + aria-pressed={isPositive} + aria-label={`${getTagDisplayName(label)}: Yes`} + title={isPositive ? "Tap to clear your Yes" : "Yes"} + > + ✓ + + setState("negative")} + data-testid={`attribute-chip-${tagName}-no`} + aria-pressed={isNegative} + aria-label={`${getTagDisplayName(label)}: No`} + title={noTitle} + > + ✕ + + ); } diff --git a/frontend/src/features/attributeChips/useTagVoting.ts b/frontend/src/features/attributeChips/useTagVoting.ts index 3cc55cac4..22dfa0b2b 100644 --- a/frontend/src/features/attributeChips/useTagVoting.ts +++ b/frontend/src/features/attributeChips/useTagVoting.ts @@ -19,7 +19,6 @@ import { useAppDispatch } from "@/common/types"; import { CHIP_POLARITY, ChipVoteState, - nextChipState, } from "@/features/attributeChips/attributeChips"; import { APISubmitTagVote } from "@/store/api"; import { setNotification } from "@/store/slices/toastsSlice"; @@ -44,9 +43,10 @@ export interface UseTagVotingResult { confidence: Record; /** The tagName currently mid-submission (disables every chip until it settles), or null. */ submittingTagName: string | null; - /** Cycles the given tag's state (untouched -> positive -> negative -> untouched) and casts - * exactly one real vote for that tap. */ - tap: (tagName: string) => void; + /** Sets the given tag's explicit state directly to `desired` and casts exactly one real + * vote for that tap - direct-access Yes/No, not a cycle (see attributeChipRender.tsx's + * header comment for why this stopped being a cycle). */ + tap: (tagName: string, desired: ChipVoteState) => void; } export function useTagVoting({ @@ -68,15 +68,14 @@ export function useTagVoting({ setConfidence(tagConfidence); }, [tagConfidence]); - const tap = (tagName: string) => { + const tap = (tagName: string, desired: ChipVoteState) => { const previousState = chipStates[tagName] ?? "untouched"; const previousConfidence = confidence[tagName] ?? 0; - const nextState = nextChipState(previousState); - const polarity = CHIP_POLARITY[nextState]; + const polarity = CHIP_POLARITY[desired]; // optimistic: nudge the fill toward the tapped direction immediately, and update the // explicit state right away - both get reconciled with the server response below - onChipStatesChange({ ...chipStates, [tagName]: nextState }); + onChipStatesChange({ ...chipStates, [tagName]: desired }); setConfidence((previous) => ({ ...previous, [tagName]: polarity === 0 ? 0 : polarity, diff --git a/frontend/src/features/attributeVoting/NoMatchReasonStrip.tsx b/frontend/src/features/attributeVoting/NoMatchReasonStrip.tsx index 9ed566c8e..082ff85c9 100644 --- a/frontend/src/features/attributeVoting/NoMatchReasonStrip.tsx +++ b/frontend/src/features/attributeVoting/NoMatchReasonStrip.tsx @@ -96,8 +96,11 @@ export const NO_MATCH_REASON_TAG_NAMES: Array = ( interface NoMatchReasonStripProps { backendURL: string; cardIdentifier: string; - /** Called once a reason has been submitted, or the user skips. */ - onDone: () => void; + /** Called once a reason has been submitted (with the chosen tagName), or the user skips + * (with no argument) - the caller uses the tagName to route not-official-printing answers + * back to the candidate grid instead of straight to the next item, see QuestionFeed.tsx's + * own onNoMatchReasonDone. */ + onDone: (chosenTagName?: string) => void; /** Called instead of the usual error toast when a submission is rejected with 429 - see * ArtistVotePicker.tsx's identical prop for the full rationale. This component has only one * caller today (QuestionFeed.tsx), so this is effectively always provided, but stays optional @@ -133,7 +136,7 @@ export function NoMatchReasonStrip({ "same-origin", "question-feed" ) - .then(() => onDone()) + .then(() => onDone(tagName)) .catch((error) => { if (isRateLimited(error) && onRateLimited) { onRateLimited(); @@ -196,7 +199,7 @@ export function NoMatchReasonStrip({