External/open-design
1
0
Fork 0
mirror of https://github.com/nexu-io/open-design.git synced 2026-10-27 09:51:12 +01:00
Code Issues Packages Projects Releases Wiki Activity

fix(web): preserve owner scroll after comment location

Browse Source
This commit is contained in:
a1chzt
2026-10-09 15:06:23 +08:00
parent 001e7b4679
commit a9d05991c3
3 changed files with 69 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/web/src/components/FileViewer.tsx
+3
View File
@@ -12706,6 +12706,9 @@ function HtmlViewer({
if (!snapshot.elementId || !isValidCommentOverlayPosition(snapshot.position)) return;
if (snapshot.elementId !== request.targetId && !commentSelectorsMatch(snapshot.selector, request.selector)) return;
request.targetId = snapshot.elementId;
// A tagged reply acknowledges srcDoc's completed locate/scroll. Later
// tracking updates must not trigger the URL bridge's initial scroll.
request.scrolled = true;
setActiveCommentTarget(snapshot);
setHoveredCommentTarget(snapshot);
setLiveCommentTargets(current => new Map(current).set(snapshot.elementId, snapshot));
apps/web/tests/components/FileViewer.comment-location.test.tsx
+27
View File
@@ -73,6 +73,33 @@ it('rechecks an old lost comment in the DOM, ignores stale location replies and
expect(screen.queryByTestId('comment-active-pin')).toBeNull();
});
it('does not scroll again after a srcDoc location acknowledgement when the owner scrolls away', async () => {
vi.stubGlobal('fetch', vi.fn(async () => Response.json({})));
render(tree());
fireEvent.click(screen.getByTestId('comment-panel-toggle'));
const frame = screen.getByTestId('artifact-preview-frame') as HTMLIFrameElement;
Object.defineProperty(frame, 'clientHeight', { configurable: true, value: 600 });
const post = vi.spyOn(frame.contentWindow!, 'postMessage');
const dispatch = async (data: object) => { await act(async () => {
window.dispatchEvent(new MessageEvent('message', { source: frame.contentWindow, data }));
}); };
const target = { ...comment, elementId: 'current-h2', position: { x: 20, y: 1100, width: 200, height: 40 } };
await dispatch({ type: 'od:comment-targets', targets: [target] });
fireEvent.click(screen.getByTestId('comment-side-item'));
const request = post.mock.calls.map(call => call[0]).find(data => data?.locate);
await dispatch({ ...target, type: 'od:comment-active-target-update', requestId: request.requestId,
position: { x: 20, y: 80, width: 200, height: 40 } });
expect(screen.getByTestId('comment-active-pin')).toHaveStyle({ top: '80px' });
// The request-id reply already confirms that the bridge scrolled. A later
// untagged position update is tracking the owner's scroll, not another locate.
await dispatch({ ...target, type: 'od:comment-active-target-update' });
expect(post.mock.calls.filter(call => call[0]?.type === 'od:preview-scroll-by')).toHaveLength(0);
fireEvent.click(screen.getByTestId('comment-side-item'));
const repeated = post.mock.calls.map(call => call[0]).filter(data => data?.locate).at(-1);
expect(repeated.requestId).not.toBe(request.requestId);
});
it('scrolls a URL preview through its existing bridge and accepts only the current DOM target', async () => {
vi.stubGlobal('fetch', vi.fn(async () => Response.json({})));
render(tree());
e2e/ui/share-comment-location.test.ts
+39 -4
View File
@@ -5,7 +5,22 @@ for (const forceInline of [false, true])
test(`[P1] Owner locates an old H2 again after scrolling away (srcdoc=${forceInline})`, async ({ page }) => {
test.setTimeout(T.xlong * 2);
const team = { ...AMR_PERSONAL_WORKSPACE_CONTEXT, workspaceId: 'ws-ui-audit-team', workspaceName: 'UI audit team', workspaceType: 'team' as const, workspaceMemberId: 'mem-ui-audit-team', seatSummary: { seatLimit: 5, usedSeats: 1, availableSeats: 4, isSeatFull: false }, teamId: 'team-ui-audit', teamName: 'UI audit team', role: 'owner' as const };
await page.addInitScript(() => { localStorage.setItem('open-design:locale', 'zh-CN'); localStorage.setItem('open-design:locale-source', 'manual'); });
await page.addInitScript(() => {
localStorage.setItem('open-design:locale', 'zh-CN');
localStorage.setItem('open-design:locale-source', 'manual');
// Observe the same real-document handshake that enables URL-mode
// comments. Opening comments before it arrives selects srcDoc.
if (window !== window.top) return;
window.addEventListener('message', event => {
const iframe = document.querySelector<HTMLIFrameElement>('iframe[data-od-render-mode="url-load"][data-od-active="true"]');
if (!iframe || event.source !== iframe.contentWindow || event.origin !== 'null') return;
const src = iframe.getAttribute('src');
if (src && event.data?.type === 'od:url-selection-bridge-ready'
&& event.data.href === new URL(src, window.location.href).href) {
Reflect.set(window, 'commentLocationReadyUrl', event.data.href);
}
});
});
await mockAmrPersonalWorkspace(page);
await page.route('**/api/workspace/directory', r => r.fulfill({ json: { items: [team], activeWorkspaceId: team.workspaceId } }));
await page.route('**/api/workspace/context', r => r.fulfill({ json: { context: team } }));
@@ -39,19 +54,39 @@ for (const forceInline of [false, true])
await page.route(`**/api/projects/${projectId}/conversations/${conversationId}/comments/lost-anchors`, r => r.fulfill({ json: { updated: 0 } }));
await page.goto(`/projects/${projectId}${forceInline ? '?forceInline=1' : ''}`);
await page.getByTestId('file-workspace').getByText('index.html', { exact: true }).first().click();
if (!forceInline) {
const preview = page.getByTestId('artifact-preview-frame');
await expect(preview).toHaveAttribute('data-od-render-mode', 'url-load');
await expect.poll(async () => {
const src = await preview.getAttribute('src');
const readyUrl = await page.evaluate(() => Reflect.get(window, 'commentLocationReadyUrl'));
return !!src && readyUrl === new URL(src, page.url()).href;
}, { message: 'URL preview comment bridge must be ready before opening comments' }).toBe(true);
}
await page.getByTestId('comment-panel-toggle').click();
const row = page.getByTestId('comment-side-item').filter({ hasText: comment.note });
await expect(row).toBeVisible();
await row.click();
const active = page.getByTestId('artifact-preview-frame');
await expect(active).toHaveAttribute('data-od-active', 'true');
await expect(active).toHaveAttribute('data-od-render-mode', forceInline ? 'srcdoc' : 'url-load');
const frame = await (await active.elementHandle())!.contentFrame();
if (!frame)
throw new Error('Preview frame missing');
const target = frame.locator('h2');
// Root-relative assets require the inline pipeline and its generated annotations.
expect((await frame.content()).includes('data.locate')).toBe(forceInline);
if (forceInline)
// Both transports carry the comment bridge. Check the active iframe's
// navigation, not bridge source text, to prove which path is exercised.
if (forceInline) {
await expect(active).toHaveAttribute('srcdoc', /[\s\S]+/);
await expect.poll(() => frame.url()).toBe('about:srcdoc');
await expect(target).toHaveAttribute('data-od-id', /^path-/);
} else {
await expect(active).not.toHaveAttribute('srcdoc', /[\s\S]*/);
await expect(active).toHaveAttribute('src', new RegExp(`/api/projects/${projectId}/`));
const src = await active.getAttribute('src');
if (!src) throw new Error('URL preview source missing');
await expect.poll(() => frame.url()).toBe(new URL(src, page.url()).href);
}
await expect(target).toBeInViewport();
const overlay = page.getByTestId('comment-target-overlay').last();
await expect(overlay).toBeVisible();