From df26b6c98e9d24d44ce951bb2b6d5c871734ecee Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Wed, 25 Feb 2026 20:25:55 -0300 Subject: [PATCH 1/3] fix(comments): cross-page collision avoidance for floating comment bubbles (SD-1998) Replace per-page collision avoidance with a single continuous column layout. Active comment pins to its anchor position while others flow around it. Re-measures heights on active comment expansion/collapse to prevent overlap. Only scrolls to anchor when it's off-screen (viewport-aware). --- .../CommentsLayer/CommentDialog.vue | 1 + .../CommentsLayer/FloatingComments.vue | 120 ++++++++++++++---- 2 files changed, 93 insertions(+), 28 deletions(-) diff --git a/packages/superdoc/src/components/CommentsLayer/CommentDialog.vue b/packages/superdoc/src/components/CommentsLayer/CommentDialog.vue index 98947725e9..c048b0848d 100644 --- a/packages/superdoc/src/components/CommentsLayer/CommentDialog.vue +++ b/packages/superdoc/src/components/CommentsLayer/CommentDialog.vue @@ -524,6 +524,7 @@ watch(editingCommentId, (commentId) => { border-radius: 12px; background-color: #f3f6fd; font-family: var(--sd-ui-font-family, Arial, Helvetica, sans-serif); + cursor: pointer; transition: background-color 250ms ease; -webkit-box-shadow: 0px 4px 12px 0px rgba(50, 50, 50, 0.15); -moz-box-shadow: 0px 4px 12px 0px rgba(50, 50, 50, 0.15); diff --git a/packages/superdoc/src/components/CommentsLayer/FloatingComments.vue b/packages/superdoc/src/components/CommentsLayer/FloatingComments.vue index a55a10cd5c..ecf2503351 100644 --- a/packages/superdoc/src/components/CommentsLayer/FloatingComments.vue +++ b/packages/superdoc/src/components/CommentsLayer/FloatingComments.vue @@ -89,21 +89,40 @@ const allPositions = computed(() => { }); } - // Collision avoidance: push overlapping comments down (per page) - const groupedByPage = {}; - for (const pos of positions) { - const key = pos.pageIndex; - if (!groupedByPage[key]) groupedByPage[key] = []; - groupedByPage[key].push(pos); - } + // Cross-page collision avoidance (Google Docs-style). + // All comments form a single continuous column sorted by anchor position. + // When a comment is active, it pins to its anchor and others flow around it. + positions.sort((a, b) => a.anchorTop - b.anchorTop); + + const activeKey = activeCommentKey.value; + const activeIndex = activeKey ? positions.findIndex((p) => p.id === activeKey) : -1; + const gap = 15; + + if (activeIndex >= 0) { + // Pin the active comment at its anchor position + positions[activeIndex].top = positions[activeIndex].anchorTop; + + // Below the active: push down from the active comment + let cursor = positions[activeIndex].top + positions[activeIndex].height + gap; + for (let i = activeIndex + 1; i < positions.length; i++) { + positions[i].top = Math.max(positions[i].anchorTop, cursor); + cursor = positions[i].top + positions[i].height + gap; + } - for (const pageComments of Object.values(groupedByPage)) { - pageComments.sort((a, b) => a.top - b.top); - for (let i = 1; i < pageComments.length; i++) { - const prev = pageComments[i - 1]; - const minTop = prev.top + prev.height + 15; - if (pageComments[i].top < minTop) { - pageComments[i].top = minTop; + // Above the active: push up from the active comment + cursor = positions[activeIndex].top - gap; + for (let i = activeIndex - 1; i >= 0; i--) { + const bottomEdge = cursor - positions[i].height; + positions[i].top = Math.min(positions[i].anchorTop, bottomEdge); + cursor = positions[i].top - gap; + } + } else { + // No active comment: simple top-to-bottom collision avoidance + for (let i = 1; i < positions.length; i++) { + const prev = positions[i - 1]; + const minTop = prev.top + prev.height + gap; + if (positions[i].top < minTop) { + positions[i].top = minTop; } } } @@ -191,28 +210,72 @@ const setPlaceholderRef = (id, el) => { } }; -// Reactive vertical offset — stays in sync as allPositions recomputes from height measurements -const verticalOffset = computed(() => { - if (!activeComment.value) return 0; - const comment = commentsStore.getComment(activeComment.value); - if (!comment) return 0; - const key = commentsStore.getCommentPositionKey(comment); - const position = allPositions.value.find((p) => p.id === key); - if (!position) return 0; - return position.anchorTop - position.top; +// Vertical offset is no longer needed — the cross-page layout algorithm pins the active +// comment at its anchor position directly, so the sidebar container stays at top: 0. +const verticalOffset = computed(() => 0); + +// Re-measure when active comment changes. The active dialog expands (reply input, thread) +// and the previously active one collapses — both change height. We re-measure after DOM +// updates so the layout algorithm uses the correct heights for collision avoidance. +watch(activeCommentKey, (newKey, oldKey) => { + const remeasure = () => { + const keys = [newKey, oldKey].filter(Boolean); + if (!keys.length) return; + + let changed = false; + const updates = { ...measuredHeights.value }; + + for (const key of keys) { + const el = placeholderRefs.value[key]; + if (!el) continue; + const dialog = el.querySelector('.comments-dialog'); + if (!dialog) continue; + const height = dialog.getBoundingClientRect().height; + if (height > 0 && height !== updates[key]) { + updates[key] = height; + _heightsCache[key] = height; + changed = true; + } + } + + if (changed) { + measuredHeights.value = updates; + } + }; + + // Two passes: first after DOM update, second after transitions settle + nextTick(() => setTimeout(remeasure, 50)); + nextTick(() => setTimeout(remeasure, 300)); }); -// Scroll active comment into view when it changes +// Scroll to the active comment ONLY when its anchor is off-screen. +// getBoundingClientRect() is viewport-relative, so it already accounts for scroll +// position and zoom — we just need to check if it's within the visible area. +// If the anchor text is visible, do nothing — let the comments reflow in place. watch(activeComment, () => { if (!activeComment.value) return; const comment = commentsStore.getComment(activeComment.value); if (!comment) return; const key = commentsStore.getCommentPositionKey(comment); + if (!key) return; - setTimeout(() => { - const el = placeholderRefs.value[key]; - el?.scrollIntoView({ behavior: 'smooth', block: 'center' }); - }, 200); + nextTick(() => { + setTimeout(() => { + const el = placeholderRefs.value[key]; + if (!el) return; + + // The active comment is pinned at its anchor Y, so the placeholder's + // viewport position matches the anchor text's vertical position. + const rect = el.getBoundingClientRect(); + const margin = 80; // buffer so it's not at the very edge + const isVisible = rect.top >= margin && rect.top <= window.innerHeight - margin; + + if (!isVisible) { + // Minimal scroll — just enough to bring the anchor into view + el.scrollIntoView({ behavior: 'smooth', block: 'nearest' }); + } + }, 100); + }); }); // PDF zoom change: reset measurements @@ -275,6 +338,7 @@ onBeforeUnmount(() => { .comment-placeholder { position: absolute; width: 300px; + transition: top 0.3s ease; } .floating-comment { From a0b471b110bcb33486151e48fb3320ab2b886d23 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Wed, 25 Feb 2026 21:58:34 -0300 Subject: [PATCH 2/3] =?UTF-8?q?refactor(comments):=20review=20fixes=20?= =?UTF-8?q?=E2=80=94=20layout=20extraction,=20negative=20tops,=20stale=20t?= =?UTF-8?q?imers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Extract resolveCollisions() into named function for testability - Floor negative top values when upward push overflows (shift all down) - Cancel stale setTimeout callbacks on rapid active-comment switching - Remove dead verticalOffset computed + template binding - Extract shared storeHeight() helper (dedup handleDialog/remeasure) - Drop unused pageIndex from position objects - Fix timer delays: 350ms to clear 300ms CSS transition --- .../CommentsLayer/FloatingComments.vue | 154 +++++++++--------- 1 file changed, 74 insertions(+), 80 deletions(-) diff --git a/packages/superdoc/src/components/CommentsLayer/FloatingComments.vue b/packages/superdoc/src/components/CommentsLayer/FloatingComments.vue index ecf2503351..81211231cc 100644 --- a/packages/superdoc/src/components/CommentsLayer/FloatingComments.vue +++ b/packages/superdoc/src/components/CommentsLayer/FloatingComments.vue @@ -13,6 +13,46 @@ import CommentDialog from '@superdoc/components/CommentsLayer/CommentDialog.vue' const ESTIMATED_HEIGHT = 80; const OBSERVER_MARGIN = 600; +// Layout algorithm: positions comments in a single column with collision avoidance. +// When a comment is active it pins at its anchor; neighbors push up/down to avoid overlap. +// If upward push produces negative tops, everything shifts down to stay on screen. +const resolveCollisions = (positions, activeIndex, gap) => { + if (activeIndex >= 0) { + positions[activeIndex].top = positions[activeIndex].anchorTop; + + // Below: push down from the active comment + let cursor = positions[activeIndex].top + positions[activeIndex].height + gap; + for (let i = activeIndex + 1; i < positions.length; i++) { + positions[i].top = Math.max(positions[i].anchorTop, cursor); + cursor = positions[i].top + positions[i].height + gap; + } + + // Above: push up from the active comment + cursor = positions[activeIndex].top - gap; + for (let i = activeIndex - 1; i >= 0; i--) { + const bottomEdge = cursor - positions[i].height; + positions[i].top = Math.min(positions[i].anchorTop, bottomEdge); + cursor = positions[i].top - gap; + } + + // Floor: if upward push produced negative tops, shift everything down + const minTop = Math.min(...positions.map((p) => p.top)); + if (minTop < 0) { + const shift = Math.abs(minTop); + for (const p of positions) p.top += shift; + } + } else { + // No active comment: simple top-to-bottom collision avoidance + for (let i = 1; i < positions.length; i++) { + const prev = positions[i - 1]; + const minTop = prev.top + prev.height + gap; + if (positions[i].top < minTop) { + positions[i].top = minTop; + } + } + } +}; + const props = defineProps({ currentDocument: { type: Object, @@ -78,54 +118,20 @@ const allPositions = computed(() => { const top = getAnchorTop(comment); if (!key || typeof top !== 'number' || isNaN(top)) continue; - const positionEntry = editorCommentPositions.value[key]; positions.push({ id: key, anchorTop: top, top, height: measuredHeights.value[key] || ESTIMATED_HEIGHT, commentRef: comment, - pageIndex: positionEntry?.pageIndex ?? 0, }); } - // Cross-page collision avoidance (Google Docs-style). - // All comments form a single continuous column sorted by anchor position. - // When a comment is active, it pins to its anchor and others flow around it. positions.sort((a, b) => a.anchorTop - b.anchorTop); const activeKey = activeCommentKey.value; const activeIndex = activeKey ? positions.findIndex((p) => p.id === activeKey) : -1; - const gap = 15; - - if (activeIndex >= 0) { - // Pin the active comment at its anchor position - positions[activeIndex].top = positions[activeIndex].anchorTop; - - // Below the active: push down from the active comment - let cursor = positions[activeIndex].top + positions[activeIndex].height + gap; - for (let i = activeIndex + 1; i < positions.length; i++) { - positions[i].top = Math.max(positions[i].anchorTop, cursor); - cursor = positions[i].top + positions[i].height + gap; - } - - // Above the active: push up from the active comment - cursor = positions[activeIndex].top - gap; - for (let i = activeIndex - 1; i >= 0; i--) { - const bottomEdge = cursor - positions[i].height; - positions[i].top = Math.min(positions[i].anchorTop, bottomEdge); - cursor = positions[i].top - gap; - } - } else { - // No active comment: simple top-to-bottom collision avoidance - for (let i = 1; i < positions.length; i++) { - const prev = positions[i - 1]; - const minTop = prev.top + prev.height + gap; - if (positions[i].top < minTop) { - positions[i].top = minTop; - } - } - } + resolveCollisions(positions, activeIndex, 15); return positions; }); @@ -176,9 +182,15 @@ const observePlaceholders = () => { } }; +// Store a measured height for a comment key. Deduplicates the update logic +// shared between initial mount (handleDialog) and active-state remeasure. +const storeHeight = (key, height) => { + if (height <= 0 || height === measuredHeights.value[key]) return; + _heightsCache[key] = height; + measuredHeights.value = { ...measuredHeights.value, [key]: height }; +}; + // When a CommentDialog mounts and reports its size, record the measured height. -// CommentDialog emits importedId when defined (prefers importedId), but allPositions -// keys by getCommentPositionKey (also prefers importedId). We normalize here to match. const handleDialog = (dialog) => { if (!dialog) return; const { elementRef, commentId: rawId } = dialog; @@ -187,16 +199,8 @@ const handleDialog = (dialog) => { nextTick(() => { const bounds = elementRef.value?.getBoundingClientRect(); if (!bounds || bounds.height <= 0) return; - - // Normalize to canonical key (matches allPositions) const key = commentsStore.getCommentPositionKey(rawId); - if (!key) return; - - const current = measuredHeights.value[key]; - if (current !== bounds.height) { - _heightsCache[key] = bounds.height; - measuredHeights.value = { ...measuredHeights.value, [key]: bounds.height }; - } + if (key) storeHeight(key, bounds.height); }); }; @@ -210,49 +214,40 @@ const setPlaceholderRef = (id, el) => { } }; -// Vertical offset is no longer needed — the cross-page layout algorithm pins the active -// comment at its anchor position directly, so the sidebar container stays at top: 0. -const verticalOffset = computed(() => 0); +// Timer IDs for cancellation on rapid active-comment switching +let remeasureTimers = []; +let scrollTimer = null; // Re-measure when active comment changes. The active dialog expands (reply input, thread) -// and the previously active one collapses — both change height. We re-measure after DOM -// updates so the layout algorithm uses the correct heights for collision avoidance. +// and the previously active one collapses — both change height. watch(activeCommentKey, (newKey, oldKey) => { - const remeasure = () => { - const keys = [newKey, oldKey].filter(Boolean); - if (!keys.length) return; + // Cancel stale timers from previous activation + remeasureTimers.forEach(clearTimeout); + remeasureTimers = []; - let changed = false; - const updates = { ...measuredHeights.value }; - - for (const key of keys) { + const remeasure = () => { + for (const key of [newKey, oldKey].filter(Boolean)) { const el = placeholderRefs.value[key]; if (!el) continue; const dialog = el.querySelector('.comments-dialog'); if (!dialog) continue; - const height = dialog.getBoundingClientRect().height; - if (height > 0 && height !== updates[key]) { - updates[key] = height; - _heightsCache[key] = height; - changed = true; - } - } - - if (changed) { - measuredHeights.value = updates; + storeHeight(key, dialog.getBoundingClientRect().height); } }; - // Two passes: first after DOM update, second after transitions settle - nextTick(() => setTimeout(remeasure, 50)); - nextTick(() => setTimeout(remeasure, 300)); + // 50ms: after Vue nextTick + browser rAF settle the initial DOM change + // 350ms: after .comment-placeholder transition (300ms ease) completes + nextTick(() => { + remeasureTimers.push(setTimeout(remeasure, 50)); + remeasureTimers.push(setTimeout(remeasure, 350)); + }); }); // Scroll to the active comment ONLY when its anchor is off-screen. -// getBoundingClientRect() is viewport-relative, so it already accounts for scroll -// position and zoom — we just need to check if it's within the visible area. -// If the anchor text is visible, do nothing — let the comments reflow in place. +// getBoundingClientRect() is viewport-relative (accounts for scroll + zoom). watch(activeComment, () => { + if (scrollTimer) clearTimeout(scrollTimer); + if (!activeComment.value) return; const comment = commentsStore.getComment(activeComment.value); if (!comment) return; @@ -260,18 +255,15 @@ watch(activeComment, () => { if (!key) return; nextTick(() => { - setTimeout(() => { + scrollTimer = setTimeout(() => { const el = placeholderRefs.value[key]; if (!el) return; - // The active comment is pinned at its anchor Y, so the placeholder's - // viewport position matches the anchor text's vertical position. const rect = el.getBoundingClientRect(); - const margin = 80; // buffer so it's not at the very edge + const margin = 80; const isVisible = rect.top >= margin && rect.top <= window.innerHeight - margin; if (!isVisible) { - // Minimal scroll — just enough to bring the anchor into view el.scrollIntoView({ behavior: 'smooth', block: 'nearest' }); } }, 100); @@ -310,7 +302,9 @@ onBeforeUnmount(() => {