Skip to content

ArtistSupportLink: drop redundant adjacent name labels, nest disclosure inside the button (#747) #753

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 6, 2026
Merged

ArtistSupportLink: drop redundant adjacent name labels, nest disclosure inside the button (#747) #753

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
147 changes: 80 additions & 67 deletions frontend/src/components/ArtistSupportLink.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,18 @@ const LINK_TYPE_LABELS: Record<string, string> = {
instagram: "Instagram",
};

// The button shell - wraps the collapsed row and (once expanded) the disclosure panel, so the
// panel renders within the same widget boundary as the link/toggle rather than as an unrelated
// sibling block below it (issue #747).
const CompactLine = styled.div`
display: flex;
flex-direction: column;
min-width: 0;
`;

// The collapsed row - primary link + toggle - never wraps to a second line, regardless of how
// narrow the caller's container is (the question feed's illustration credit caps it at 220px).
const CompactLine = styled.div`
const CompactLineRow = styled.div`
display: flex;
align-items: center;
gap: 6px;
Expand Down Expand Up @@ -149,76 +158,80 @@ export function ArtistSupportLink({
data-testid="artist-support-applet"
>
<CompactLine>
<CompactLink
href={pageHref}
target="_blank"
rel="noopener noreferrer"
title={`via ${MTGArtistConnection}`}
className="btn btn-primary btn-sm"
data-testid="artist-support-link"
>
<CompactLinkLabel>{artistName}</CompactLinkLabel>
<Icon bootstrapIconName="box-arrow-up-right" />
</CompactLink>
<ExpandToggle
type="button"
onClick={() => setExpanded((previous) => !previous)}
aria-expanded={expanded}
aria-controls={expandedPanelId}
data-testid="artist-support-toggle"
>
<Icon bootstrapIconName={expanded ? "chevron-up" : "chevron-down"} />
<span className="visually-hidden">
{expanded ? "Hide artist links" : "Show more artist links"}
</span>
</ExpandToggle>
</CompactLine>
{expanded && (
<div id={expandedPanelId} data-testid="artist-support-expanded">
{commerceLinks.length > 0 && (
<CompactLineRow>
<CompactLink
href={pageHref}
target="_blank"
rel="noopener noreferrer"
title={`via ${MTGArtistConnection}`}
className="btn btn-primary btn-sm"
data-testid="artist-support-link"
>
<CompactLinkLabel>{artistName}</CompactLinkLabel>
<Icon bootstrapIconName="box-arrow-up-right" />
</CompactLink>
<ExpandToggle
type="button"
onClick={() => setExpanded((previous) => !previous)}
aria-expanded={expanded}
aria-controls={expandedPanelId}
data-testid="artist-support-toggle"
>
<Icon
bootstrapIconName={expanded ? "chevron-up" : "chevron-down"}
/>
<span className="visually-hidden">
{expanded ? "Hide artist links" : "Show more artist links"}
</span>
</ExpandToggle>
</CompactLineRow>
{expanded && (
<div id={expandedPanelId} data-testid="artist-support-expanded">
{commerceLinks.length > 0 && (
<div
className="d-grid gap-2 mt-2"
data-testid="artist-support-commerce-links"
>
{commerceLinks.map((link) => (
<a
key={link.type}
href={link.url}
target="_blank"
rel="noopener noreferrer"
className="btn btn-outline-primary btn-sm w-100"
data-testid="artist-support-commerce-link"
data-link-type={link.type}
>
{LINK_TYPE_LABELS[link.type] ?? link.type}{" "}
<Icon bootstrapIconName="box-arrow-up-right" />
</a>
))}
</div>
)}
{hasSignatureService && (
<span
className="badge text-bg-secondary mt-2"
data-testid="artist-support-signature-badge"
>
<Icon bootstrapIconName="pen" /> Mark&apos;s Signature Service
</span>
)}
<div
className="d-grid gap-2 mt-2"
data-testid="artist-support-commerce-links"
className="text-muted small mt-1"
data-testid="artist-support-credit"
>
{commerceLinks.map((link) => (
<a
key={link.type}
href={link.url}
target="_blank"
rel="noopener noreferrer"
className="btn btn-outline-primary btn-sm w-100"
data-testid="artist-support-commerce-link"
data-link-type={link.type}
>
{LINK_TYPE_LABELS[link.type] ?? link.type}{" "}
<Icon bootstrapIconName="box-arrow-up-right" />
</a>
))}
Source:{" "}
<a
href={MTGArtistConnectionHomepageURL}
target="_blank"
rel="noopener noreferrer"
>
{MTGArtistConnection}
</a>
</div>
)}
{hasSignatureService && (
<span
className="badge text-bg-secondary mt-2"
data-testid="artist-support-signature-badge"
>
<Icon bootstrapIconName="pen" /> Mark&apos;s Signature Service
</span>
)}
<div
className="text-muted small mt-1"
data-testid="artist-support-credit"
>
Source:{" "}
<a
href={MTGArtistConnectionHomepageURL}
target="_blank"
rel="noopener noreferrer"
>
{MTGArtistConnection}
</a>
</div>
</div>
)}
)}
</CompactLine>
</div>
);
}
19 changes: 6 additions & 13 deletions frontend/src/features/display/ArtistSection.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
* page link, the MTGAC credit line) rather than a single caller-styled `<a>` wrapping caller-
* supplied `children` - this caller no longer passes `className`/children for button styling,
* the applet owns its own layout ("buttons stretch to fill the applet", owner instruction). The
* plain credit line ("Art by <Name>") stays separate, non-linked text above the applet.
* applet's own page-link button already names the artist, so this caller renders no separate
* name label alongside it (issue #747).
*
* Upstream-divergence note (docs/upstreaming/readiness-audit.md's styling-divergence ledger):
* chilli-axe/mpc-autofill has no artist-support surface at all - ArtistSupportLink is a
Expand Down Expand Up @@ -39,18 +40,10 @@ export function ArtistSection({ cardDocument }: ArtistSectionProps) {
return (
<div data-testid="display-artist-section">
{cardDocument.canonicalArtist != null ? (
<>
<div className="by mb-1">
Art by{" "}
<span className="fw-semibold">
{cardDocument.canonicalArtist.name}
</span>
</div>
<ArtistSupportLink
artistName={cardDocument.canonicalArtist.name}
defaultExpanded
/>
</>
<ArtistSupportLink
artistName={cardDocument.canonicalArtist.name}
defaultExpanded
/>
) : (
<span className="text-muted">Unknown</span>
)}
Expand Down
10 changes: 2 additions & 8 deletions frontend/src/features/questionFeed/QuestionFeed.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -747,8 +747,8 @@ export function QuestionFeed() {
const [fetchToken, setFetchToken] = useState<number>(0);
// Artist Support Links v1 - set once the user casts a real (non-"Unknown") artist vote on an
// "artist"-type item, via ArtistVotePicker's onArtistConfirmed below. Drives the post-answer
// "Art by <Name> - support them" banner - reset on every new item alongside the other
// per-question state, so it can't bleed into the next question.
// ArtistSupportLink banner - reset on every new item alongside the other per-question state,
// so it can't bleed into the next question.
const [confirmedArtistName, setConfirmedArtistName] = useState<string | null>(
null
);
Expand Down Expand Up @@ -1711,9 +1711,6 @@ export function QuestionFeed() {
</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>
)}
Expand Down Expand Up @@ -1869,9 +1866,6 @@ export function QuestionFeed() {
/>
{confirmedArtistName != null && (
<div className="mt-2" data-testid="question-feed-artist-support">
<div className="text-muted small mb-1">
Art by {confirmedArtistName} - support them
</div>
<ArtistSupportLink artistName={confirmedArtistName} />
</div>
)}
Expand Down
10 changes: 1 addition & 9 deletions frontend/tests/DisplayPage.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -928,7 +928,7 @@ test.describe("DisplayPage (Proposal H, Step 1)", () => {
await expect(select).toHaveValue("force-bleed");
});

test("the promoted artist line shows a plain credit + the MTG Artist Connection applet (page-link button, credit line) for a card with a known canonical artist (M2 applet round, docs/features/artist-support-links.md)", async ({
test("the promoted artist line shows the MTG Artist Connection applet (page-link button, credit line) for a card with a known canonical artist (M2 applet round, docs/features/artist-support-links.md)", async ({
page,
network,
}) => {
Expand All @@ -943,14 +943,6 @@ test.describe("DisplayPage (Proposal H, Step 1)", () => {
await importTextOnEditorLanding(page, "1 card 8 (xyz) 001");
await page.getByTestId("page-preview-slot").first().click();

// Plain (non-linked) credit line names the artist.
await expect(page.getByTestId("display-artist-section")).toContainText(
"Art by"
);
await expect(page.getByTestId("display-artist-section")).toContainText(
"Alpha Artist"
);

// M2 round: ArtistSupportLink is now a self-contained applet (RTK Query fetch, no
// caller-supplied children/className for button styling - see ArtistSupportLink.tsx's own
// module docstring). The primary page-link button now names the ARTIST (the applet's own
Expand Down
10 changes: 4 additions & 6 deletions frontend/tests/QuestionFeed.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -879,9 +879,9 @@ test.describe("question feed - artist question type", () => {
).toBeVisible();
});

// Artist Support Links v1 - the post-answer moment ("Art by <Name> - support them"), a
// zero-crawl link-out to MTG Artist Connection built deterministically from the artist name
// the user just voted for. See docs/features/artist-support-links.md.
// Artist Support Links v1 - the post-answer moment, a zero-crawl link-out to MTG Artist
// Connection built deterministically from the artist name the user just voted for. See
// docs/features/artist-support-links.md.
test("voting for a named artist shows the Artist Support Link, built from that artist's name", async ({
page,
network,
Expand All @@ -907,10 +907,8 @@ test.describe("question feed - artist question type", () => {

const banner = page.getByTestId("question-feed-artist-support");
await expect(banner).toBeVisible();
await expect(banner).toContainText(
`Art by ${canonicalArtist1.name} - support them`
);
const link = banner.getByTestId("artist-support-link");
await expect(link).toContainText(canonicalArtist1.name);
await expect(link).toHaveAttribute(
"href",
`https://www.mtgartistconnection.com/artist/${encodeURIComponent(
Expand Down