diff --git a/web/app/components/editor/comments/CommentRenderer.nuxt.test.ts b/web/app/components/editor/comments/CommentRenderer.nuxt.test.ts index 85b14314..289185e6 100644 --- a/web/app/components/editor/comments/CommentRenderer.nuxt.test.ts +++ b/web/app/components/editor/comments/CommentRenderer.nuxt.test.ts @@ -1,4 +1,4 @@ -import { mountSuspended } from "@nuxt/test-utils/runtime" +import { mockNuxtImport, mountSuspended } from "@nuxt/test-utils/runtime" import { Editor as TiptapEditor, type Editor } from "@tiptap/core" import { ySyncPluginKey } from "@tiptap/y-tiptap" import { @@ -62,6 +62,13 @@ vi.mock("vue-virtual-scroller", async () => { } }) +// happy-dom lays nothing out, so the popover never learns its height. +// The tests set the height by hand. +const popover = vi.hoisted(() => ({ + height: null as unknown as { value: number }, +})) +mockNuxtImport("useElementSize", () => () => ({ height: popover.height })) + const DOCUMENT_ID = makeXid("doc") const BRANCH_ID = makeXid("branch") const ME = makeXid("usme") @@ -255,6 +262,7 @@ describe("", { concurrent: false }, () => { beforeEach(() => { clearTeleportedOverlays() stubThemeColorContext() + popover.height = ref(0) clearQueryCache() useEditorMeta().setEditable(true) useEditorStore().updateActiveDocumentId(DOCUMENT_ID) @@ -333,6 +341,60 @@ describe("", { concurrent: false }, () => { ) }) + it("scrolls a thread into view once it is drawn", async ({ expect }) => { + const scrollIntoView = vi.spyOn(Element.prototype, "scrollIntoView") + const editor = textDoc("hello world") + const wrapper = await mountRenderer(editor) + await api(wrapper).addNewComment(0) + await flushPromises() + + popover.height.value = 120 + await flushPromises() + + expect(scrollIntoView).toHaveBeenCalledExactlyOnceWith({ + behavior: "smooth", + block: "nearest", + }) + expect(scrollIntoView.mock.contexts[0]).toBe( + wrapper.find(".z-popover").element, + ) + }) + + it("scrolls a thread into view again when its height changes", async ({ + expect, + }) => { + const editor = textDoc("hello world") + const wrapper = await mountRenderer(editor) + await api(wrapper).addNewComment(0) + popover.height.value = 120 + await flushPromises() + const scrollIntoView = vi.spyOn(Element.prototype, "scrollIntoView") + + popover.height.value = 200 + await flushPromises() + + expect(scrollIntoView).toHaveBeenCalledExactlyOnceWith({ + behavior: "smooth", + block: "nearest", + }) + expect(scrollIntoView.mock.contexts[0]).toBe( + wrapper.find(".z-popover").element, + ) + }) + + it("leaves the page where it is while a thread is not drawn", async ({ + expect, + }) => { + const scrollIntoView = vi.spyOn(Element.prototype, "scrollIntoView") + const editor = textDoc("hello world") + const wrapper = await mountRenderer(editor) + + await api(wrapper).addNewComment(0) + await flushPromises() + + expect(scrollIntoView).not.toHaveBeenCalled() + }) + it("opens nothing while the reader is not signed in", async ({ expect }) => { seedAuthSession(null) const editor = textDoc("hello world") diff --git a/web/app/components/editor/comments/CommentRenderer.vue b/web/app/components/editor/comments/CommentRenderer.vue index c9c1e3d9..d63eb577 100644 --- a/web/app/components/editor/comments/CommentRenderer.vue +++ b/web/app/components/editor/comments/CommentRenderer.vue @@ -128,6 +128,7 @@ const userData = computed(() => { }) const activePendingId = ref(null) const popoverElem = useTemplateRef("comment-popover") +const { height: popoverHeight } = useElementSize(popoverElem) const popoverPosition = ref({ left: 0, top: 0, @@ -418,6 +419,17 @@ watchDeep(loadedCommentThread, () => { updateCanScrollToBottom() }) +// the popover has no height until it is drawn, so an opening thread +// scrolls once it is measured. A thread that changes height scrolls +// again. +watch([() => selectedComment.value?.id, popoverHeight], () => { + if (!popoverHeight.value) { + return + } + + void scrollPopoverIntoView() +}) + function removeEditorListeners() { // the editor view may already be destroyed when the diff editor // unmounts — in that case listeners are already cleaned up @@ -1360,6 +1372,17 @@ async function updatePopoverPosition() { popoverPosition.value = { left: x, top: y, placement: placement } } +// the popover is placed a tick after its thread changes. Scrolling +// earlier aims at the place it had before. +async function scrollPopoverIntoView() { + await nextTick() + await updatePopoverPosition() + await nextTick() + + // eslint-disable-next-line @typescript-eslint/no-unsafe-call -- eslint's ts program resolves .vue imports as error typed, vue-tsc accepts this + popoverElem.value?.scrollIntoView({ behavior: "smooth", block: "nearest" }) +} + // --- diff mode helpers --- // find the position of the node carrying the given uid in the source @@ -1756,7 +1779,7 @@ function updateNodeCommentIdInEditor(