From ffa5bee89d26b217e54078bbf553ad11cc199653 Mon Sep 17 00:00:00 2001 From: William Laugesen Date: Wed, 19 Aug 2026 11:58:26 +1200 Subject: [PATCH 1/7] Turn "Copy as markdown" into a split button and delete "Use Octopus docs with AI" The "Open this page as markdown" link moves from the menu at the bottom of the article into a dropdown on the copy action, built on the shared SplitButton the "Open in Claude" button uses. "Open all docs as markdown" is dropped: llms-full.txt is too large to use with any current model. SplitButton now takes an optional href and a slotted primary label, so the copy button can stay a + ) } + + diff --git a/src/components/MarkdownLinks.astro b/src/components/MarkdownLinks.astro deleted file mode 100644 index 190653b662..0000000000 --- a/src/components/MarkdownLinks.astro +++ /dev/null @@ -1,57 +0,0 @@ ---- -import { Lang, Translations } from '@util/Languages'; -import { SITE } from '@config'; -import { getPageMarkdownPath } from '@util/mdxContent'; - -type Props = { - lang: string; -}; -const { lang } = Astro.props satisfies Props; - -const _ = Lang(lang); - -const pageMdUrl = getPageMarkdownPath(Astro.url.pathname); -const allDocsUrl = SITE.subfolder.replace(/\/$/, '') + '/llms-full.txt'; ---- - -{ - pageMdUrl && ( - - ) -} diff --git a/src/components/SplitButton.astro b/src/components/SplitButton.astro index 6210b6b5df..c10b9d27bc 100644 --- a/src/components/SplitButton.astro +++ b/src/components/SplitButton.astro @@ -11,8 +11,10 @@ export type SplitButtonItem = { }; type Props = HTMLAttributes<'div'> & { - label: string; - href: string | URL; + // Either a `label`, or markup in the default slot, as Button asks for + label?: string; + // Omitted when the primary acts on the page rather than navigating away + href?: string | URL; icon?: string; size?: 'xSmall' | 'small' | 'medium'; importance?: 'default' | 'loud'; @@ -20,6 +22,10 @@ type Props = HTMLAttributes<'div'> & { menuLabel: string; // accessible name for the caret, which has no text target?: string; rel?: string; + // The primary carries these rather than the wrapper, for a caller whose + // script or styles are hung off the button itself. + primaryClass?: string; + primaryData?: Record; }; const { @@ -32,6 +38,8 @@ const { menuLabel, target, rel, + primaryClass, + primaryData, class: className, ...rest } = Astro.props satisfies Props; @@ -40,6 +48,23 @@ if (items.length === 0) { throw new Error('A SplitButton with no menu items should just be a Button'); } +// Button's own version of this check cannot see through to the caller: the +// `` handed to it below counts as slotted markup whether or not anything +// was passed in here. +if (!label && !Astro.slots.has('default')) { + throw new Error('A SplitButton needs a label, or markup for one in its slot'); +} + +// Button types the two halves of its own union separately - an anchor needs an +// `href`, a plain button forbids one - so the link attributes are handed over as +// a set rather than as three possibly-undefined props. `target` and `rel` say +// where a link opens, and mean nothing on a button. +const primaryLink = href ? { href, target, rel } : {}; + +const primaryClasses = primaryClass + ? `split-btn__primary ${primaryClass}` + : 'split-btn__primary'; + const menuItems = items.map((item) => ({ ...item, kind: 'link' as const, @@ -50,15 +75,16 @@ const menuItems = items.map((item) => ({
-
diff --git a/src/scripts/main.js b/src/scripts/main.js index 3fa6d85c28..24eed5f812 100644 --- a/src/scripts/main.js +++ b/src/scripts/main.js @@ -4,7 +4,6 @@ import { addListImageIntersectionObserver, } from './modules/animation.js'; import { addResizedEvent } from './modules/resizing.js'; -import { markdownLinkMenus } from './modules/markdown-links.js'; import { setClickableBlocks } from './modules/click-blocks.js'; import { setExternalLinkAttributes } from './modules/external-links.js'; import { monitorInputType } from './modules/input-type.js'; diff --git a/src/scripts/modules/markdown-links.js b/src/scripts/modules/markdown-links.js deleted file mode 100644 index 65513fb386..0000000000 --- a/src/scripts/modules/markdown-links.js +++ /dev/null @@ -1,41 +0,0 @@ -// @ts-check -import { qs, qsa } from './query.js'; - -class MarkdownLinks { - constructor(menu) { - this.menu = menu; - this.trigger = qs('[data-md-links-trigger]', menu); - - this.addListeners(); - } - - handleKeyboardNavigation(e) { - if (!this.menu.open) return; - - if (e.key === 'Escape') { - e.preventDefault(); - this.menu.open = false; - this.trigger.focus(); - } - } - - handleOutsideClick(e) { - if (!this.menu.open) return; - if (e.target instanceof Node && this.menu.contains(e.target)) return; - this.menu.open = false; - } - - addListeners() { - this.menu.addEventListener('keydown', (e) => - this.handleKeyboardNavigation(e) - ); - - document.addEventListener('click', (e) => this.handleOutsideClick(e)); - } -} - -const markdownLinkMenus = Array.from(qsa('[data-md-links-menu]')).map( - (menu) => new MarkdownLinks(menu) -); - -export { markdownLinkMenus }; diff --git a/src/styles/main.css b/src/styles/main.css index 73e904b4eb..e912e04708 100644 --- a/src/styles/main.css +++ b/src/styles/main.css @@ -1582,159 +1582,6 @@ html[data-theme='dark'] img.card__icon { visibility: visible; } -/* "Use Octopus docs with AI" dropdown */ -.octo-md-links { - margin-block-start: var(--block-gap); -} - -.octo-md-links__menu { - display: inline-block; - position: relative; -} - -.octo-md-links__trigger { - display: inline-flex; - align-items: center; - gap: 0.5rem; - padding: 0.5rem 0.875rem; - border: 1px solid var(--border-color-menu-open); - border-radius: 999px; - background-color: transparent; - color: var(--color-menu-link); - font: inherit; - cursor: pointer; - list-style: none; - text-decoration: none; - user-select: none; - transition: - border-color 200ms cubic-bezier(0.4, 0, 0.2, 1), - background-color 200ms cubic-bezier(0.4, 0, 0.2, 1), - color 200ms cubic-bezier(0.4, 0, 0.2, 1), - box-shadow 200ms cubic-bezier(0.4, 0, 0.2, 1); -} - -.octo-md-links__trigger > * { - text-decoration: none; - color: inherit; -} - -.octo-md-links__trigger::-webkit-details-marker, -.octo-md-links__trigger::marker { - content: ''; - display: none; -} - -.octo-md-links__trigger-icon, -.octo-md-links__trigger-caret { - display: inline-flex; - align-items: center; - justify-content: center; - line-height: 1; -} - -.octo-md-links__trigger-icon::before { - content: '\f0eb'; /* fa-lightbulb */ - font-family: fa-solid; - font-size: 0.95em; - line-height: 1; - color: var(--color-menu-link-alt); - transition: color 200ms cubic-bezier(0.4, 0, 0.2, 1); -} - -.octo-md-links__trigger-caret::before { - content: '\f078'; /* fa-chevron-down */ - font-family: fa-solid; - font-size: 0.7em; - line-height: 1; - color: currentColor; - transition: transform 200ms cubic-bezier(0.4, 0, 0.2, 1); -} - -.octo-md-links__menu[open] - > .octo-md-links__trigger - .octo-md-links__trigger-caret::before { - transform: rotate(180deg); -} - -.octo-md-links__trigger:hover { - color: var(--color-menu-link-active); - border-color: var(--color-menu-link-alt); -} - -.octo-md-links__menu[open] > .octo-md-links__trigger { - color: var(--color-menu-link-active); - border-color: var(--color-menu-link-alt); - background-color: var(--bg-color-menu-open); - box-shadow: 0 0.0625rem 0.25rem rgba(13, 128, 216, 0.08); -} - -.octo-md-links__trigger:focus-visible { - outline: 2px solid var(--color-menu-link-alt); - outline-offset: 2px; -} - -.octo-md-links .octo-md-links__options { - position: absolute; - z-index: 10; - inset-block-start: calc(100% + 0.5rem); - inset-inline-start: 0; - margin: 0; - padding: 0.5rem; - list-style: none; - background-color: var(--bg-color-menu); - border: 1px solid var(--border-color-menu-open); - border-radius: 0.625rem; - min-width: 20rem; - box-sizing: border-box; - overflow: hidden; - box-shadow: - 0 0.625rem 1.875rem rgba(15, 37, 53, 0.12), - 0 0.125rem 0.375rem rgba(15, 37, 53, 0.06); -} - -.octo-md-links .octo-md-links__options .octo-md-links__option { - display: inline-flex; - align-items: center; - gap: 0.625rem; - padding: 0.625rem 0.75rem; - border: none; - border-radius: 0.375rem; - background: transparent; - color: var(--color-menu-link); - font: inherit; - text-align: start; - text-decoration: none; - cursor: pointer; - transition: - background-color 150ms cubic-bezier(0.4, 0, 0.2, 1), - color 150ms cubic-bezier(0.4, 0, 0.2, 1); -} - -.octo-md-links .octo-md-links__options .octo-md-links__option:hover, -.octo-md-links .octo-md-links__options .octo-md-links__option:focus-visible { - background-color: var(--bg-color-menu-open); - color: var(--color-menu-link-active); - outline: none; -} - -.octo-md-links__option::before { - font-family: fa-solid; - font-size: 0.95em; - width: 1.1em; - text-align: center; - color: var(--color-menu-link-alt); - flex-shrink: 0; - transition: color 150ms cubic-bezier(0.4, 0, 0.2, 1); -} - -.octo-md-links__option--view::before { - content: '\f15c'; /* fa-file-lines */ -} - -.octo-md-links__option--all::before { - content: '\f02d'; /* fa-book */ -} - /* Live regions for announcing the result of an action, such as copying a URL or a code block. Read by screen readers, never shown. */ .copy-status { diff --git a/tests/llm-endpoints.spec.ts b/tests/llm-endpoints.spec.ts index 2a07adb932..6e30961dda 100644 --- a/tests/llm-endpoints.spec.ts +++ b/tests/llm-endpoints.spec.ts @@ -124,8 +124,44 @@ test('markdown page actions are hidden on the ineligible MDX page', async ({ 'expected no copy action on ineligible page' ).toBe(0); expect( - await page.locator('[data-md-links-menu]').count(), - 'expected no markdown links dropdown on ineligible page' + await page.locator('.octo-copy-md-actions').count(), + 'expected no copy split button on ineligible page' + ).toBe(0); +}); + +// The "Open this page as markdown" link used to sit in a menu at the bottom of +// the article, alongside a link to llms-full.txt. It is in the copy action's +// menu now, and the llms-full.txt link is gone. +test('the copy action menu holds exactly one item, linking to the page markdown', async ({ + page, +}) => { + await page.goto(STABLE_PLAIN_MD_PATH); + + const menu = page.locator('.octo-copy-md-actions [data-menu]'); + await menu.locator('summary').click(); + + const items = menu.locator('.menu__action'); + await expect(items).toHaveCount(1); + await expect(items).toHaveAttribute('href', STABLE_PLAIN_MD_PATH + '.md'); + await expect(items).toHaveAttribute('target', '_blank'); +}); + +test('no page links to llms-full.txt', async ({ page }) => { + await page.goto(STABLE_PLAIN_MD_PATH); + expect( + await page.locator('a[href$="llms-full.txt"]').count(), + 'expected the llms-full.txt link to be gone from the docs UI' + ).toBe(0); +}); + +// What the deleted component rendered, in the position it rendered it. +test('nothing renders below the taxonomy where the markdown links menu was', async ({ + page, +}) => { + await page.goto(STABLE_PLAIN_MD_PATH); + expect( + await page.locator('.octo-md-links, [data-md-links-menu]').count(), + 'expected the "Use Octopus docs with AI" menu to be gone' ).toBe(0); }); From a0686065357c23bfdf6b12659abd01361738b9d2 Mon Sep 17 00:00:00 2001 From: William Laugesen Date: Wed, 19 Aug 2026 12:53:05 +1200 Subject: [PATCH 2/7] Drop the SplitButton label guard Both callers pass a label or slot markup, so the check guarded against a caller that does not exist. Co-Authored-By: Claude Opus 5 (1M context) --- src/components/SplitButton.astro | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/components/SplitButton.astro b/src/components/SplitButton.astro index c10b9d27bc..4c4b42df2b 100644 --- a/src/components/SplitButton.astro +++ b/src/components/SplitButton.astro @@ -48,13 +48,6 @@ if (items.length === 0) { throw new Error('A SplitButton with no menu items should just be a Button'); } -// Button's own version of this check cannot see through to the caller: the -// `` handed to it below counts as slotted markup whether or not anything -// was passed in here. -if (!label && !Astro.slots.has('default')) { - throw new Error('A SplitButton needs a label, or markup for one in its slot'); -} - // Button types the two halves of its own union separately - an anchor needs an // `href`, a plain button forbids one - so the link attributes are handed over as // a set rather than as three possibly-undefined props. `target` and `rel` say From f8a5d304dd7d2bd3f53d6d180a22cac2a5bae9b0 Mon Sep 17 00:00:00 2001 From: William Laugesen Date: Wed, 19 Aug 2026 14:22:08 +1200 Subject: [PATCH 3/7] Keep SplitButton's props as they were The copy half needs a data attribute and a state class on the button itself. Both now sit on the wrapping control instead, so SplitButton takes no new props: the copy listener matches the primary half and reads the URL from its ancestor, and the CSS and tests reach the button through .octo-copy-md .split-btn__primary. label goes back to required. The rest label comes from the prop and the two results are slotted, stacked with it in one grid cell so the button is still sized by the longest of the three. Co-Authored-By: Claude Opus 5 (1M context) --- src/components/CopyAsMarkdown.astro | 40 ++++++++++++--------------- src/components/SplitButton.astro | 16 ++--------- src/scripts/copy-as-markdown.ts | 11 ++++++-- src/styles/main.css | 42 +++++++++++++++++++---------- tests/copy-button.spec.ts | 7 ++--- tests/llm-endpoints.spec.ts | 28 +++++++++++++------ 6 files changed, 80 insertions(+), 64 deletions(-) diff --git a/src/components/CopyAsMarkdown.astro b/src/components/CopyAsMarkdown.astro index 5cb2b9e17a..06a855b80f 100644 --- a/src/components/CopyAsMarkdown.astro +++ b/src/components/CopyAsMarkdown.astro @@ -16,9 +16,9 @@ const pageMdUrl = getPageMarkdownPath(Astro.url.pathname); { pageMdUrl && ( - {/* All three states are rendered, stacked in one grid cell, so the button - is sized by its longest label and keeps that width as the state - changes. Only the current one is visible, and `visibility` keeps the - other two out of the accessible name. */} - - - {_(Translations.octopus_copy_md.label)} - - - {_(Translations.octopus_copy_md.copied)} - - - {_(Translations.octopus_copy_md.error)} - + {/* Both results are rendered, stacked with the rest label in one grid + cell, so the button is sized by the longest of the three and keeps + that width as the state changes. Only the current one is visible, and + `visibility` keeps the other two out of the accessible name. */} + + {_(Translations.octopus_copy_md.copied)} + + + {_(Translations.octopus_copy_md.error)} ) } diff --git a/src/components/SplitButton.astro b/src/components/SplitButton.astro index 4c4b42df2b..d62c5ebcfa 100644 --- a/src/components/SplitButton.astro +++ b/src/components/SplitButton.astro @@ -11,8 +11,7 @@ export type SplitButtonItem = { }; type Props = HTMLAttributes<'div'> & { - // Either a `label`, or markup in the default slot, as Button asks for - label?: string; + label: string; // Omitted when the primary acts on the page rather than navigating away href?: string | URL; icon?: string; @@ -22,10 +21,6 @@ type Props = HTMLAttributes<'div'> & { menuLabel: string; // accessible name for the caret, which has no text target?: string; rel?: string; - // The primary carries these rather than the wrapper, for a caller whose - // script or styles are hung off the button itself. - primaryClass?: string; - primaryData?: Record; }; const { @@ -38,8 +33,6 @@ const { menuLabel, target, rel, - primaryClass, - primaryData, class: className, ...rest } = Astro.props satisfies Props; @@ -54,10 +47,6 @@ if (items.length === 0) { // where a link opens, and mean nothing on a button. const primaryLink = href ? { href, target, rel } : {}; -const primaryClasses = primaryClass - ? `split-btn__primary ${primaryClass}` - : 'split-btn__primary'; - const menuItems = items.map((item) => ({ ...item, kind: 'link' as const, @@ -68,13 +57,12 @@ const menuItems = items.map((item) => ({
diff --git a/src/scripts/copy-as-markdown.ts b/src/scripts/copy-as-markdown.ts index 411e52cd97..790891ee74 100644 --- a/src/scripts/copy-as-markdown.ts +++ b/src/scripts/copy-as-markdown.ts @@ -25,7 +25,10 @@ function showLabelResult(button: HTMLElement, ok: boolean): string { // Handed over unresolved: the clipboard write starts while the page is still // downloading, which is what keeps the copy working in Safari. function pageMarkdown(button: HTMLElement): Promise | null { - const url = button.dataset.copyMdUrl; + // The URL sits on the split button wrapping the copy half, so that a click on + // the caret beside it resolves to the menu rather than to a copy. + const url = + button.closest('[data-copy-md-url]')?.dataset.copyMdUrl; if (!url) return null; return fetch(url).then((response) => { @@ -34,4 +37,8 @@ function pageMarkdown(button: HTMLElement): Promise | null { }); } -copyOnClick('[data-copy-md-url]', pageMarkdown, { show: showLabelResult }); +// Matched on the primary half rather than on the control, so that the caret does +// not copy the page on its way to opening the menu. +copyOnClick('[data-copy-md-url] .split-btn__primary', pageMarkdown, { + show: showLabelResult, +}); diff --git a/src/styles/main.css b/src/styles/main.css index e912e04708..6a8ee03703 100644 --- a/src/styles/main.css +++ b/src/styles/main.css @@ -1546,39 +1546,53 @@ html[data-theme='dark'] img.card__icon { mask: url('../assets/icons/github.svg') center / contain no-repeat; } -/* "Copy as markdown" page action */ +/* "Copy as markdown" page action. The copy half is the primary of a split + button, and the script marks that button with the state, so these hang off it + rather than off the wrapping control. */ .octo-copy-md__icon { background-color: var(--colorIconPrimary); mask: url('../assets/icons/copy.svg') center / contain no-repeat; } -.octo-copy-md[data-copied] .octo-copy-md__icon { +.octo-copy-md .split-btn__primary[data-copied] .octo-copy-md__icon { mask: url('../assets/icons/check.svg') center / contain no-repeat; } -/* Slotted, so button.css's scoped .btn__label padding does not reach it. */ -.octo-copy-md__labels { - display: grid; - padding-inline: var(--space4); - text-align: start; +/* The rest label and both results share one grid cell, so the button is sized by + the longest of the three and holds that width as the state changes. The icon + keeps a column of its own, which `.btn`'s flex layout gave it before. */ +.octo-copy-md .split-btn__primary { + display: inline-grid; + grid-template-columns: auto auto; + align-items: center; + justify-content: center; } -.octo-copy-md__label { +.octo-copy-md .split-btn__primary > .btn__icon { grid-area: 1 / 1; - visibility: hidden; +} + +.octo-copy-md .split-btn__primary > .btn__label, +.octo-copy-md .split-btn__primary > .octo-copy-md__label { + grid-area: 1 / 2; + text-align: start; white-space: nowrap; } -.octo-copy-md__label--rest { - visibility: visible; +/* Slotted, so button.css's scoped .btn__label padding does not reach these. */ +.octo-copy-md .split-btn__primary > .octo-copy-md__label { + padding-inline: var(--space4); + visibility: hidden; } -.octo-copy-md:is([data-copied], [data-failed]) .octo-copy-md__label--rest { +.octo-copy-md + .split-btn__primary:is([data-copied], [data-failed]) + > .btn__label { visibility: hidden; } -.octo-copy-md[data-copied] .octo-copy-md__label--copied, -.octo-copy-md[data-failed] .octo-copy-md__label--failed { +.octo-copy-md .split-btn__primary[data-copied] > .octo-copy-md__label--copied, +.octo-copy-md .split-btn__primary[data-failed] > .octo-copy-md__label--failed { visibility: visible; } diff --git a/tests/copy-button.spec.ts b/tests/copy-button.spec.ts index f9c508fd1c..9bb499fdce 100644 --- a/tests/copy-button.spec.ts +++ b/tests/copy-button.spec.ts @@ -64,7 +64,8 @@ test('the tooltip arrow overlaps the bubble it points from', async ({ const overlap = await button.evaluate((el) => { const bubble = el.querySelector('.tooltip'); - if (!bubble) throw new Error('expected the button to have a .tooltip bubble'); + if (!bubble) + throw new Error('expected the button to have a .tooltip bubble'); const caret = getComputedStyle(bubble, '::after'); const bubbleHeight = bubble.getBoundingClientRect().height; @@ -147,7 +148,7 @@ test('the copy action puts the page markdown on the clipboard', async ({ await page.goto(MD_PAGE); - const button = page.locator('.octo-copy-md'); + const button = page.locator('.octo-copy-md .split-btn__primary'); await button.click(); await expect(button).toHaveAttribute('data-copied', ''); @@ -172,7 +173,7 @@ test('the copy action reports a failure when the markdown cannot be fetched', as // Routed after the page has loaded, so only the copy's own fetch is refused. await page.route('**/*.md', (route) => route.abort()); - const button = page.locator('.octo-copy-md'); + const button = page.locator('.octo-copy-md .split-btn__primary'); await button.click(); await expect(button).toHaveAttribute('data-failed', ''); diff --git a/tests/llm-endpoints.spec.ts b/tests/llm-endpoints.spec.ts index 6e30961dda..50ae704524 100644 --- a/tests/llm-endpoints.spec.ts +++ b/tests/llm-endpoints.spec.ts @@ -96,8 +96,8 @@ test('Copy as markdown action advertises a working .md URL on the eligible page' expect(target.status()).toBe(200); }); -// All three labels are always in the DOM, stacked, so reporting a result must -// not reflow the page actions row. +// The rest label and both results are always in the DOM, stacked, so reporting +// a result must not reflow the page actions row. test('the copy action keeps its width while it reports a result', async ({ page, context, @@ -105,7 +105,7 @@ test('the copy action keeps its width while it reports a result', async ({ await context.grantPermissions(['clipboard-read', 'clipboard-write']); await page.goto(STABLE_PLAIN_MD_PATH); - const button = page.locator('.octo-copy-md'); + const button = page.locator('.octo-copy-md .split-btn__primary'); const before = await button.boundingBox(); await button.click(); @@ -123,10 +123,6 @@ test('markdown page actions are hidden on the ineligible MDX page', async ({ await page.locator('.octo-copy-md').count(), 'expected no copy action on ineligible page' ).toBe(0); - expect( - await page.locator('.octo-copy-md-actions').count(), - 'expected no copy split button on ineligible page' - ).toBe(0); }); // The "Open this page as markdown" link used to sit in a menu at the bottom of @@ -137,7 +133,7 @@ test('the copy action menu holds exactly one item, linking to the page markdown' }) => { await page.goto(STABLE_PLAIN_MD_PATH); - const menu = page.locator('.octo-copy-md-actions [data-menu]'); + const menu = page.locator('.octo-copy-md [data-menu]'); await menu.locator('summary').click(); const items = menu.locator('.menu__action'); @@ -146,6 +142,22 @@ test('the copy action menu holds exactly one item, linking to the page markdown' await expect(items).toHaveAttribute('target', '_blank'); }); +// The URL sits on the control while the copy listener matches the primary half. +// Matching the control instead would copy the page every time the menu opened. +test('opening the menu does not copy the page', async ({ page, context }) => { + await context.grantPermissions(['clipboard-read', 'clipboard-write']); + await page.goto(STABLE_PLAIN_MD_PATH); + + const control = page.locator('.octo-copy-md'); + await control.locator('summary').click(); + await expect(control.locator('.menu__list')).toBeVisible(); + + await expect(control.locator('.split-btn__primary')).not.toHaveAttribute( + 'data-copied', + '' + ); +}); + test('no page links to llms-full.txt', async ({ page }) => { await page.goto(STABLE_PLAIN_MD_PATH); expect( From 47d9c5866560c1f180193abd603e0c7a2b38d172 Mon Sep 17 00:00:00 2001 From: William Laugesen Date: Wed, 19 Aug 2026 14:48:28 +1200 Subject: [PATCH 4/7] Drop the failed-copy state from the markdown copy action Writing text to the clipboard is not something a reader can act on the failure of, and clipboard.js already logs whichever rung of the write chain refused. A failed copy now leaves the button as it was and announces nothing, so the button carries one label and one result rather than three stacked states. The heading and code-block copy buttons keep their own tooltip failure reporting. Co-Authored-By: Claude Opus 5 (1M context) --- src/components/CopyAsMarkdown.astro | 13 +++++------- src/data/language.json | 3 --- src/scripts/copy-as-markdown.ts | 30 +++++++++++----------------- src/styles/main.css | 31 +++++++++++++---------------- tests/copy-button.spec.ts | 8 ++++++-- 5 files changed, 37 insertions(+), 48 deletions(-) diff --git a/src/components/CopyAsMarkdown.astro b/src/components/CopyAsMarkdown.astro index 06a855b80f..3dd1cc6677 100644 --- a/src/components/CopyAsMarkdown.astro +++ b/src/components/CopyAsMarkdown.astro @@ -33,16 +33,13 @@ const pageMdUrl = getPageMarkdownPath(Astro.url.pathname); target="_blank" rel="noopener" > - {/* Both results are rendered, stacked with the rest label in one grid - cell, so the button is sized by the longest of the three and keeps - that width as the state changes. Only the current one is visible, and - `visibility` keeps the other two out of the accessible name. */} - + {/* Stacked with the label in one grid cell, so the button is sized by the + longer of the two and keeps that width while it reports the copy. + `visibility` is what hides it, which also keeps it out of the + accessible name until it is the one showing. */} + {_(Translations.octopus_copy_md.copied)} - - {_(Translations.octopus_copy_md.error)} - ) } diff --git a/src/data/language.json b/src/data/language.json index 52a8da45db..3b994a4592 100644 --- a/src/data/language.json +++ b/src/data/language.json @@ -52,9 +52,6 @@ "copied": { "en": "Copied" }, - "error": { - "en": "Copy failed" - }, "view_raw": { "en": "Open this page as markdown" }, diff --git a/src/scripts/copy-as-markdown.ts b/src/scripts/copy-as-markdown.ts index 790891ee74..d89a2d5612 100644 --- a/src/scripts/copy-as-markdown.ts +++ b/src/scripts/copy-as-markdown.ts @@ -1,24 +1,18 @@ import { copyOnClick, revertAfter } from './modules/clipboard.js'; -// Which label shows, and which glyph the icon masks, is CSS's decision - all -// three labels are in the DOM, and this only says which state the button is in. -function showLabelResult(button: HTMLElement, ok: boolean): string { - const state = ok ? 'copied' : 'failed'; +// Which label shows, and which glyph the icon masks, is CSS's decision - both +// labels are in the DOM, and this only says whether the copy landed. +// +// A copy that fails says nothing. Writing text to the clipboard is not something +// a reader can act on the failure of, and the chain in clipboard.js already logs +// whichever rung refused. +function showCopied(button: HTMLElement, ok: boolean): string { + if (!ok) return ''; - // Both are cleared every time, so a failure followed by a success inside the - // revert window does not leave the button wearing two states at once. - const rest = () => { - delete button.dataset.copied; - delete button.dataset.failed; - }; + button.dataset.copied = ''; + revertAfter(button, () => delete button.dataset.copied); - rest(); - button.dataset[state] = ''; - revertAfter(button, rest); - - const label = button.querySelector( - `.octo-copy-md__label--${state}` - ); + const label = button.querySelector('.octo-copy-md__copied'); return label?.textContent?.trim() ?? ''; } @@ -40,5 +34,5 @@ function pageMarkdown(button: HTMLElement): Promise | null { // Matched on the primary half rather than on the control, so that the caret does // not copy the page on its way to opening the menu. copyOnClick('[data-copy-md-url] .split-btn__primary', pageMarkdown, { - show: showLabelResult, + show: showCopied, }); diff --git a/src/styles/main.css b/src/styles/main.css index 6a8ee03703..4fb71a4899 100644 --- a/src/styles/main.css +++ b/src/styles/main.css @@ -1547,20 +1547,16 @@ html[data-theme='dark'] img.card__icon { } /* "Copy as markdown" page action. The copy half is the primary of a split - button, and the script marks that button with the state, so these hang off it - rather than off the wrapping control. */ + button, and the script marks that button once the copy lands, so everything + here hangs off it rather than off the control wrapping it. */ .octo-copy-md__icon { background-color: var(--colorIconPrimary); mask: url('../assets/icons/copy.svg') center / contain no-repeat; } -.octo-copy-md .split-btn__primary[data-copied] .octo-copy-md__icon { - mask: url('../assets/icons/check.svg') center / contain no-repeat; -} - -/* The rest label and both results share one grid cell, so the button is sized by - the longest of the three and holds that width as the state changes. The icon - keeps a column of its own, which `.btn`'s flex layout gave it before. */ +/* The label and the "Copied" it turns into share one grid cell, so the button is + sized by the longer of the two and holds that width while it reports. The icon + keeps a column of its own, as `.btn`'s flex layout gave it. */ .octo-copy-md .split-btn__primary { display: inline-grid; grid-template-columns: auto auto; @@ -1573,29 +1569,30 @@ html[data-theme='dark'] img.card__icon { } .octo-copy-md .split-btn__primary > .btn__label, -.octo-copy-md .split-btn__primary > .octo-copy-md__label { +.octo-copy-md__copied { grid-area: 1 / 2; text-align: start; white-space: nowrap; } -/* Slotted, so button.css's scoped .btn__label padding does not reach these. */ -.octo-copy-md .split-btn__primary > .octo-copy-md__label { +/* Slotted, so button.css's scoped .btn__label padding does not reach it. */ +.octo-copy-md__copied { padding-inline: var(--space4); visibility: hidden; } -.octo-copy-md - .split-btn__primary:is([data-copied], [data-failed]) - > .btn__label { +.octo-copy-md [data-copied] > .btn__label { visibility: hidden; } -.octo-copy-md .split-btn__primary[data-copied] > .octo-copy-md__label--copied, -.octo-copy-md .split-btn__primary[data-failed] > .octo-copy-md__label--failed { +.octo-copy-md [data-copied] > .octo-copy-md__copied { visibility: visible; } +.octo-copy-md [data-copied] .octo-copy-md__icon { + mask: url('../assets/icons/check.svg') center / contain no-repeat; +} + /* Live regions for announcing the result of an action, such as copying a URL or a code block. Read by screen readers, never shown. */ .copy-status { diff --git a/tests/copy-button.spec.ts b/tests/copy-button.spec.ts index 9bb499fdce..e4703dfd11 100644 --- a/tests/copy-button.spec.ts +++ b/tests/copy-button.spec.ts @@ -165,7 +165,9 @@ test('the copy action puts the page markdown on the clipboard', async ({ ); }); -test('the copy action reports a failure when the markdown cannot be fetched', async ({ +// A copy that fails says nothing, so the button has to stay as it was rather +// than claim it copied. +test('the copy action stays at rest when the markdown cannot be fetched', async ({ page, }) => { await page.goto(MD_PAGE); @@ -174,8 +176,10 @@ test('the copy action reports a failure when the markdown cannot be fetched', as await page.route('**/*.md', (route) => route.abort()); const button = page.locator('.octo-copy-md .split-btn__primary'); + const label = button.locator('.btn__label'); await button.click(); - await expect(button).toHaveAttribute('data-failed', ''); await expect(button).not.toHaveAttribute('data-copied', ''); + await expect(label).toBeVisible(); + await expect(page.locator('.copy-status')).toHaveText(''); }); From c78ed946786bb8c7d839f1fea77cc00a01d1e010 Mon Sep 17 00:00:00 2001 From: William Laugesen Date: Wed, 19 Aug 2026 15:07:31 +1200 Subject: [PATCH 5/7] Move the copy control's tests to the copy button spec The clipboard, the reporting width and the menu belong with the other copy buttons. llm-endpoints keeps what is about the endpoints themselves: the .md URL resolving, page eligibility, and llms-full.txt no longer being linked. The width test was already in llm-endpoints and moves across with them. Co-Authored-By: Claude Opus 5 (1M context) --- tests/copy-button.spec.ts | 48 ++++++++++++++++++++++++++++++++++ tests/llm-endpoints.spec.ts | 52 ------------------------------------- 2 files changed, 48 insertions(+), 52 deletions(-) diff --git a/tests/copy-button.spec.ts b/tests/copy-button.spec.ts index e4703dfd11..e3de3395ce 100644 --- a/tests/copy-button.spec.ts +++ b/tests/copy-button.spec.ts @@ -183,3 +183,51 @@ test('the copy action stays at rest when the markdown cannot be fetched', async await expect(label).toBeVisible(); await expect(page.locator('.copy-status')).toHaveText(''); }); + +// The label and the "Copied" it turns into are both in the DOM, stacked, so +// reporting a result must not reflow the page actions row. +test('the copy action keeps its width while it reports a result', async ({ + page, +}) => { + await page.goto(MD_PAGE); + + const button = page.locator('.octo-copy-md .split-btn__primary'); + const before = await button.boundingBox(); + + await button.click(); + await expect(button).toHaveAttribute('data-copied', ''); + + const after = await button.boundingBox(); + expect(after!.width).toBe(before!.width); +}); + +// "Open this page as markdown" used to sit in a menu at the bottom of the +// article. It is the copy action's one menu item now. +test('the copy action menu holds exactly one item, linking to the page markdown', async ({ + page, +}) => { + await page.goto(MD_PAGE); + + const menu = page.locator('.octo-copy-md [data-menu]'); + await menu.locator('summary').click(); + + const items = menu.locator('.menu__action'); + await expect(items).toHaveCount(1); + await expect(items).toHaveAttribute('href', MD_PAGE + '.md'); + await expect(items).toHaveAttribute('target', '_blank'); +}); + +// The URL sits on the control while the copy listener matches the primary half. +// Matching the control instead would copy the page every time the menu opened. +test('opening the menu does not copy the page', async ({ page }) => { + await page.goto(MD_PAGE); + + const control = page.locator('.octo-copy-md'); + await control.locator('summary').click(); + await expect(control.locator('.menu__list')).toBeVisible(); + + await expect(control.locator('.split-btn__primary')).not.toHaveAttribute( + 'data-copied', + '' + ); +}); diff --git a/tests/llm-endpoints.spec.ts b/tests/llm-endpoints.spec.ts index 50ae704524..c033c9dde3 100644 --- a/tests/llm-endpoints.spec.ts +++ b/tests/llm-endpoints.spec.ts @@ -96,25 +96,6 @@ test('Copy as markdown action advertises a working .md URL on the eligible page' expect(target.status()).toBe(200); }); -// The rest label and both results are always in the DOM, stacked, so reporting -// a result must not reflow the page actions row. -test('the copy action keeps its width while it reports a result', async ({ - page, - context, -}) => { - await context.grantPermissions(['clipboard-read', 'clipboard-write']); - await page.goto(STABLE_PLAIN_MD_PATH); - - const button = page.locator('.octo-copy-md .split-btn__primary'); - const before = await button.boundingBox(); - - await button.click(); - await expect(button).toHaveAttribute('data-copied', ''); - - const after = await button.boundingBox(); - expect(after!.width).toBe(before!.width); -}); - test('markdown page actions are hidden on the ineligible MDX page', async ({ page, }) => { @@ -125,39 +106,6 @@ test('markdown page actions are hidden on the ineligible MDX page', async ({ ).toBe(0); }); -// The "Open this page as markdown" link used to sit in a menu at the bottom of -// the article, alongside a link to llms-full.txt. It is in the copy action's -// menu now, and the llms-full.txt link is gone. -test('the copy action menu holds exactly one item, linking to the page markdown', async ({ - page, -}) => { - await page.goto(STABLE_PLAIN_MD_PATH); - - const menu = page.locator('.octo-copy-md [data-menu]'); - await menu.locator('summary').click(); - - const items = menu.locator('.menu__action'); - await expect(items).toHaveCount(1); - await expect(items).toHaveAttribute('href', STABLE_PLAIN_MD_PATH + '.md'); - await expect(items).toHaveAttribute('target', '_blank'); -}); - -// The URL sits on the control while the copy listener matches the primary half. -// Matching the control instead would copy the page every time the menu opened. -test('opening the menu does not copy the page', async ({ page, context }) => { - await context.grantPermissions(['clipboard-read', 'clipboard-write']); - await page.goto(STABLE_PLAIN_MD_PATH); - - const control = page.locator('.octo-copy-md'); - await control.locator('summary').click(); - await expect(control.locator('.menu__list')).toBeVisible(); - - await expect(control.locator('.split-btn__primary')).not.toHaveAttribute( - 'data-copied', - '' - ); -}); - test('no page links to llms-full.txt', async ({ page }) => { await page.goto(STABLE_PLAIN_MD_PATH); expect( From f53e46a166bf2d0bda8f76fed83a2b066055e47e Mon Sep 17 00:00:00 2001 From: William Laugesen Date: Wed, 19 Aug 2026 15:16:33 +1200 Subject: [PATCH 6/7] Move the copy action's styles into its component Nothing outside CopyAsMarkdown, its script and its tests uses these classes, so main.css was holding component-private rules. They go in the component's own