feat: bookmark api for cross session document addressing (SD-2358)#2663
feat: bookmark api for cross session document addressing (SD-2358)#2663luccas-harbour wants to merge 32 commits intomainfrom
Conversation
…ress type Extend BookmarkAddress, CommentAddress, and TrackedChangeAddress with an optional story locator for cross-story targeting. Add NavigableEntityAddress union type and story validation in bookmark ops.
Bookmark list/get/insert/rename/remove now resolve a story runtime, enabling bookmarks in footnotes, endnotes, and other non-body stories. Addresses include the story locator for non-body contexts.
…cked-change navigation Introduces PresentationEditor.navigateTo() that routes to the correct navigation strategy by entity type. Bookmarks in non-body stories resolve through story runtimes with header/footer region activation. Adds resolveAnchorPosition fallback for bookmarks missing from layout.
Delegates to the active document's PresentationEditor for bookmark, comment, and tracked-change navigation. Resolves the presentation editor via active document ID with fallback to first available.
Cover bookmark, comment, tracked-change, and header-story bookmark navigation through the SuperDoc.navigateTo API using Playwright.
…target.entityType
… reference in bookmarks.list
…ailure paths The cleanup that exits header/footer mode only ran inside the catch block, so non-exceptional early returns (editor timeout, missing setTextSelection) left the editor stuck in header/footer mode. Wrap post-activation logic in try/finally to guarantee cleanup.
…ross-story collisions allocateBookmarkId only scanned the story-local doc, so inserting a bookmark in a header could reuse an ID already present in the body. Reuse findAllBookmarksInDocument to scan all stories, matching the scope of the existing name-uniqueness check.
luccas-harbour
force-pushed
the
luccas/sd-2358-bookmark-api-for-cross-session-document-addressing
branch
from
88c2c39 to
ebdcbdd
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 88c2c39c2e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
luccas-harbour
requested review from
VladaHarbour,
artem-harbour,
caio-pizzol,
harbournick and
tupizz
…tions - bookmarksListWrapper: body listing, story-scoped listing, pagination - bookmarksGetWrapper: body resolve, story-qualified header resolve - resolveBookmarkTarget: found and not-found cases - extractBookmarkInfo: range positions, tableColumn, story-qualified address - findAllBookmarks: paired ends, orphaned starts - normalizeStory / buildBookmarkAddress: body normalization, non-body passthrough
There was a problem hiding this comment.
@luccas-harbour solid work — the cross-story bookmark API is well structured and tests are thorough.
one small correctness thing: #exitHeaderFooterMode() can get called twice when header/footer navigation fails. left an inline comment with a fix.
one cleanup suggestion on duplicated test helpers.
also opened SD-2454 for showing bookmark bracket indicators in the document (like Word's "Show bookmarks" toggle) — not needed for this PR but a nice follow-up.
left a couple inline comments.
| runtime.editor.view?.focus?.(); | ||
| return true; | ||
| } catch (error) { | ||
| this.#exitHeaderFooterMode(); |
There was a problem hiding this comment.
when #waitForHeaderFooterEditor throws, #exitHeaderFooterMode() runs once in the finally and again here in the outer catch. the session exit itself is safe (it checks if already in body mode), but the second call still triggers an extra rerender and focus. could either guard #exitHeaderFooterMode() to skip if already in body mode, or drop this call since finally already handles it.
| this.#exitHeaderFooterMode(); | |
| console.error('[PresentationEditor] navigateTo bookmark failed:', error); |
| variant: entry.variant ?? 'default', | ||
| } as any; | ||
|
|
||
| const toRanges = (item: any) => { |
There was a problem hiding this comment.
the toRanges helper here and in navigate-to-entity.spec.ts do the same thing but slightly differently — the other one also handles a legacy fallback. worth pulling into a shared helper in tests/behavior/helpers/document-api.ts?
3 participants
This PR adds cross-session, story-aware document addressing for bookmarks and introduces a higher-level navigation API for bookmarks, comments, and tracked changes.
On the document API side, bookmark addresses can now include a
story, andbookmarks.list()can be scoped within. The super-editor adapters now resolve bookmark operations against the correct story runtime, including header/footer stories, and return story-qualified bookmark addresses for non-body stories. At the same time, this branch restores document-wide bookmark name uniqueness so we remain compliant with OOXML even when bookmarks are created outside the body.On the navigation side,
SuperDoc.navigateTo()now supports entity-based navigation for:For bookmarks, anchor navigation can now fall back to resolving the target from the active ProseMirror document when the layout bookmark map is insufficient. For header/footer bookmark navigation, the editor activates the target region, waits for the live sub-editor, and places the caret at the resolved bookmark position.
This branch also updates the document-api contract/types/schema surface to expose these capabilities, adds conformance and unit coverage for story-aware bookmark handling, and adds behavior tests covering: