From 84b673b87b12a58314ea7aed4a386f2baca7c80f Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Thu, 29 Oct 2026 16:06:50 -0700 Subject: [PATCH] perf(editor): collect review note elements once per geometry pass (#34986) * perf(editor): collect review note elements once per geometry pass * test(editor): drop selector call-count tests for review note heights --------- Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com> --- .../rich-markdown-review-note-heights.test.ts | 83 +++++++++++++++++++ .../rich-markdown-review-note-positioning.ts | 12 ++- 2 files changed, 91 insertions(+), 4 deletions(-) create mode 100644 src/renderer/src/components/editor/rich-markdown-review-note-heights.test.ts diff --git a/src/renderer/src/components/editor/rich-markdown-review-note-heights.test.ts b/src/renderer/src/components/editor/rich-markdown-review-note-heights.test.ts new file mode 100644 index 00000000000..07c0cec4100 --- /dev/null +++ b/src/renderer/src/components/editor/rich-markdown-review-note-heights.test.ts @@ -0,0 +1,83 @@ +// @vitest-environment happy-dom +import { Editor } from '@tiptap/core' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { DiffComment } from '../../../../shared/diff-comment-types' +import { createRichMarkdownExtensions } from './rich-markdown-extensions' +import { measureRichMarkdownReviewNotePositions } from './rich-markdown-review-note-positioning' +import { createRichMarkdownEditorCodec } from './rich-markdown-source-transport' + +const editors: Editor[] = [] + +function createEditor() { + const editor = new Editor({ + element: document.createElement('div'), + extensions: createRichMarkdownExtensions({ codec: createRichMarkdownEditorCodec() }), + content: { + type: 'doc', + content: [{ type: 'paragraph', content: [{ type: 'text', text: 'Reviewable paragraph' }] }] + } + }) + editors.push(editor) + vi.spyOn(editor.view, 'coordsAtPos').mockReturnValue({ + top: 100, + bottom: 120, + left: 0, + right: 10 + }) + return editor +} + +function comment(id: string): DiffComment { + return { + id, + worktreeId: 'workspace', + filePath: 'notes.md', + source: 'markdown', + lineNumber: 1, + body: 'Review', + createdAt: 0, + side: 'modified' + } +} + +function addCard(container: HTMLElement, id: string, height: number) { + const card = document.createElement('div') + card.setAttribute('data-rich-markdown-review-note-id', id) + const rect = vi + .spyOn(card, 'getBoundingClientRect') + .mockReturnValue(new DOMRect(0, 0, 10, height)) + container.append(card) + return { card, rect } +} + +afterEach(() => { + editors.splice(0).forEach((editor) => editor.destroy()) + vi.restoreAllMocks() +}) + +describe('review note height collection', () => { + it('remeasures current cards and preserves first-match and missing-card behavior', () => { + const editor = createEditor() + const container = document.createElement('div') + const markdownComments = [comment('first'), comment('second')] + const first = addCard(container, 'first', 20) + const duplicate = addCard(container, 'first', 500) + const unrelated = addCard(container, 'unrelated', 500) + addCard(container, 'second', 30) + const measure = () => + measureRichMarkdownReviewNotePositions({ + editor, + container, + markdownComments, + markdownSourceLineOffset: 0 + }) + expect(measure().map((position) => position.top)).toEqual([100, 128]) + first.rect.mockReturnValue(new DOMRect(0, 0, 10, 100)) + expect(measure().map((position) => position.top)).toEqual([100, 208]) + first.card.remove() + duplicate.card.remove() + expect(measure().map((position) => position.top)).toEqual([100, 210]) + expect(duplicate.rect).not.toHaveBeenCalled() + expect(unrelated.rect).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/components/editor/rich-markdown-review-note-positioning.ts b/src/renderer/src/components/editor/rich-markdown-review-note-positioning.ts index b7cc8bc63e2..a67109f3ebe 100644 --- a/src/renderer/src/components/editor/rich-markdown-review-note-positioning.ts +++ b/src/renderer/src/components/editor/rich-markdown-review-note-positioning.ts @@ -54,10 +54,14 @@ function measureReviewNoteHeights( positions: RichMarkdownReviewNotePosition[] ): Map { const measuredHeights = new Map() - for (const pos of positions) { - const el = container.querySelector(`[data-rich-markdown-review-note-id="${pos.comment.id}"]`) - if (el) { - measuredHeights.set(pos.comment.id, el.getBoundingClientRect().height) + if (positions.length === 0) { + return measuredHeights + } + const commentIds = new Set(positions.map((position) => position.comment.id)) + for (const el of container.querySelectorAll('[data-rich-markdown-review-note-id]')) { + const id = el.getAttribute('data-rich-markdown-review-note-id') + if (id !== null && commentIds.has(id) && !measuredHeights.has(id)) { + measuredHeights.set(id, el.getBoundingClientRect().height) } } return measuredHeights