From 24db6d587799e627da0e734b285757c699374962 Mon Sep 17 00:00:00 2001 From: Yixin Tian Date: Fri, 30 Oct 2026 22:27:36 -0400 Subject: [PATCH] Make Share the leading filled action and explain rejected shares Share creation fell from about 20% to 3.7% of new generations when the Oct 12 release made Share a small secondary button after Copy. Share is now the compact filled button placed first; Copy stays secondary and the privacy notice is unchanged. Rejected shares now show readable messages instead of raw details or "[object Object]", and phones keep the short label while a link is being created so the toolbar no longer scrolls sideways at 320px. --- client/e2e/learning-path-feedback.e2e.js | 58 ++++++++++++++++++- .../src/pages/LearningPath/LearningPath.jsx | 58 ++++++++++++------- .../src/pages/LearningPath/LearningPath.scss | 9 ++- client/src/util/shareSnapshot.js | 15 +++++ client/src/util/shareSnapshot.test.js | 29 +++++++++- 5 files changed, 143 insertions(+), 26 deletions(-) diff --git a/client/e2e/learning-path-feedback.e2e.js b/client/e2e/learning-path-feedback.e2e.js index 704aab6..7c90c3b 100644 --- a/client/e2e/learning-path-feedback.e2e.js +++ b/client/e2e/learning-path-feedback.e2e.js @@ -190,7 +190,7 @@ test("only concept headings drag; summaries show details without reordering", as ); }); -test("share action stays compact and the share link opens a new tab", async ({ +test("share leads as a compact filled action and its link opens a new tab", async ({ page, }) => { await openGenerated(page); @@ -198,7 +198,15 @@ test("share action stays compact and the share link opens a new tab", async ({ route.fulfill({ json: { share_id: "abcdefghijklmnopqrstuv" } }), ); const button = page.getByRole("button", { name: "Share path", exact: true }); - expect((await button.boundingBox()).width).toBeLessThan(200); + const copy = page.getByRole("button", { name: "Copy path", exact: true }); + await expect(button).toHaveClass(/primary-button/); + await expect(copy).toHaveClass(/secondary-button/); + const shareBox = await button.boundingBox(); + expect(shareBox.width).toBeLessThan(200); + expect(shareBox.x).toBeLessThan((await copy.boundingBox()).x); + await expect(button).toHaveAccessibleDescription( + "Anyone with the link can view a copy of this path.", + ); await button.click(); const link = page.getByRole("link", { name: /Open share link/ }); await expect(link).toHaveAttribute("target", "_blank"); @@ -211,6 +219,28 @@ test("share action stays compact and the share link opens a new tab", async ({ await popup.close(); }); +test("a rejected share explains itself instead of showing raw errors", async ({ + page, +}) => { + await openGenerated(page); + await page.route("**/v1/shares", (route) => + route.fulfill({ + status: 422, + json: { + detail: [{ loc: ["body", "graph"], msg: "Invalid", type: "value_error" }], + }, + }), + ); + await page.getByRole("button", { name: "Share path", exact: true }).click(); + await expect( + page.getByText("This path can't be shared. Use Copy to save it instead."), + ).toBeVisible(); + await expect(page.getByText("[object Object]")).toHaveCount(0); + await expect( + page.getByRole("button", { name: "Share path", exact: true }), + ).toBeEnabled(); +}); + for (const width of [320, 375, 390]) { test(`nav and path controls each fit one row on a ${width}px phone`, async ({ page, @@ -243,5 +273,29 @@ for (const width of [320, 375, 390]) { expect( await page.evaluate(() => document.documentElement.scrollWidth), ).toBe(width); + + // The row still fits while the share link is being created. + let release; + const held = new Promise((resolve) => { + release = resolve; + }); + await page.route("**/v1/shares", async (route) => { + await held; + await route.fulfill({ json: { share_id: "abcdefghijklmnopqrstuv" } }); + }); + await page.getByRole("button", { name: "Share path", exact: true }).click(); + await expect( + page.getByRole("button", { name: "Creating link...", exact: true }), + ).toBeDisabled(); + expect( + await rowCount(".view-toggle, .copy-action, .share-action"), + ).toBeLessThan(4); + expect( + await page.evaluate(() => document.documentElement.scrollWidth), + ).toBe(width); + release(); + await expect( + page.getByRole("link", { name: /Open share link/ }), + ).toBeVisible(); }); } diff --git a/client/src/pages/LearningPath/LearningPath.jsx b/client/src/pages/LearningPath/LearningPath.jsx index 72f5520..50d02fb 100644 --- a/client/src/pages/LearningPath/LearningPath.jsx +++ b/client/src/pages/LearningPath/LearningPath.jsx @@ -30,7 +30,10 @@ import { apiUrl } from "../../config/api"; import { findPublicTopicByAlias } from "../../data/publicTopics"; import Seo from "../../seo/Seo"; import NetworkGraph from "./NetworkGraph"; -import { createShareSnapshot } from "../../util/shareSnapshot"; +import { + createShareSnapshot, + shareErrorMessage, +} from "../../util/shareSnapshot"; const LEVEL_ORDER = ["Beginner", "Intermediate", "Advanced"]; const MODERATION_DETAIL_HINTS = ["content policy", "moderation", "flagged"]; @@ -172,8 +175,10 @@ export default function LearningPath() { ), }); const body = await response.json().catch(() => ({})); - if (!response.ok || !body.share_id) - throw new Error(body.detail || "Unable to create share."); + if (!response.ok || !body.share_id) { + enqueueSnackbar(shareErrorMessage(response.status, body)); + return; + } const createdUrl = `${window.location.origin}/share/${body.share_id}`; setShareUrl(createdUrl); if (navigator.clipboard?.writeText) { @@ -188,8 +193,10 @@ export default function LearningPath() { } else { enqueueSnackbar("Share created. Copy the link shown below."); } - } catch (error) { - enqueueSnackbar(error.message || "Unable to create share."); + } catch { + enqueueSnackbar( + "Couldn't reach the server. Check your connection and try again.", + ); } finally { setIsSharing(false); } @@ -331,28 +338,21 @@ export default function LearningPath() { role="group" aria-label="Learning path actions" > -