Skip to content

feat(question-feed): credit each illustration cluster via ArtistSupportLink #688

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
WilfordGrimley merged 1 commit into from
Aug 4, 2026
Merged

feat(question-feed): credit each illustration cluster via ArtistSupportLink #688

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 17 additions & 1 deletion docs/features/artist-support-links.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ non-null) or a vote the user just cast themselves - never for a vote-
pending or unknown artist, since there'd be no name to build a URL from.
M2 didn't widen this gate at all, only what gets rendered once it's open.

## Surfaces (three, as of the Proposal H pane migration)
## Surfaces (four, as of the question-feed illustration-cluster credit)

1. **Card Detail Modal** (`CardDetailedViewModal.tsx`'s attribute table,
the `"Canonical Aritst"` row - yes, that's a pre-existing typo in the
Expand Down Expand Up @@ -114,6 +114,22 @@ M2 didn't widen this gate at all, only what gets rendered once it's open.
rail's currently-selected slot's own `CardDocument` (already resident in
`cardDocumentsByIdentifier`) for the artist name - the applet's own data
fetch is separate (keyed by that name), not piggybacked on this read.
4. **`/whatsthat`'s Level 2 illustration clusters** (`QuestionFeed.tsx`,
the `IllustrationGroup` wrapper `illustrationGroups` renders around
candidates sharing a Scryfall `illustrationId`). Gating here differs
from surfaces 1-3: those gate on a _confirmed_ artist (a cast vote, or
`CardDocument.canonicalArtist` consensus); this one gates on the first
non-blank `candidate.artist` across the cluster's members - the artist
of a _candidate printing_, sourced from canonical Scryfall reference
data, before the voter has picked anything. Worded `"Illustration by <Name>"` rather than the `"Art by <Name>"` phrasing surfaces 2-3 use, so
it doesn't read as an assertion about the voter's own scanned card. The
applet itself renders unmodified; a `max-width` wrapper
(`IllustrationCredit`, caller-side only, not a `className` override of
the applet's own "stretch to fill" rule) keeps one full-bleed CTA per
cluster from reading as page-width inside a multi-cluster grid. The
per-tile artist caption is dropped inside grouped clusters (redundant
with the new cluster-level credit) but kept on ungrouped tiles, which
have no cluster credit of their own.

**Not built** (explicitly out of scope, noted so a future session doesn't
have to re-derive why): the confidently-known-artist collapsed display
Expand Down
81 changes: 57 additions & 24 deletions frontend/src/features/questionFeed/QuestionFeed.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,14 @@ const IllustrationGroupLabel = styled.p`
margin: 0 0 4px;
`;

// Caps the width of the reused ArtistSupportLink applet so a full-bleed button (its own
// "stretch to fill" rule - see the component's docstring, not overridden here) reads as a
// compact cluster credit rather than a page-width CTA repeated once per cluster.
const IllustrationCredit = styled.div`
max-width: 220px;
margin-bottom: 8px;
`;

// The spec's `.btn` base + variants (section 1c) - min 44px thumb targets (mobile funnel
// pass, WCAG 2.5.5/Apple HIG), replacing the old `ThumbButton`/`FilterToggleButton` gold
// overrides with plain token-derived variants. A native <button>, not a react-bootstrap
Expand Down Expand Up @@ -1442,7 +1450,13 @@ export function QuestionFeed() {
// endpoints despite sharing this exact markup.
const renderCandidateTile = (
candidate: PrintingCandidate,
onSelect: () => void = () => selectCandidate(candidate, false)
onSelect: () => void = () => selectCandidate(candidate, false),
// Illustration clusters (below) pass false: the cluster now carries its own
// ArtistSupportLink credit above the grid, so repeating the same name on every tile
// inside it is redundant. Ungrouped tiles have no cluster-level credit, so they keep
// this caption at its default (true) - the only place a candidate's artist is still
// shown at all for that grid.
showArtistCaption: boolean = true
) => (
<CandidateButton
key={candidate.identifier}
Expand Down Expand Up @@ -1478,7 +1492,7 @@ export function QuestionFeed() {
{candidate.expansionCode.toUpperCase()}{" "}
{candidate.collectorNumber}
</div>
<div className="cs">{candidate.artist}</div>
{showArtistCaption && <div className="cs">{candidate.artist}</div>}
</CandidateCaption>
</CandidateButton>
);
Expand Down Expand Up @@ -1594,29 +1608,48 @@ export function QuestionFeed() {
</Btn>
</div>
)}
{illustrationGroups.map((group) => (
<IllustrationGroup
key={group[0].illustrationId}
data-testid="question-feed-illustration-group"
data-illustration-id={group[0].illustrationId}
>
<IllustrationGroupLabel>
Same illustration - {group.length} printings
</IllustrationGroupLabel>
<CandidateGrid>
{group.map((candidate) =>
renderCandidateTile(candidate, () =>
// every member of `group` shares this non-null illustrationId - see the
// grouping logic above, which only clusters candidates that have one.
selectIllustrationGroup(
candidate.illustrationId as string,
candidate
)
)
{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() !== "");
return (
<IllustrationGroup
key={group[0].illustrationId}
data-testid="question-feed-illustration-group"
data-illustration-id={group[0].illustrationId}
>
<IllustrationGroupLabel>
Same illustration - {group.length} printings
</IllustrationGroupLabel>
{illustrationArtist != null && (
<IllustrationCredit data-testid="question-feed-illustration-credit">
<div className="text-muted small mb-1">
Illustration by {illustrationArtist}
</div>
<ArtistSupportLink artistName={illustrationArtist} />
</IllustrationCredit>
)}
</CandidateGrid>
</IllustrationGroup>
))}
<CandidateGrid>
{group.map((candidate) =>
renderCandidateTile(
candidate,
() =>
// every member of `group` shares this non-null illustrationId - see the
// grouping logic above, which only clusters candidates that have one.
selectIllustrationGroup(
candidate.illustrationId as string,
candidate
),
false
)
)}
</CandidateGrid>
</IllustrationGroup>
);
})}
{ungroupedCandidates.length > 0 && (
<CandidateGrid data-testid="question-feed-candidate-grid-ungrouped">
{ungroupedCandidates.map((candidate) =>
Expand Down
Loading