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
82 changes: 82 additions & 0 deletions frontend/src/features/attributeChips/AttributeChipPanel.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@ function buildRoute(path: string): string {
function Wrapper({
store,
onRateLimited,
pruneContradicted,
}: {
store: AppStore;
onRateLimited?: () => void;
pruneContradicted?: boolean;
}) {
const [states, setStates] = React.useState(initialChipStates());
return (
Expand All @@ -35,6 +37,7 @@ function Wrapper({
onChipStatesChange={setStates}
cardSlot={<div data-testid="card-slot-stub">card</div>}
onRateLimited={onRateLimited}
pruneContradicted={pruneContradicted}
/>
</Provider>
);
Expand Down Expand Up @@ -124,6 +127,85 @@ describe("AttributeChipPanel", () => {
expect(sibling.getAttribute("data-chip-state")).toBe("untouched");
});

it("with pruneContradicted, disqualifies untouched exclusion-group siblings entirely", async () => {
server.use(
http.post(buildRoute("2/submitTagVote/"), async ({ request }) => {
const body = (await request.json()) as {
tagName: string;
polarity: number;
};
return HttpResponse.json(
{
tagName: body.tagName,
resolvedPolarity: null,
netPolarity: 1,
tally: [],
},
{ status: 200 }
);
})
);
render(<Wrapper store={setupStore()} pruneContradicted />);

fireEvent.click(screen.getByTestId("attribute-chip-Black Border-yes"));
await waitFor(() =>
expect(
screen
.getByTestId("attribute-chip-Black Border")
.getAttribute("data-chip-state")
).toBe("positive")
);

// the positive chip stays; its untouched group-mates are hidden, not dimmed
expect(screen.getByTestId("attribute-chip-Black Border")).toBeVisible();
expect(screen.queryByTestId("attribute-chip-White Border")).toBeNull();
expect(screen.queryByTestId("attribute-chip-Silver Border")).toBeNull();
// standalone chips are never contradicted and stay visible
expect(screen.getByTestId("attribute-chip-Full Art")).toBeVisible();
});

it("with pruneContradicted, retracting the positive restores the hidden siblings", async () => {
server.use(
http.post(buildRoute("2/submitTagVote/"), async ({ request }) => {
const body = (await request.json()) as {
tagName: string;
polarity: number;
};
return HttpResponse.json(
{
tagName: body.tagName,
resolvedPolarity: null,
netPolarity: body.polarity,
tally: [],
},
{ status: 200 }
);
})
);
render(<Wrapper store={setupStore()} pruneContradicted />);

fireEvent.click(screen.getByTestId("attribute-chip-Black Border-yes"));
await waitFor(() =>
expect(
screen
.getByTestId("attribute-chip-Black Border")
.getAttribute("data-chip-state")
).toBe("positive")
);
expect(screen.queryByTestId("attribute-chip-White Border")).toBeNull();

// tapping the already-active Yes retracts to untouched - the siblings reappear
fireEvent.click(screen.getByTestId("attribute-chip-Black Border-yes"));
await waitFor(() =>
expect(screen.getByTestId("attribute-chip-White Border")).toBeVisible()
);
expect(
screen
.getByTestId("attribute-chip-Black Border")
.getAttribute("data-chip-state")
).toBe("untouched");
});

it("reverts the explicit state on a failed submit", async () => {
server.use(
http.post(buildRoute("2/submitTagVote/"), () =>
Expand Down
26 changes: 23 additions & 3 deletions frontend/src/features/attributeChips/AttributeChipPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,10 @@ import {
} from "@/features/attributeChips/attributeChipRender";
import {
ALL_ATTRIBUTE_CHIPS,
AttributeChipDef,
ChipVoteState,
EXCLUSION_GROUPS,
isChipContradicted,
STANDALONE_CHIPS,
} from "@/features/attributeChips/attributeChips";
import { useTagVoting } from "@/features/attributeChips/useTagVoting";
Expand Down Expand Up @@ -155,6 +157,15 @@ interface AttributeChipPanelProps {
* but stays optional to match the same safe-default convention as the other funnel
* components (see ArtistVotePicker.tsx's identical prop for the full rationale). */
onRateLimited?: () => void;
/** Context-dependent disqualification (DESIGN-REPASS-2026-08.md Rule 5): when true, an
* untouched chip whose own exclusion group already has an explicitly-positive sibling is
* hidden entirely rather than rendered dimmed (implied-negative). An active positive answer
* has already ruled the sibling values out - a card has one border color / one frame era -
* so the disqualified chips are dropped to reclaim their row space, mirroring how the deeper
* question grids prune options that contradict the answer given. False (default) keeps the
* historical dim-and-collapse treatment for callers that want the full taxonomy visible
* (the /display rail's AttributesSection). */
pruneContradicted?: boolean;
}

export function AttributeChipPanel({
Expand All @@ -165,6 +176,7 @@ export function AttributeChipPanel({
onChipStatesChange,
cardSlot,
onRateLimited,
pruneContradicted = false,
}: AttributeChipPanelProps) {
const getTagDisplayName = useTagDisplayName();
const { confidence, submittingTagName, tap } = useTagVoting({
Expand All @@ -184,6 +196,14 @@ export function AttributeChipPanel({
getTagDisplayName,
};

// Context-dependent disqualification (pruneContradicted) filters the render list, not the
// vote state - a disqualified chip is hidden but stays "untouched" in chipStates, so it
// springs straight back the moment its group's positive answer is retracted.
const visibleChips = (chips: AttributeChipDef[]) =>
pruneContradicted
? chips.filter((chip) => !isChipContradicted(chip.tagName, chipStates))
: chips;

// EXCLUSION_GROUPS[0] (Border Color) renders left, [1] (Frame Style) renders right - an
// arbitrary but fixed assignment, not a semantic left/right meaning for either group.
const [leftGroup, rightGroup] = EXCLUSION_GROUPS;
Expand All @@ -199,7 +219,7 @@ export function AttributeChipPanel({
);
const topArea = (
<TopArea>
{STANDALONE_CHIPS.map((chip) =>
{visibleChips(STANDALONE_CHIPS).map((chip) =>
renderAttributeChip(chipArgs, chip.tagName, chip.label)
)}
</TopArea>
Expand All @@ -208,7 +228,7 @@ export function AttributeChipPanel({
<LeftArea>
<GroupHeading>{leftGroup.label}</GroupHeading>
<ExclusionChipRow>
{leftGroup.chips.map((chip) =>
{visibleChips(leftGroup.chips).map((chip) =>
renderAttributeChip(chipArgs, chip.tagName, chip.label)
)}
</ExclusionChipRow>
Expand All @@ -218,7 +238,7 @@ export function AttributeChipPanel({
<RightArea>
<GroupHeading>{rightGroup.label}</GroupHeading>
<ExclusionChipRow>
{rightGroup.chips.map((chip) =>
{visibleChips(rightGroup.chips).map((chip) =>
renderAttributeChip(chipArgs, chip.tagName, chip.label)
)}
</ExclusionChipRow>
Expand Down
19 changes: 6 additions & 13 deletions frontend/src/features/attributeChips/attributeChipRender.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import React from "react";

import {
ChipVoteState,
findExclusionGroup,
isChipContradicted,
} from "@/features/attributeChips/attributeChips";

// Mobile funnel pass (thumb-native tap targets): measured at ~30px tall against the previous
Expand Down Expand Up @@ -63,7 +63,7 @@ export const ChipGroup = styled.div<{ impliedNegative: boolean }>`

export const ChipLabel = styled.span<{ fill: string; collapsed?: boolean }>`
background-color: ${(props) => props.fill};
padding: ${(props) => (props.collapsed ? "0" : "0.35rem 0.5rem")};
padding: ${(props) => (props.collapsed ? "0" : "0.3rem 0.45rem")};
width: ${(props) => (props.collapsed ? "0" : "auto")};
display: ${(props) => (props.collapsed ? "none" : "inline-flex")};
align-items: center;
Expand All @@ -90,10 +90,11 @@ export const ChipStateButton = styled.button<{
font-weight: ${(props) => (props.$active ? 700 : 400)};
min-height: 44px;
min-width: 32px;
padding: 0.35rem 0.4rem;
padding: 0.3rem 0.3rem;
display: inline-flex;
align-items: center;
justify-content: center;
touch-action: manipulation;

&:disabled {
opacity: 0.5;
Expand All @@ -103,7 +104,7 @@ export const ChipStateButton = styled.button<{
export const ChipRow = styled.div`
display: flex;
flex-wrap: wrap;
gap: 0.4rem;
gap: 0.3rem;
justify-content: center;
`;

Expand Down Expand Up @@ -157,15 +158,7 @@ export function renderAttributeChip(
const explicitState = chipStates[tagName] ?? "untouched";
const isPositive = explicitState === "positive";
const isNegative = explicitState === "negative";
const group = findExclusionGroup(tagName);
const impliedNegative =
explicitState === "untouched" &&
group != null &&
group.chips.some(
(sibling) =>
sibling.tagName !== tagName &&
(chipStates[sibling.tagName] ?? "untouched") === "positive"
);
const impliedNegative = isChipContradicted(tagName, chipStates);
const lean = leanTooltip(confidence[tagName] ?? 0);
const disabled = submittingTagName != null;
const setState = (desired: "positive" | "negative") =>
Expand Down
41 changes: 41 additions & 0 deletions frontend/src/features/attributeChips/attributeChips.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
findExclusionGroup,
getAutoTagChips,
getOpenExclusionGroups,
isChipContradicted,
nextChipState,
} from "./attributeChips";

Expand Down Expand Up @@ -126,3 +127,43 @@ describe("getOpenExclusionGroups", () => {
expect(openGroups.map((group) => group.id)).toEqual(["borderColor"]);
});
});

describe("isChipContradicted", () => {
it("is true for an untouched exclusion-group sibling of an explicit positive", () => {
expect(
isChipContradicted("White Border", { "Black Border": "positive" })
).toBe(true);
expect(
isChipContradicted("Silver Border", { "Black Border": "positive" })
).toBe(true);
});

it("is false for the chip that owns the positive vote itself", () => {
expect(
isChipContradicted("Black Border", { "Black Border": "positive" })
).toBe(false);
});

it("is false for an explicitly-voted sibling, even when a group-mate is positive", () => {
// an explicit negative is itself an active filter, not a disqualified option
expect(
isChipContradicted("White Border", {
"Black Border": "positive",
"White Border": "negative",
})
).toBe(false);
});

it("is false for a negative vote alone - it does not rule out any sibling value", () => {
expect(
isChipContradicted("White Border", { "Black Border": "negative" })
).toBe(false);
});

it("is false for standalone chips, which have no exclusion group", () => {
expect(isChipContradicted("Full Art", { "Full Art": "positive" })).toBe(
false
);
expect(isChipContradicted("Full Art", {})).toBe(false);
});
});
31 changes: 31 additions & 0 deletions frontend/src/features/attributeChips/attributeChips.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,37 @@ export function findExclusionGroup(
);
}

/**
* True when a chip is contradicted by the current vote state: it is itself untouched AND an
* explicitly-positive sibling in its own exclusion group already answers the group's question
* (a card has exactly one border color / one frame era, so "Black Border" being yes leaves
* "White Border"/"Silver Border" factually disqualified). Standalone chips and explicitly-
* voted chips (positive or negative) are never contradicted - the latter are themselves the
* active filters that disqualify others.
*
* Surfaces may either dim such chips (implied-negative styling) or hide them entirely -
* the question feed's filter panel hides them (context-dependent disqualification,
* DESIGN-REPASS-2026-08.md Rule 5), mirroring how the deeper question grids drop any option
* that contradicts the answer already given, rather than only greying it out.
*/
export function isChipContradicted(
tagName: string,
chipStates: Record<string, ChipVoteState>
): boolean {
if ((chipStates[tagName] ?? "untouched") !== "untouched") {
return false;
}
const group = findExclusionGroup(tagName);
if (group == null) {
return false;
}
return group.chips.some(
(sibling) =>
sibling.tagName !== tagName &&
(chipStates[sibling.tagName] ?? "untouched") === "positive"
);
}

/**
* Filters candidates against the current explicit chip vote states: a positive chip drops
* any candidate that doesn't match it, a negative chip drops any candidate that does. Implied-
Expand Down
4 changes: 4 additions & 0 deletions frontend/src/features/printingTags/cardPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,10 @@ export const CandidateButton = styled.button`
color: inherit;
text-align: left;
cursor: pointer;
/* DESIGN-REPASS Rule 2 (#715) - kills the mobile double-tap-zoom gesture that swallows a
fast single tap (the first tap starts a zoom, the second lands the click, reading as
"needs two taps"), so every tap on a candidate tile registers on the first press. */
touch-action: manipulation;

/* Issue #705 - same fix as SuggestedThumb (QuestionFeed.tsx): clip to the rounded tile at
rest, stop clipping for exactly the hover duration ZoomableThumbnail scales its <img> up,
Expand Down
36 changes: 36 additions & 0 deletions frontend/src/features/questionFeed/QuestionFeed.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,42 @@ describe("QuestionFeed", () => {
).not.toBeInTheDocument();
});

it("the feed's filter panel hides exclusion-group siblings of an explicit positive (context-dependent disqualification)", async () => {
server.use(
questionFeedOnce(),
http.post(buildRoute("2/submitTagVote/"), async ({ request }) => {
const body = (await request.json()) as {
tagName: string;
polarity: number;
};
return HttpResponse.json(
{
tagName: body.tagName,
resolvedPolarity: null,
netPolarity: body.polarity,
tally: [],
},
{ status: 200 }
);
})
);
renderFeed();
await revealCard();
await screen.findByTestId("attribute-chip-Black Border");

fireEvent.click(screen.getByTestId("attribute-chip-Black Border-yes"));
await waitFor(() =>
expect(
screen
.getByTestId("attribute-chip-Black Border")
.getAttribute("data-chip-state")
).toBe("positive")
);
// the contradicted siblings are pruned from the feed's panel, not just dimmed
expect(screen.queryByTestId("attribute-chip-White Border")).toBeNull();
expect(screen.queryByTestId("attribute-chip-Silver Border")).toBeNull();
});

it("clicking 'None of these' submits a no-match printing vote", async () => {
server.use(questionFeedOnce());
let submittedIsNoMatch: boolean | undefined;
Expand Down
Loading
Loading