mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-11 09:49:41 +02:00
fix(viewer): make in-document links in rendered markdown scroll to their heading
marked emits no heading ids and, with <base href="/">, a bare #section href points at the dashboard root, so [Install](#installation) in the File Viewer did nothing. The shared click delegate now resolves fragment links against the rendered document: GitHub-style slugs in data-md-anchor (never ids, so a heading cannot capture an app element), case-insensitive and percent-decoded, repeats numbered -1/-2, # = top, explicit ids supported, an unmatched fragment ignored instead of navigating. Tests: slug/assign/find unit tests (CI gate) and a real-browser test clicking links in a File Viewer document, which fails without the change. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JrzFKEdBLwVfu6ev2ZscJS
This commit is contained in:
Devvyn
co-authored by
Claude Sonnet 5.5
Claude Sonnet 5.5
parent
3a0cee6b90
commit
8da4a07a60
@@ -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 `<base href="/">` 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);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,110 @@
|
||||
/**
|
||||
* In-document links in rendered markdown (`[Install](#installation)`).
|
||||
*
|
||||
* marked emits no heading ids, and with `<base href="/">` 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(
|
||||
`<!doctype html><body><div id="settings">app element</div><div class="doc">${html}</div></body>`,
|
||||
{
|
||||
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 = `
|
||||
<h1>Overview</h1><p>x</p>
|
||||
<h2>Install</h2><h2>Usage</h2><h3>Install</h3><h3>Install</h3>
|
||||
<h2>My Title</h2><a id="custom-spot"></a>`;
|
||||
|
||||
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('<h1>Settings</h1>');
|
||||
const found = anchors.find(doc, '#settings');
|
||||
expect(found?.tagName).toBe('H1'); // the heading, not <div id="settings"> 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');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user