Skip to content

Commit 9dcc5cf

Browse files
committed
fix(anchor-nav): use correct scroll container for sub-page bookmark navigation
goToAnchor was scrolling the visibleHost element which has overflow:visible and cannot scroll. Now uses #scrollContainer (the first scrollable ancestor) and computes precise Y offsets from layout fragment positions for sub-page scroll precision. SD-2186
1 parent 3313568 commit 9dcc5cf

2 files changed

Lines changed: 29 additions & 7 deletions

File tree

packages/super-editor/src/core/presentation-editor/PresentationEditor.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4997,6 +4997,7 @@ export class PresentationEditor extends EventEmitter {
49974997
bookmarks: this.#layoutState.bookmarks,
49984998
pageGeometryHelper: this.#pageGeometryHelper ?? undefined,
49994999
painterHost: this.#painterHost,
5000+
scrollContainer: this.#scrollContainer ?? this.#visibleHost,
50005001
scrollPageIntoView: (pageIndex) => this.#scrollPageIntoView(pageIndex),
50015002
waitForPageMount: (pageIndex, timeoutMs) => this.#waitForPageMount(pageIndex, { timeout: timeoutMs }),
50025003
getActiveEditor: () => this.getActiveEditor(),

packages/super-editor/src/core/presentation-editor/utils/AnchorNavigation.ts

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ export type GoToAnchorDeps = {
8585
bookmarks: Map<string, number>;
8686
pageGeometryHelper?: PageGeometryHelper;
8787
painterHost: HTMLElement;
88+
scrollContainer: Element | Window;
8889
scrollPageIntoView: (pageIndex: number) => void;
8990
waitForPageMount: (pageIndex: number, timeoutMs: number) => Promise<boolean>;
9091
getActiveEditor: () => Editor;
@@ -99,6 +100,7 @@ export async function goToAnchor({
99100
bookmarks,
100101
pageGeometryHelper,
101102
painterHost,
103+
scrollContainer,
102104
scrollPageIntoView,
103105
waitForPageMount,
104106
getActiveEditor,
@@ -117,14 +119,16 @@ export async function goToAnchor({
117119
const rects = selectionToRects(layout, blocks, measures, pmPos, pmPos + 1, pageGeometryHelper) ?? [];
118120
const rect = rects[0];
119121

120-
// Find the page containing this position by scanning fragments
121-
// Bookmarks often fall in gaps between fragments (e.g., at page/section breaks),
122-
// so we also track the first fragment starting after the position as a fallback
122+
// Find the page and fragment Y offset for the bookmark position.
123+
// selectionToRects often returns empty for bookmarks (zero-width inline nodes),
124+
// so we scan layout fragments to find the precise Y coordinate within the page.
123125
let pageIndex: number | null = rect?.pageIndex ?? null;
126+
let fragmentY: number | null = rect?.top ?? null;
124127

125128
if (pageIndex == null) {
126129
let nextFragmentPage: number | null = null;
127130
let nextFragmentStart: number | null = null;
131+
let nextFragmentY: number | null = null;
128132

129133
for (const page of layout.pages) {
130134
for (const fragment of page.fragments) {
@@ -136,13 +140,15 @@ export async function goToAnchor({
136140
// Exact match: position is within this fragment
137141
if (pmPos >= fragStart && pmPos < fragEnd) {
138142
pageIndex = page.number - 1;
143+
fragmentY = fragment.y;
139144
break;
140145
}
141146

142147
// Track the first fragment that starts after our position
143148
if (fragStart > pmPos && (nextFragmentStart === null || fragStart < nextFragmentStart)) {
144149
nextFragmentPage = page.number - 1;
145150
nextFragmentStart = fragStart;
151+
nextFragmentY = fragment.y;
146152
}
147153
}
148154
if (pageIndex != null) break;
@@ -151,6 +157,7 @@ export async function goToAnchor({
151157
// Use the page of the next fragment if bookmark is in a gap
152158
if (pageIndex == null && nextFragmentPage != null) {
153159
pageIndex = nextFragmentPage;
160+
fragmentY = nextFragmentY;
154161
}
155162
}
156163

@@ -160,9 +167,25 @@ export async function goToAnchor({
160167
scrollPageIntoView(pageIndex);
161168
await waitForPageMount(pageIndex, timeoutMs);
162169

163-
// Scroll the page element into view
170+
// Scroll to the precise position within the page using the fragment Y offset.
171+
// We use the passed-in scrollContainer rather than discovering it via DOM traversal,
172+
// because intermediate elements (like painterHost) may have overflow CSS but are
173+
// not the actual scroll viewport.
164174
const pageEl = getPageElementByIndex(painterHost, pageIndex);
165-
if (pageEl) {
175+
176+
if (pageEl && fragmentY != null) {
177+
if (scrollContainer instanceof Element) {
178+
const pageRect = pageEl.getBoundingClientRect();
179+
const containerRect = scrollContainer.getBoundingClientRect();
180+
const targetY = pageRect.top - containerRect.top + scrollContainer.scrollTop + fragmentY;
181+
scrollContainer.scrollTo({ top: targetY, behavior: 'instant' });
182+
} else {
183+
// Window scroll
184+
const pageRect = pageEl.getBoundingClientRect();
185+
const targetY = pageRect.top + scrollContainer.scrollY + fragmentY;
186+
scrollContainer.scrollTo({ top: targetY, behavior: 'instant' });
187+
}
188+
} else if (pageEl) {
166189
pageEl.scrollIntoView({ behavior: 'instant', block: 'start' });
167190
}
168191

@@ -171,8 +194,6 @@ export async function goToAnchor({
171194
if (activeEditor?.commands?.setTextSelection) {
172195
activeEditor.commands.setTextSelection({ from: pmPos, to: pmPos });
173196
} else {
174-
// Navigation succeeded visually (page scrolled), but caret positioning is unavailable
175-
// This is not an error - log a warning for debugging
176197
console.warn(
177198
'[PresentationEditor] goToAnchor: Navigation succeeded but could not move caret (editor commands unavailable)',
178199
);

0 commit comments

Comments
 (0)