Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions docs/features/printing-tags.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
32 changes: 19 additions & 13 deletions frontend/src/features/attributeChips/AttributeChipPanel.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -61,8 +61,8 @@ describe("AttributeChipPanel", () => {
);
render(<Wrapper store={setupStore()} />);

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
Expand All @@ -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");
});

Expand All @@ -108,7 +114,7 @@ describe("AttributeChipPanel", () => {
);
render(<Wrapper store={setupStore()} />);

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
Expand All @@ -126,7 +132,7 @@ describe("AttributeChipPanel", () => {
);
render(<Wrapper store={setupStore()} />);

fireEvent.click(screen.getByTestId("attribute-chip-Full Art"));
fireEvent.click(screen.getByTestId("attribute-chip-Full Art-yes"));
await waitFor(() =>
expect(
screen
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions frontend/src/features/attributeChips/AttributeChipPanel.tsx
Original file line number Diff line number Diff line change
@@ -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.
Expand Down
126 changes: 100 additions & 26 deletions frontend/src/features/attributeChips/attributeChipRender.tsx
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -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;
Expand Down Expand Up @@ -78,7 +120,10 @@ export interface RenderAttributeChipArgs {
confidence: Record<string, number>;
chipStates: Record<string, ChipVoteState>;
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;
}

Expand All @@ -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" &&
Expand All @@ -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 (
<Chip
<ChipGroup
key={tagName}
type="button"
fill={confidenceFill(confidence[tagName] ?? 0)}
impliedNegative={impliedNegative}
disabled={submittingTagName != null}
onClick={() => tap(tagName)}
data-testid={`attribute-chip-${tagName}`}
data-chip-state={explicitState}
title={title}
title={lean ?? undefined}
>
{getTagDisplayName(label)}
</Chip>
<ChipLabel fill={confidenceFill(confidence[tagName] ?? 0)}>
{getTagDisplayName(label)}
</ChipLabel>
<ChipStateButton
type="button"
$active={isPositive}
$polarity="positive"
disabled={disabled}
onClick={() => setState("positive")}
data-testid={`attribute-chip-${tagName}-yes`}
aria-pressed={isPositive}
aria-label={`${getTagDisplayName(label)}: Yes`}
title={isPositive ? "Tap to clear your Yes" : "Yes"}
>
&#10003;
</ChipStateButton>
<ChipStateButton
type="button"
$active={isNegative}
$polarity="negative"
disabled={disabled}
onClick={() => setState("negative")}
data-testid={`attribute-chip-${tagName}-no`}
aria-pressed={isNegative}
aria-label={`${getTagDisplayName(label)}: No`}
title={noTitle}
>
&#10005;
</ChipStateButton>
</ChipGroup>
);
}

Expand Down
15 changes: 7 additions & 8 deletions frontend/src/features/attributeChips/useTagVoting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -44,9 +43,10 @@ export interface UseTagVotingResult {
confidence: Record<string, number>;
/** 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({
Expand All @@ -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,
Expand Down
11 changes: 7 additions & 4 deletions frontend/src/features/attributeVoting/NoMatchReasonStrip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,11 @@ export const NO_MATCH_REASON_TAG_NAMES: Array<string> = (
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
Expand Down Expand Up @@ -133,7 +136,7 @@ export function NoMatchReasonStrip({
"same-origin",
"question-feed"
)
.then(() => onDone())
.then(() => onDone(tagName))
.catch((error) => {
if (isRateLimited(error) && onRateLimited) {
onRateLimited();
Expand Down Expand Up @@ -196,7 +199,7 @@ export function NoMatchReasonStrip({
<Button
variant="outline-secondary"
disabled={submittingTagName != null}
onClick={onDone}
onClick={() => onDone()}
data-testid="no-match-reason-skip"
>
Skip
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/features/questionFeed/QuestionFeed.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -728,7 +728,7 @@ describe("QuestionFeed", () => {

// expand the filter and set "Full Art" positive - narrows item 1's grid to printing-2 only
fireEvent.click(screen.getByTestId("question-feed-filter-toggle"));
fireEvent.click(await screen.findByTestId("attribute-chip-Full Art"));
fireEvent.click(await screen.findByTestId("attribute-chip-Full Art-yes"));
await waitFor(() =>
expect(screen.queryByTestId("attribute-chip-Full Art")).toHaveAttribute(
"data-chip-state",
Expand Down
Loading
Loading