From dc8c269b42e5f1938add36010d265cbc4bd96efc Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Thu, 6 Aug 2026 02:32:38 +0000 Subject: [PATCH] ArtistSupportLink: collapse to a single line by default, expand on demand The applet used to always stack a page-link button, up to five commerce buttons, a signature badge, and a credit line - never fewer than 2 rows, up to ~8. It's a same-origin link-out to our own cache-only backend endpoint, not a third-party embed. Default render is now one line: the artist page link plus a disclosure toggle. Commerce links, the signature badge, and the MTG Artist Connection credit move into a panel that mounts only once expanded - every link that existed before stays reachable, just not paid for in vertical space by default. The /editor rail's Artist section (ArtistSection.tsx) starts expanded via a new optional defaultExpanded prop: that surface is a dedicated accordion pane with room for the full applet, and its own Playwright coverage already asserts the credit line renders without interaction. Every other caller (question feed's inline credits, the card-detail modal's metadata table) is space-constrained next to whatever the surface is actually about, so it defaults collapsed. QuestionFeed.tsx and CardDetailedViewBody.tsx need no changes at all - both already call ArtistSupportLink with only artistName, which now composes correctly. --- .../src/components/ArtistSupportLink.test.tsx | 79 ++++++- frontend/src/components/ArtistSupportLink.tsx | 192 ++++++++++++------ .../src/features/display/ArtistSection.tsx | 5 +- frontend/tests/QuestionFeed.spec.ts | 56 +++++ 4 files changed, 258 insertions(+), 74 deletions(-) diff --git a/frontend/src/components/ArtistSupportLink.test.tsx b/frontend/src/components/ArtistSupportLink.test.tsx index a5f4c83a2..6e68a113f 100644 --- a/frontend/src/components/ArtistSupportLink.test.tsx +++ b/frontend/src/components/ArtistSupportLink.test.tsx @@ -1,4 +1,4 @@ -import { render, screen, waitFor } from "@testing-library/react"; +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; import React from "react"; import { Provider } from "react-redux"; @@ -18,15 +18,28 @@ import { setupStore } from "@/store/store"; import { ArtistSupportLink, buildArtistSupportURL } from "./ArtistSupportLink"; -function renderApplet(artistName: string, backend = localBackend) { +function renderApplet( + artistName: string, + backend = localBackend, + defaultExpanded = false +) { const store = setupStore({ backend }); render( - + ); } +// Issue #709 - the applet defaults to collapsed (a single line: the page-link + a disclosure +// toggle); this expands it so tests can assert on the commerce links/badge/credit it reveals. +function expandApplet() { + fireEvent.click(screen.getByTestId("artist-support-toggle")); +} + describe("buildArtistSupportURL", () => { it("URL-encodes the artist name into an MTG Artist Connection artist-page URL", () => { expect(buildArtistSupportURL("Harold McNeill")).toBe( @@ -42,7 +55,7 @@ describe("buildArtistSupportURL", () => { }); describe("ArtistSupportLink applet", () => { - it("never renders as an empty box: the MTGAC page link and the credit are present before any network response resolves", () => { + it("never renders as an empty box: the MTGAC page link is present before any network response resolves, collapsed to a single line by default", () => { // no server.use(...) at all - the request is in flight (or, with noBackend below, never // even fires) - the applet's base shape must already be there, not a spinner/empty state. renderApplet("Harold McNeill"); @@ -52,9 +65,10 @@ describe("ArtistSupportLink applet", () => { "href", buildArtistSupportURL("Harold McNeill") ); - expect(screen.getByTestId("artist-support-credit")).toHaveTextContent( - "MTG Artist Connection" - ); + expect(screen.getByTestId("artist-support-toggle")).toBeInTheDocument(); + expect( + screen.queryByTestId("artist-support-credit") + ).not.toBeInTheDocument(); expect( screen.queryByTestId("artist-support-commerce-links") ).not.toBeInTheDocument(); @@ -63,7 +77,36 @@ describe("ArtistSupportLink applet", () => { ).not.toBeInTheDocument(); }); - it("with no remote backend configured, still renders the fallback applet and makes no request at all", () => { + it("expanding the collapsed applet reveals the credit line, even with nothing else to show", () => { + renderApplet("Harold McNeill"); + + expandApplet(); + + expect(screen.getByTestId("artist-support-credit")).toHaveTextContent( + "MTG Artist Connection" + ); + }); + + it("collapsing again after expanding hides the credit line without unmounting the applet", () => { + renderApplet("Harold McNeill"); + + expandApplet(); + expect(screen.getByTestId("artist-support-credit")).toBeInTheDocument(); + + expandApplet(); + expect( + screen.queryByTestId("artist-support-credit") + ).not.toBeInTheDocument(); + expect(screen.getByTestId("artist-support-applet")).toBeInTheDocument(); + }); + + it("defaultExpanded starts the applet open, with the credit line visible without any interaction (the /editor rail's usage)", () => { + renderApplet("Harold McNeill", localBackend, true); + + expect(screen.getByTestId("artist-support-credit")).toBeInTheDocument(); + }); + + it("with no remote backend configured, still renders the fallback link and makes no request at all", () => { // No server.use(...) either - if a request were made, MSW's onUnhandledRequest: "error" // config would fail this test, so a clean pass here IS the "no request" assertion. renderApplet("Harold McNeill", noBackend); @@ -72,6 +115,8 @@ describe("ArtistSupportLink applet", () => { "href", buildArtistSupportURL("Harold McNeill") ); + + expandApplet(); expect(screen.getByTestId("artist-support-credit")).toBeInTheDocument(); }); @@ -85,6 +130,7 @@ describe("ArtistSupportLink applet", () => { buildArtistSupportURL("Harold McNeill") ) ); + expandApplet(); expect( screen.queryByTestId("artist-support-commerce-links") ).not.toBeInTheDocument(); @@ -94,7 +140,7 @@ describe("ArtistSupportLink applet", () => { expect(screen.getByTestId("artist-support-credit")).toBeInTheDocument(); }); - it("zero commerce links (found: true): still renders the MTGAC page link and the credit, no commerce buttons, no empty box", async () => { + it("zero commerce links (found: true): still renders the MTGAC page link and (once expanded) the credit, no commerce buttons, no empty box", async () => { server.use(artistExternalLinksZeroLinks); renderApplet(ArtistExternalLinksTestArtists.zeroLinks); @@ -106,6 +152,7 @@ describe("ArtistSupportLink applet", () => { )}` ) ); + expandApplet(); expect( screen.queryByTestId("artist-support-commerce-links") ).not.toBeInTheDocument(); @@ -137,10 +184,15 @@ describe("ArtistSupportLink applet", () => { ); }); - it("exactly one commerce link renders one stretched button", async () => { + it("exactly one commerce link renders one stretched button, reachable via the expand toggle", async () => { server.use(artistExternalLinksOneLink); renderApplet(ArtistExternalLinksTestArtists.oneLink); + await waitFor(() => + expect(screen.getByTestId("artist-support-toggle")).toBeInTheDocument() + ); + expandApplet(); + const links = await screen.findAllByTestId("artist-support-commerce-link"); expect(links).toHaveLength(1); expect(links[0]).toHaveAttribute( @@ -153,6 +205,7 @@ describe("ArtistSupportLink applet", () => { it("full row (5 commerce links) renders in the fixed priority order, capped at 5", async () => { server.use(artistExternalLinksFullRow); renderApplet(ArtistExternalLinksTestArtists.fullRow); + expandApplet(); const links = await screen.findAllByTestId("artist-support-commerce-link"); expect(links).toHaveLength(5); @@ -168,6 +221,7 @@ describe("ArtistSupportLink applet", () => { it("an artist whose only link is instagram surfaces it (the 157-artist rescue scenario)", async () => { server.use(artistExternalLinksInstagramOnly); renderApplet(ArtistExternalLinksTestArtists.instagramOnly); + expandApplet(); const links = await screen.findAllByTestId("artist-support-commerce-link"); expect(links).toHaveLength(1); @@ -178,6 +232,7 @@ describe("ArtistSupportLink applet", () => { it("the Mark's Signature Service flag renders as a badge, never as a link", async () => { server.use(artistExternalLinksWithSignatureBadge); renderApplet(ArtistExternalLinksTestArtists.signatureService); + expandApplet(); const badge = await screen.findByTestId("artist-support-signature-badge"); expect(badge.tagName).toBe("SPAN"); @@ -193,9 +248,10 @@ describe("ArtistSupportLink applet", () => { } }); - it("the MTG Artist Connection credit is always present and links to their homepage", async () => { + it("the MTG Artist Connection credit is always reachable and links to their homepage", async () => { server.use(artistExternalLinksOneLink); renderApplet(ArtistExternalLinksTestArtists.oneLink); + expandApplet(); const credit = await screen.findByTestId("artist-support-credit"); expect(credit).toHaveTextContent("MTG Artist Connection"); @@ -214,6 +270,7 @@ describe("ArtistSupportLink applet", () => { expect(primaryLink).toHaveAttribute("target", "_blank"); expect(primaryLink).toHaveAttribute("rel", "noopener noreferrer"); + expandApplet(); const commerceLink = await screen.findByTestId( "artist-support-commerce-link" ); diff --git a/frontend/src/components/ArtistSupportLink.tsx b/frontend/src/components/ArtistSupportLink.tsx index c0afff16c..5e9d8704f 100644 --- a/frontend/src/components/ArtistSupportLink.tsx +++ b/frontend/src/components/ArtistSupportLink.tsx @@ -13,33 +13,30 @@ * still-loading request - are treated identically here: fall back to the deterministic * `buildArtistSupportURL` construction. **Never a broken or empty state.** * + * **Compact by default, expandable** (issue #709 - the applet used to always stack the page-link + * button, up to five commerce buttons, a badge, and a credit line, up to ~8 rows deep next to + * whatever the surface was actually about). The default render is ONE line: the artist page link + * plus a small disclosure toggle. Commerce links, the signature badge, and the MTGAC credit line + * move into a panel that only mounts once the user opts in - every link that existed before is + * still reachable, just not always paid for in vertical space. + * * **Design target: zero and one commerce links, not four or five.** Measured against the real * 2,389-artist export: 812 have zero commerce links, 818 have exactly one, only 13 have all five * (instagram - added later as a last-resort exception, not a commerce field - rescues 157 of * those 812 down to 655; ~598 have nothing but the MTGAC page link regardless of what's - * allowlisted). This applet ALWAYS renders the MTGAC page link and the "Source: MTG Artist - * Connection" credit (an obligation, not decoration - site credits were part of the MTGAC - * partnership) - commerce buttons are purely additive on top of that stable base, so the ~69% of - * artists with zero or one link see a small, correctly-proportioned applet, not an empty box or - * a layout built for a row of five buttons that's usually mostly missing. - * - * **No layout shift while loading**: the loading state and the "found: false"/zero-commerce-link - * state render IDENTICALLY (MTGAC page link + credit only, no commerce buttons, no badge). - * Commerce buttons and the signature-service badge are added on top once data confirms they - * exist - for a third of all artists that's not a shift at all, since the loaded state IS the - * base shape; for the rest, buttons appear below the stable base rather than a spinner/skeleton - * collapsing into a completely different-shaped final layout. - * - * **Buttons stretch to fill the applet** (owner instruction) - every rendered ``/button here - * is full-width within whatever container the caller gives it, so the applet reads consistently - * whether that container is a narrow table cell or a full-width rail panel. + * allowlisted). The "Source: MTG Artist Connection" credit (an obligation, not decoration - site + * credits were part of the MTGAC partnership) is always reachable through the disclosure, whether + * or not this particular artist has any commerce links to go with it. * * Callers gate rendering on the artist being confirmed/known (the same precedence chain * `Card.serialise` exposes via `canonicalArtist`, or a vote just cast) - this component has no * opinion on that and never widens it; it only takes `artistName` (required, non-nullable) plus - * an optional `className` for the caller's own layout/spacing. + * an optional `className` for the caller's own layout/spacing, and an optional `defaultExpanded` + * for the one surface (the /editor rail's Artist section) that has room to show the full applet + * up front. */ -import React from "react"; +import styled from "@emotion/styled"; +import React, { useId, useState } from "react"; import { MTGArtistConnection, @@ -69,16 +66,68 @@ const LINK_TYPE_LABELS: Record = { instagram: "Instagram", }; +// 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` + display: flex; + align-items: center; + gap: 6px; + min-width: 0; +`; + +const CompactLink = styled.a` + display: inline-flex; + align-items: center; + gap: 4px; + min-width: 0; + flex: 1 1 auto; +`; + +// The artist name is the part that can be arbitrarily long, so ellipsis lives on this inner +// span rather than the flex anchor itself (text-overflow needs a single-line inline box, not a +// flex container with a sibling icon). +const CompactLinkLabel = styled.span` + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + min-width: 0; +`; + +const ExpandToggle = styled.button` + flex: 0 0 auto; + display: inline-flex; + align-items: center; + justify-content: center; + width: 28px; + height: 28px; + padding: 0; + background: transparent; + border: 1px solid var(--bs-border-color, currentColor); + border-radius: var(--r-btn, 4px); + color: inherit; + cursor: pointer; +`; + interface ArtistSupportLinkProps { artistName: string; className?: string; + /** The /editor rail's Artist section (ArtistSection.tsx) is a dedicated accordion pane with + * room for the full stacked applet, and its own Playwright coverage already asserts the credit + * line renders without any interaction - so that one caller opts into starting expanded. Every + * other caller (the question feed's inline credits, the card-detail modal's metadata table) is + * space-constrained next to whatever the surface is actually about, so `undefined`/`false` + * (the default) starts collapsed to a single line. */ + defaultExpanded?: boolean; } export function ArtistSupportLink({ artistName, className, + defaultExpanded = false, }: ArtistSupportLinkProps) { const { data } = useGetArtistExternalLinksQuery(artistName); + const [expanded, setExpanded] = useState(defaultExpanded); + const expandedPanelId = useId(); // Prefer MTGAC's own authoritative pageUrl when we have it - this is the actual point of the // applet, not a nice-to-have: 197/2,389 (8.2%) of this project's deterministically-constructed @@ -99,58 +148,77 @@ export function ArtistSupportLink({ className={["artist-support-applet", className].filter(Boolean).join(" ")} data-testid="artist-support-applet" > - - {artistName} - - {commerceLinks.length > 0 && ( -
+ - {commerceLinks.map((link) => ( + {artistName} + + + setExpanded((previous) => !previous)} + aria-expanded={expanded} + aria-controls={expandedPanelId} + data-testid="artist-support-toggle" + > + + + {expanded ? "Hide artist links" : "Show more artist links"} + + + + {expanded && ( +
+ {commerceLinks.length > 0 && ( +
+ {commerceLinks.map((link) => ( + + {LINK_TYPE_LABELS[link.type] ?? link.type}{" "} + + + ))} +
+ )} + {hasSignatureService && ( + + Mark's Signature Service + + )} +
+ Source:{" "} - {LINK_TYPE_LABELS[link.type] ?? link.type}{" "} - + {MTGArtistConnection} - ))} +
)} - {hasSignatureService && ( - - Mark's Signature Service - - )} -
- Source:{" "} - - {MTGArtistConnection} - -
); } diff --git a/frontend/src/features/display/ArtistSection.tsx b/frontend/src/features/display/ArtistSection.tsx index c85324ab2..e6cdca362 100644 --- a/frontend/src/features/display/ArtistSection.tsx +++ b/frontend/src/features/display/ArtistSection.tsx @@ -46,7 +46,10 @@ export function ArtistSection({ cardDocument }: ArtistSectionProps) { {cardDocument.canonicalArtist.name} - + ) : ( Unknown diff --git a/frontend/tests/QuestionFeed.spec.ts b/frontend/tests/QuestionFeed.spec.ts index 2012da6cf..edf7b50f6 100644 --- a/frontend/tests/QuestionFeed.spec.ts +++ b/frontend/tests/QuestionFeed.spec.ts @@ -341,6 +341,62 @@ test.describe("question feed - Level 2 illustration grouping", () => { ).toHaveAttribute("src", illustrationGroupCandidateC.mediumThumbnailUrl); }); + // Issue #709 - the illustration-credit ArtistSupportLink used to always stack the page-link + // button, up to five commerce buttons, a badge, and a credit line next to the question - up to + // ~8 rows. It now defaults to one collapsed line and expands on demand; the expansion must + // never cover the pinned reference image (SPEC-wtc-rebuild.md Amendment A2). + test("the illustration-credit Artist Support Link is compact by default and its expansion never overlaps the pinned reference image", async ({ + page, + network, + }) => { + network.use( + questionFeedIdentifyPrintingGroupedByIllustration, + ...defaultHandlers + ); + await loadPageWithDefaultBackend(page, "whatsthat"); + + const credit = page.getByTestId("question-feed-illustration-credit"); + const applet = credit.getByTestId("artist-support-applet"); + await expect(applet.getByTestId("artist-support-link")).toContainText( + "Some Artist" + ); + await expect(applet.getByTestId("artist-support-credit")).toHaveCount(0); + await expect( + applet.getByTestId("artist-support-commerce-links") + ).toHaveCount(0); + + // Collapsed: one line, well under the height a stacked applet (page link + credit, let + // alone commerce buttons) would need. + const collapsedBox = await applet.boundingBox(); + expect(collapsedBox).not.toBeNull(); + expect((collapsedBox as { height: number }).height).toBeLessThan(40); + + await applet.getByTestId("artist-support-toggle").click(); + await expect(applet.getByTestId("artist-support-credit")).toBeVisible(); + + const subjectBox = await page + .getByTestId("question-feed-subject-art") + .boundingBox(); + const expandedBox = await applet.boundingBox(); + expect(subjectBox).not.toBeNull(); + expect(expandedBox).not.toBeNull(); + expect( + boxesIntersect( + subjectBox as { x: number; y: number; width: number; height: number }, + expandedBox as { + x: number; + y: number; + width: number; + height: number; + } + ) + ).toBe(false); + + // Collapsing again hides the credit without unmounting the applet. + await applet.getByTestId("artist-support-toggle").click(); + await expect(applet.getByTestId("artist-support-credit")).toHaveCount(0); + }); + // Issue #503 (WTC phase C2) / #524 - supersedes this describe block's former "selecting a // grouped candidate submits the identical payload to the identical endpoint as an ungrouped // one" test (see .github/coverage-acks.txt for the rename ack). That title asserted phase