diff --git a/.changeset/markdown-anchor-links.md b/.changeset/markdown-anchor-links.md new file mode 100644 index 00000000..1c9b279e --- /dev/null +++ b/.changeset/markdown-anchor-links.md @@ -0,0 +1,5 @@ +--- +"aicodeman": patch +--- + +A link to another heading of the same file in the markdown File Viewer (`[Install](#installation)`) now scrolls to that heading. It did nothing: marked emits no heading ids, so there was nothing to jump to, and with `` a bare `#installation` href points at the dashboard's root, not at the page, so letting the browser follow it was never an in-page jump either. The shared link handler (File Viewer and Response Viewer) now resolves fragment links itself against the rendered document: headings get GitHub-style slugs (lower-case, punctuation dropped, a repeated title numbered `-1`, `-2`), matched case-insensitively and percent-decoded, `#` goes to the top, an explicit `` in the document works, and a fragment that matches nothing is ignored instead of navigating the app. The anchors are `data-md-anchor` attributes looked up inside the document, never `id`s, so a heading called "Settings" cannot collide with an element of the app itself. diff --git a/config/test-suites.ts b/config/test-suites.ts index 6283d361..ee36a4ad 100644 --- a/config/test-suites.ts +++ b/config/test-suites.ts @@ -45,6 +45,7 @@ export const BROWSER_TEST_GLOBS = [ 'test/spreadsheet-preview.browser.test.ts', 'test/mobile-ime-preview.browser.test.ts', 'test/run-mode-menu-scroll.browser.test.ts', + 'test/markdown-anchor-links.browser.test.ts', ]; /** diff --git a/docs/wiki/Working-With-Files.md b/docs/wiki/Working-With-Files.md index bfa4a6ef..8e7b5afa 100644 --- a/docs/wiki/Working-With-Files.md +++ b/docs/wiki/Working-With-Files.md @@ -14,7 +14,7 @@ It renders what it can: | Kind | Behaviour | | ------------------------ | ------------------------------------------------------------------------- | | Text and code | Plain preview with Lines (line numbers) and Wrap toggles in the header. Long files are truncated in plain preview. | -| Markdown | Rendered by default: headings, tables, code blocks with copy buttons, images and links relative to the file (root-relative ones resolve from the workspace root, as on GitHub). Opened from an attachment card, where the file's folder is unknown, relative images show their alt text and relative links show as plain text. The MD pill in the header flips to source. | +| Markdown | Rendered by default: headings, tables, code blocks with copy buttons, images and links relative to the file (root-relative ones resolve from the workspace root, as on GitHub). Links to another heading of the same file (`[Install](#installation)`) scroll to it, with GitHub's heading names (lower-case, punctuation dropped, repeats numbered `-1`, `-2`), and never leave the page. Opened from an attachment card, where the file's folder is unknown, relative images show their alt text and relative links show as plain text. The MD pill in the header flips to source. | | Images | Inline. | | Audio and video | Inline with a working scrub bar, because range requests are supported. | | Spreadsheets (`.xlsx`) | Read-only grid, parsed in your browser (never on the server), up to 10 MB. `.xls` and `.ods` are download only. | diff --git a/src/web/public/app.js b/src/web/public/app.js index 1c945bd2..aa4520e0 100644 --- a/src/web/public/app.js +++ b/src/web/public/app.js @@ -2732,6 +2732,23 @@ class CodemanApp { return; } + // An in-document link (`[Install](#installation)`). The browser must not follow it: with + // `` a bare fragment points at the dashboard's root and would navigate the + // app away. Resolve it inside this rendered document and scroll there (constants.js). + // A fragment that matches nothing is simply ignored, never a navigation. + const fragmentLink = ev.target.closest('a[href^="#"]'); + if (fragmentLink && body.contains(fragmentLink)) { + ev.preventDefault(); + ev.stopPropagation(); + const root = fragmentLink.closest('.rv-text') || body; + const target = window.CodemanMarkdownAnchors?.find(root, fragmentLink.getAttribute('href')); + if (target) { + const calm = window.matchMedia?.('(prefers-reduced-motion: reduce)')?.matches; + target.scrollIntoView({ block: 'start', behavior: calm ? 'auto' : 'smooth' }); + } + return; + } + // A `localhost` URL in the agent's answer: from another device that can // only load through the server, so hand it to a proxied web tab // (webview-tabs.js). Every other link keeps its new-tab default. diff --git a/src/web/public/constants.js b/src/web/public/constants.js index 30cd2649..91d17060 100644 --- a/src/web/public/constants.js +++ b/src/web/public/constants.js @@ -1301,6 +1301,63 @@ function cleanCopiedSelection(text, options) { return lines.join('\n'); } +// ── Markdown heading anchors ──────────────────────────────────────────────── +// marked emits no `id` on headings, so a rendered document's own `[Install](#installation)` links had +// nothing to jump to. And with `` a bare `#installation` href points at the dashboard's +// root, not at the page, so letting the browser follow it navigates the app away. The click delegate +// (`_bindResponseViewerInteractions`) therefore resolves in-document links itself, with the helpers +// below. Anchors are `data-md-anchor` attributes, NOT `id`s: a heading titled "Settings" must not claim +// the id of an element in the app's own DOM, and the lookup is scoped to the rendered document. + +/** + * GitHub's heading slug: lower-cased, anything that is not a letter, mark, number, `_`, `-` or space + * dropped, each space a hyphen (`Why `codeman`? → `why-codeman`, `Über uns` → `über-uns`). + */ +function markdownHeadingSlug(text) { + return String(text ?? '') + .trim() + .toLowerCase() + .replace(/[^\p{L}\p{M}\p{N}_\- ]/gu, '') + .replace(/ /g, '-'); +} + +/** Give every h1..h6 under `root` its slug in `data-md-anchor`; a repeat gets `-1`, `-2`, ... as on GitHub. Idempotent. */ +function assignMarkdownHeadingAnchors(root) { + const used = new Set(); + for (const heading of root.querySelectorAll('h1, h2, h3, h4, h5, h6')) { + const base = markdownHeadingSlug(heading.textContent); + let slug = base; + for (let n = 1; used.has(slug); n += 1) slug = `${base}-${n}`; + used.add(slug); + heading.dataset.mdAnchor = slug; + } +} + +/** + * The element inside `root` that an in-document link (`#installation`, `#Installation`, `#my%20title`) + * points at, or null. An empty fragment (`#`) means the top of the document. Headings are matched by + * slug, then a heading the author wrote an explicit ``/`id` for, looked up INSIDE `root` + * only (never `document.getElementById`, which could find an app element of the same name). + */ +function findMarkdownAnchorTarget(root, href) { + let fragment = String(href ?? '').replace(/^#/, ''); + try { + fragment = decodeURIComponent(fragment); + } catch { + /* a malformed escape: use it as written */ + } + if (!fragment) return root; + assignMarkdownHeadingAnchors(root); + const wanted = [fragment.toLowerCase(), markdownHeadingSlug(fragment)]; + for (const heading of root.querySelectorAll('[data-md-anchor]')) { + if (wanted.includes(heading.dataset.mdAnchor)) return heading; + } + for (const el of root.querySelectorAll('[id]')) { + if (el.id === fragment) return el; + } + return null; +} + if (typeof window !== 'undefined') { window.WEBGL_FALLBACK = WEBGL_FALLBACK; window.evaluateWebGLLongTaskTrip = evaluateWebGLLongTaskTrip; @@ -1370,6 +1427,11 @@ if (typeof window !== 'undefined') { window.CodemanCopySelection = { clean: cleanCopiedSelection, }; + window.CodemanMarkdownAnchors = { + slug: markdownHeadingSlug, + assign: assignMarkdownHeadingAnchors, + find: findMarkdownAnchorTarget, + }; window.CodemanTerminalFont = { DEFAULT_STACK: TERMINAL_FONT_DEFAULT_STACK, resolve: resolveTerminalFontFamily, diff --git a/test/markdown-anchor-links.browser.test.ts b/test/markdown-anchor-links.browser.test.ts new file mode 100644 index 00000000..843c5574 --- /dev/null +++ b/test/markdown-anchor-links.browser.test.ts @@ -0,0 +1,167 @@ +/** + * Clicking a link to another heading of the SAME markdown file in the File Viewer must scroll to + * that heading. It did nothing: marked emits no heading ids, and with `` a bare + * `#section` href is not an in-page link anyway. Real xterm-free browser, real preview overlay, + * real click. + * + * Browser-driven, so excluded from `npm run test:ci` (config/test-suites.ts). Run locally: + * npm run test:browser -- test/markdown-anchor-links.browser.test.ts + * + * Port: 3294 + */ +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { homedir } from 'node:os'; +import { join } from 'node:path'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { chromium, type Browser, type Page } from 'playwright'; +import { WebServer } from '../src/web/server.js'; + +const PORT = 3294; +const filler = (n: number) => + Array.from({ length: n }, (_, i) => `Paragraph ${i} of filler text so the document is taller than the viewer.`).join( + '\n\n' + ); + +describe('in-document links in the markdown File Viewer', () => { + let server: WebServer; + let browser: Browser; + let page: Page; + let root: string; + + beforeAll(async () => { + root = mkdtempSync(join(homedir(), 'md-anchors-')); + mkdirSync(root, { recursive: true }); + writeFileSync( + join(root, 'README.md'), + [ + '# Title', + '', + '[Usage](#usage) · [Second install](#install-1) · [Mixed case](#My-Title) · [Encoded](#my%20title) · [Nowhere](#nope) · [Top](#)', + '', + '## Install', + filler(30), + '## Usage', + filler(30), + '## Install', + filler(30), + '## My Title', + filler(30), + '## Settings', + filler(10), + ].join('\n') + ); + server = new WebServer(PORT, false, true); + await server.start(); + browser = await chromium.launch({ headless: true }); + page = await (await browser.newContext({ viewport: { width: 1100, height: 700 } })).newPage(); + // Reduced motion makes the scroll instant (the code uses behavior 'auto' for it), so the + // position can be read right after the click instead of racing a smooth-scroll animation. + await page.emulateMedia({ reducedMotion: 'reduce' }); + await page.goto(`http://localhost:${PORT}`, { waitUntil: 'domcontentloaded' }); + await page.waitForFunction(() => (window as any).app?.terminal, null, { timeout: 30000 }); + const sessionId = await page.evaluate(async (workingDir) => { + const res = await fetch('/api/sessions', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ workingDir, mode: 'shell' }), + }); + const id = (await res.json()).data.session.id; + await fetch(`/api/sessions/${id}/shell`, { method: 'POST' }); + return id as string; + }, root); + await page.evaluate(async (sid) => { + const app = (window as any).app; + for (let i = 0; i < 100 && !app.sessions.has(sid); i++) await new Promise((r) => setTimeout(r, 100)); + await app.selectSession(sid); + }, sessionId); + await page.evaluate(({ sid, path }) => (window as any).app.openFilePreview(path, sid), { + sid: sessionId, + path: join(root, 'README.md'), + }); + await page.waitForSelector('#filePreviewBody .file-preview-md h2'); + // A marker that a full navigation would wipe. + await page.evaluate(() => ((window as any).__stillHere = true)); + }, 90000); + + afterAll(async () => { + if (browser) await browser.close(); + if (server) await server.stop(); + rmSync(root, { recursive: true, force: true }); + }, 60000); + + const state = () => + page.evaluate(() => { + const body = document.getElementById('filePreviewBody')!; + return { + scrollTop: body.scrollTop, + stillHere: (window as any).__stillHere === true, + url: location.href, + bodyTop: body.getBoundingClientRect().top, + }; + }); + /** How far the nth heading's top is from the top of the scrolling viewer, in px. */ + const headingOffset = (text: string, nth = 0) => + page.evaluate( + ({ text, nth }) => { + const body = document.getElementById('filePreviewBody')!; + const hs = [...body.querySelectorAll('.file-preview-md h1, .file-preview-md h2')].filter( + (h) => h.textContent === text + ); + return Math.round(hs[nth].getBoundingClientRect().top - body.getBoundingClientRect().top); + }, + { text, nth } + ); + const click = async (name: string) => { + // A DOM click: Playwright's own click scrolls the link into view first, which would move the + // very scroll position these tests measure. + await page.locator(`#filePreviewBody a:text-is("${name}")`).evaluate((a) => (a as HTMLElement).click()); + await page.waitForTimeout(100); + }; + + it('scrolls to the heading a link names, and stays on the page', async () => { + const before = await state(); + expect(before.scrollTop).toBe(0); + await click('Usage'); + const after = await state(); + expect(after.scrollTop).toBeGreaterThan(500); + expect(Math.abs(await headingOffset('Usage'))).toBeLessThan(60); // the heading is at the top of the viewer + expect(after.stillHere).toBe(true); // no navigation + expect(after.url).toBe(before.url); + }); + + it('a repeated heading is reachable as slug-1, as on GitHub', async () => { + await page.evaluate(() => (document.getElementById('filePreviewBody')!.scrollTop = 0)); + await click('Second install'); + expect(Math.abs(await headingOffset('Install', 1))).toBeLessThan(60); + }); + + it('matches case-insensitively and with an encoded space', async () => { + for (const name of ['Mixed case', 'Encoded']) { + await page.evaluate(() => (document.getElementById('filePreviewBody')!.scrollTop = 0)); + await click(name); + expect(Math.abs(await headingOffset('My Title')), name).toBeLessThan(60); + } + }); + + it('a link to nowhere does nothing (no scroll, no navigation), and # goes back to the top', async () => { + await page.evaluate(() => (document.getElementById('filePreviewBody')!.scrollTop = 0)); + await click('Nowhere'); + const stay = await state(); + expect(stay.scrollTop).toBe(0); + expect(stay.stillHere).toBe(true); + + await click('Usage'); + expect((await state()).scrollTop).toBeGreaterThan(500); + await page.evaluate(() => (document.getElementById('filePreviewBody')!.scrollTop = 99999)); + await page.evaluate(() => document.querySelector('#filePreviewBody a:not([data-path])')!.scrollIntoView()); + await click('Top'); + expect((await state()).scrollTop).toBeLessThan(5); + }); + + it('a heading cannot capture an element of the app by id (Settings heading, no id given)', async () => { + const ids = await page.evaluate(() => + [...document.querySelectorAll('#filePreviewBody h1, #filePreviewBody h2')].map((h) => h.id) + ); + expect(ids.every((id) => id === '')).toBe(true); + }); +}); diff --git a/test/markdown-anchors.test.ts b/test/markdown-anchors.test.ts new file mode 100644 index 00000000..1dfe5b72 --- /dev/null +++ b/test/markdown-anchors.test.ts @@ -0,0 +1,110 @@ +/** + * In-document links in rendered markdown (`[Install](#installation)`). + * + * marked emits no heading ids, and with `` a bare `#installation` href points at the + * dashboard's root, so the File Viewer's links neither found a target nor stayed on the page. The + * helpers in constants.js give headings GitHub-style slugs (as `data-md-anchor`, never `id`) and + * resolve a fragment to a heading inside the rendered document only. + * + * Builds a JSDOM window in-test under the default node env (do NOT declare a per-file jsdom + * environment: it externalizes node:fs under vite and the readFileSync below stops working). + */ +import { readFileSync } from 'node:fs'; +import { JSDOM } from 'jsdom'; +import { describe, expect, it } from 'vitest'; + +const CONSTANTS = readFileSync(new URL('../src/web/public/constants.js', import.meta.url), 'utf-8'); +const APP = readFileSync(new URL('../src/web/public/app.js', import.meta.url), 'utf-8'); + +interface Anchors { + slug(text: string): string; + assign(root: Element): void; + find(root: Element, href: string): Element | null; +} + +function boot(html: string) { + const dom = new JSDOM( + `
app element
${html}
`, + { + url: 'http://localhost/', + runScripts: 'outside-only', + } + ); + const win = dom.window as unknown as Window & { eval(code: string): unknown; CodemanMarkdownAnchors: Anchors }; + win.eval(CONSTANTS); + const doc = win.document.querySelector('.doc') as Element; + return { win, doc, anchors: win.CodemanMarkdownAnchors }; +} + +describe('markdownHeadingSlug (GitHub rules)', () => { + const { anchors } = boot(''); + it.each([ + ['Installation', 'installation'], + ['Why `codeman`?', 'why-codeman'], + ['Über uns', 'über-uns'], + ['A B', 'a--b'], + [' Trim me ', 'trim-me'], + ['Q&A: what, why?', 'qa-what-why'], + ['snake_case and kebab-case', 'snake_case-and-kebab-case'], + ['日本語 の 見出し', '日本語-の-見出し'], + ['🚀 Launch', '-launch'], + ['', ''], + ])('%j -> %j', (text, slug) => expect(anchors.slug(text)).toBe(slug)); +}); + +describe('assign + find', () => { + const html = ` +

Overview

x

+

Install

Usage

Install

Install

+

My Title

`; + + it('slugs every heading, and a repeated title gets -1, -2 like GitHub', () => { + const { doc, anchors } = boot(html); + anchors.assign(doc); + const slugs = [...doc.querySelectorAll('h1, h2, h3')].map((h) => (h as HTMLElement).dataset.mdAnchor); + expect(slugs).toEqual(['overview', 'install', 'usage', 'install-1', 'install-2', 'my-title']); + anchors.assign(doc); // idempotent + expect([...doc.querySelectorAll('h1, h2, h3')].map((h) => (h as HTMLElement).dataset.mdAnchor)).toEqual(slugs); + }); + + it('finds a heading by its slug, case-insensitively, percent-encoded or with a space', () => { + const { doc, anchors } = boot(html); + const title = doc.querySelectorAll('h2')[2]; + for (const href of ['#my-title', '#My-Title', '#my%20title', '#My Title']) { + expect(anchors.find(doc, href), href).toBe(title); + } + expect(anchors.find(doc, '#install-2')?.tagName).toBe('H3'); + }); + + it('finds an explicit id the author wrote, and treats # as the top of the document', () => { + const { doc, anchors } = boot(html); + expect(anchors.find(doc, '#custom-spot')?.id).toBe('custom-spot'); + expect(anchors.find(doc, '#')).toBe(doc); + expect(anchors.find(doc, '')).toBe(doc); + }); + + it('returns null for a fragment that matches nothing, and a malformed escape is used as written', () => { + const { doc, anchors } = boot(html); + expect(anchors.find(doc, '#nope')).toBeNull(); + expect(() => anchors.find(doc, '#%E0%A4%A')).not.toThrow(); + }); + + it('never resolves against the app: a heading cannot claim an element id outside the document', () => { + const { doc, anchors } = boot('

Settings

'); + const found = anchors.find(doc, '#settings'); + expect(found?.tagName).toBe('H1'); // the heading, not
in the app + expect(found?.id).toBe(''); // and the heading was not given an id + }); +}); + +describe('the click delegate', () => { + it('handles fragment links before anything that would let the browser follow them', () => { + const delegate = APP.slice(APP.indexOf('_bindResponseViewerInteractions(body) {')); + const fragment = delegate.indexOf('a[href^="#"]'); + expect(fragment).toBeGreaterThan(-1); + // After file-path links (their href is '#', they open the preview) and before the loopback/new-tab handling. + expect(fragment).toBeGreaterThan(delegate.indexOf("closest('a.rv-path')")); + expect(fragment).toBeLessThan(delegate.indexOf('openLinkThroughWebTabIfLoopback')); + expect(delegate.slice(fragment, fragment + 400)).toContain('preventDefault'); + }); +});