From 6c7fc43506fdc4312b69cca9ee6602e4b7b58870 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Wed, 29 Apr 2026 07:06:24 -0300 Subject: [PATCH 1/2] chore(document-api): remove UI-shaped surfaces (SD-2795) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `editor.doc.*` is the request/response Document API contract. UI-shaped surfaces (subscriptions, viewport scroll, geometry, focus) belong on the browser-only `superdoc/ui` controller. This drops the two members that were already on the contract-parity exemption list: - `editor.doc.selection.onChange` — push subscription. Replacement is `createSuperDocUI({ superdoc }).select(s => s.selection, ...)` on the controller's selector substrate. - `editor.doc.ranges.scrollIntoView` — viewport scroll, browser-only. Replacement is `ui.viewport.scrollIntoView(input)` (also called by `ui.comments.scrollTo` and `ui.review.scrollTo`). Both shipped only in `v1.30.0-next.*` / `v1.28.0-next.*` pre-release tags — no stable release contains them — so the cut is clean and needs no deprecation cycle. Engine wiring - `SelectionAdapter` is now `current()` only. - `SelectionApi` is now `current()` only. - `RangesApi` / `RangesAdapter` are `resolve()` only. - `executeScrollIntoView`, `RangeScrollAdapter`, and `SelectionChangeListener` are deleted. - `assembleDocumentApiAdapters` no longer wires `selection.onChange` or `ranges.scrollIntoView`. - `subscribeToSelection` removed from `selection-info-resolver`; the pure read path `resolveCurrentSelectionInfo` stays. Relocation - Scroll resolution moves to `packages/super-editor/src/ui/scroll-into-view.ts`. `ui.viewport.scrollIntoView`, `ui.comments.scrollTo`, and `ui.review.scrollTo` call it directly through the routed presentation editor. - `ScrollIntoViewInput` / `ScrollIntoViewOutput` value types stay in doc-api — they describe the request shape `superdoc/ui` consumers marshal across all three handles. Parity check `META_MEMBER_PATHS` in `check-contract-parity.ts` now contains only the dispatcher (`invoke`) and reference aliases. Verified locally: "contract parity check passed (389 operations, 389 API members)". Tests - 11954 super-editor pass; 70 ui-domain pass. - 1381 doc-api pass. - Stubs in `comments.test.ts`, `review.test.ts`, `viewport.test.ts` updated to wire `presentationEditor.navigateTo` instead of the removed `editor.doc.ranges.scrollIntoView`. - `consumer-typecheck` smoke test for scroll moved to the `ViewportHandle` shape. Generated artifacts Regenerated via `pnpm run generate:all`. SDK tool catalogs / contract manifest pick up the smaller surface automatically — no operations added or removed (neither member was an OPERATION_DEFINITIONS entry). --- .../scripts/check-contract-parity.ts | 22 +- packages/document-api/src/index.test.ts | 12 - packages/document-api/src/index.ts | 50 +--- packages/document-api/src/ranges/index.ts | 2 - .../document-api/src/ranges/ranges.types.ts | 23 +- .../src/ranges/scroll-into-view.test.ts | 175 -------------- .../src/ranges/scroll-into-view.ts | 73 ------ .../document-api/src/selection/selection.ts | 23 -- .../assemble-adapters.test.ts | 4 - .../assemble-adapters.ts | 5 +- .../helpers/scroll-into-view-adapter.test.ts | 218 ------------------ .../helpers/scroll-into-view-adapter.ts | 87 ------- .../helpers/selection-info-resolver.test.ts | 113 +-------- .../helpers/selection-info-resolver.ts | 68 +----- packages/super-editor/src/index.ts | 1 - packages/super-editor/src/ui/comments.test.ts | 31 ++- .../src/ui/create-super-doc-ui.ts | 29 ++- packages/super-editor/src/ui/review.test.ts | 29 ++- .../super-editor/src/ui/scroll-into-view.ts | 84 +++++++ packages/super-editor/src/ui/types.ts | 27 +-- packages/super-editor/src/ui/viewport.test.ts | 20 +- packages/superdoc/src/index.js | 1 - .../src/customer-scenario.ts | 49 ++-- 23 files changed, 196 insertions(+), 950 deletions(-) delete mode 100644 packages/document-api/src/ranges/scroll-into-view.test.ts delete mode 100644 packages/document-api/src/ranges/scroll-into-view.ts delete mode 100644 packages/super-editor/src/editors/v1/document-api-adapters/helpers/scroll-into-view-adapter.test.ts delete mode 100644 packages/super-editor/src/editors/v1/document-api-adapters/helpers/scroll-into-view-adapter.ts create mode 100644 packages/super-editor/src/ui/scroll-into-view.ts diff --git a/packages/document-api/scripts/check-contract-parity.ts b/packages/document-api/scripts/check-contract-parity.ts index 34efc7c107..e8c51fca4f 100644 --- a/packages/document-api/scripts/check-contract-parity.ts +++ b/packages/document-api/scripts/check-contract-parity.ts @@ -22,26 +22,10 @@ import { OPERATION_REFERENCE_DOC_PATH_MAP } from '../src/contract/reference-doc- import { buildDispatchTable } from '../src/invoke/invoke.js'; /** - * Meta-methods and helper methods on DocumentApi that are not contract - * operations: - * - * - `ranges.scrollIntoView` is a browser-only UI side-effect (scrolls - * the viewport via the presentation editor). It has no headless - * implementation, so it is intentionally excluded from the RPC - * dispatch surface and the CLI command catalog. Direct calls through - * `editor.doc.ranges.scrollIntoView()` are still supported. - * - `selection.onChange` is a subscription primitive (push-based, no - * request/response shape) rather than a request-response operation, - * so it is not represented in `OPERATION_DEFINITIONS` / schemas / - * dispatch. Direct calls through `editor.doc.selection.onChange()` - * are still supported. + * Meta-methods on DocumentApi that are not contract operations: the + * dispatcher itself plus the documented reference aliases. */ -const META_MEMBER_PATHS = [ - 'invoke', - 'ranges.scrollIntoView', - 'selection.onChange', - ...REFERENCE_OPERATION_ALIASES.map((alias) => alias.memberPath), -]; +const META_MEMBER_PATHS = ['invoke', ...REFERENCE_OPERATION_ALIASES.map((alias) => alias.memberPath)]; function collectFunctionMemberPaths(value: unknown, prefix = ''): string[] { if (!value || typeof value !== 'object') return []; diff --git a/packages/document-api/src/index.test.ts b/packages/document-api/src/index.test.ts index ff63295362..2e27a3c8df 100644 --- a/packages/document-api/src/index.test.ts +++ b/packages/document-api/src/index.test.ts @@ -2173,18 +2173,6 @@ describe('createDocumentApi', () => { expect(e.code).toBe('SELECTION_ADAPTER_UNAVAILABLE'); } }); - - it('throws SELECTION_ADAPTER_UNAVAILABLE when selection.onChange is called without a selection adapter', () => { - const api = makeApiWithoutSelection(); - try { - api.selection.onChange(() => {}); - expect.fail('expected SELECTION_ADAPTER_UNAVAILABLE to be thrown'); - } catch (err: unknown) { - const e = err as { name: string; code: string }; - expect(e.name).toBe('DocumentApiValidationError'); - expect(e.code).toBe('SELECTION_ADAPTER_UNAVAILABLE'); - } - }); }); describe('comments.patch target validation', () => { diff --git a/packages/document-api/src/index.ts b/packages/document-api/src/index.ts index a30988b4a3..aa1724e5f1 100644 --- a/packages/document-api/src/index.ts +++ b/packages/document-api/src/index.ts @@ -28,16 +28,9 @@ export type { RangeResolverAdapter, ScrollIntoViewInput, ScrollIntoViewOutput, - RangeScrollAdapter, } from './ranges/index.js'; -export { executeResolveRange, executeScrollIntoView } from './ranges/index.js'; -export type { - SelectionApi, - SelectionAdapter, - SelectionCurrentInput, - SelectionInfo, - SelectionChangeListener, -} from './selection/selection.js'; +export { executeResolveRange } from './ranges/index.js'; +export type { SelectionApi, SelectionAdapter, SelectionCurrentInput, SelectionInfo } from './selection/selection.js'; export { executeSelectionCurrent } from './selection/selection.js'; export type { HeaderFootersAdapter, HeaderFootersApi } from './header-footers/header-footers.js'; export * from './header-footers/header-footers.types.js'; @@ -136,22 +129,9 @@ import { import type { InsertInput } from './insert/insert.js'; import { executeDelete } from './delete/delete.js'; import { executeResolveRange } from './ranges/resolve.js'; -import { executeScrollIntoView } from './ranges/scroll-into-view.js'; -import type { - RangeResolverAdapter, - ResolveRangeInput, - ResolveRangeOutput, - ScrollIntoViewInput, - ScrollIntoViewOutput, -} from './ranges/ranges.types.js'; +import type { RangeResolverAdapter, ResolveRangeInput, ResolveRangeOutput } from './ranges/ranges.types.js'; import { executeSelectionCurrent } from './selection/selection.js'; -import type { - SelectionApi, - SelectionAdapter, - SelectionCurrentInput, - SelectionInfo, - SelectionChangeListener, -} from './selection/selection.js'; +import type { SelectionApi, SelectionAdapter, SelectionCurrentInput, SelectionInfo } from './selection/selection.js'; import { executeInsert } from './insert/insert.js'; import type { ListsAdapter, ListsApi } from './lists/lists.js'; import type { @@ -1507,19 +1487,10 @@ export interface MutationsApi { export interface RangesApi { resolve(input: ResolveRangeInput): ResolveRangeOutput; - /** - * Scroll the editor viewport so the target range is visible. Handles - * paginated, virtualized layouts by mounting the target page on demand. - * Async — resolves once the scroll settles (or the page-mount timeout - * expires). Target accepts `TextAddress`, `TextTarget`, or `EntityAddress` - * (comment / tracked change by id). - */ - scrollIntoView(input: ScrollIntoViewInput): Promise; } export interface RangesAdapter { resolve(input: ResolveRangeInput): ResolveRangeOutput; - scrollIntoView(input: ScrollIntoViewInput): Promise; } export interface QueryAdapter { @@ -3183,9 +3154,6 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { resolve(input: ResolveRangeInput): ResolveRangeOutput { return executeResolveRange(adapters.ranges, input); }, - scrollIntoView(input: ScrollIntoViewInput): Promise { - return executeScrollIntoView(adapters.ranges, input); - }, }, selection: { current(input?: SelectionCurrentInput): SelectionInfo { @@ -3198,16 +3166,6 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { } return executeSelectionCurrent(adapter, input); }, - onChange(listener: SelectionChangeListener): () => void { - const adapter = adapters.selection; - if (!adapter) { - throw new DocumentApiValidationError( - 'SELECTION_ADAPTER_UNAVAILABLE', - 'No selection adapter was registered. Pass `selection` in DocumentApiAdapters to call selection.onChange().', - ); - } - return adapter.onChange(listener); - }, }, mutations: { preview(input: MutationsPreviewInput): MutationsPreviewOutput { diff --git a/packages/document-api/src/ranges/index.ts b/packages/document-api/src/ranges/index.ts index 2c4aa60b53..578efc0c1d 100644 --- a/packages/document-api/src/ranges/index.ts +++ b/packages/document-api/src/ranges/index.ts @@ -10,7 +10,5 @@ export type { RangeResolverAdapter, ScrollIntoViewInput, ScrollIntoViewOutput, - RangeScrollAdapter, } from './ranges.types.js'; export { executeResolveRange } from './resolve.js'; -export { executeScrollIntoView } from './scroll-into-view.js'; diff --git a/packages/document-api/src/ranges/ranges.types.ts b/packages/document-api/src/ranges/ranges.types.ts index b5d69c7173..7d51bf7d18 100644 --- a/packages/document-api/src/ranges/ranges.types.ts +++ b/packages/document-api/src/ranges/ranges.types.ts @@ -122,13 +122,14 @@ export interface RangeResolverAdapter { } // --------------------------------------------------------------------------- -// scrollIntoView +// scrollIntoView — input/output value types // --------------------------------------------------------------------------- /** - * Input for `ranges.scrollIntoView` — scrolls the editor viewport so the - * given text target is visible. Handles paginated, virtualized layouts by - * mounting the target page if it isn't yet in the DOM. + * Input for `ui.viewport.scrollIntoView` — scrolls the editor + * viewport so the given target is visible. Handles paginated, + * virtualized layouts by mounting the target page if it isn't yet in + * the DOM. */ export interface ScrollIntoViewInput { /** @@ -146,18 +147,10 @@ export interface ScrollIntoViewInput { } /** - * Result of `ranges.scrollIntoView`. - * `success: false` when the target couldn't be resolved or a page failed to - * mount within the navigation timeout. + * Result of `ui.viewport.scrollIntoView`. `success: false` when the + * target couldn't be resolved or a page failed to mount within the + * navigation timeout. */ export interface ScrollIntoViewOutput { success: boolean; } - -/** - * Adapter method for `ranges.scrollIntoView`. Async because virtualized - * pages may need to mount before the scroll completes. - */ -export interface RangeScrollAdapter { - scrollIntoView(input: ScrollIntoViewInput): Promise; -} diff --git a/packages/document-api/src/ranges/scroll-into-view.test.ts b/packages/document-api/src/ranges/scroll-into-view.test.ts deleted file mode 100644 index c9eaa0d3b5..0000000000 --- a/packages/document-api/src/ranges/scroll-into-view.test.ts +++ /dev/null @@ -1,175 +0,0 @@ -import { describe, expect, it, mock } from 'bun:test'; -import { executeScrollIntoView } from './scroll-into-view.js'; -import type { RangeScrollAdapter, ScrollIntoViewInput, ScrollIntoViewOutput } from './ranges.types.js'; - -function makeAdapter(output: ScrollIntoViewOutput = { success: true }): RangeScrollAdapter & { - scrollIntoView: ReturnType; -} { - const scrollIntoView = mock(async () => output); - return { scrollIntoView } as unknown as RangeScrollAdapter & { scrollIntoView: ReturnType }; -} - -async function expectValidationError(fn: () => Promise, code: string, messageMatch: string): Promise { - try { - await fn(); - throw new Error(`expected ${code}, nothing thrown`); - } catch (err: unknown) { - const e = err as { name?: string; code?: string; message?: string }; - expect(e.name).toBe('DocumentApiValidationError'); - expect(e.code).toBe(code); - expect(e.message ?? '').toContain(messageMatch); - } -} - -describe('executeScrollIntoView — validation', () => { - it('rejects a null / undefined input', async () => { - const adapter = makeAdapter(); - await expectValidationError( - () => executeScrollIntoView(adapter, null as unknown as ScrollIntoViewInput), - 'INVALID_INPUT', - 'non-null object', - ); - await expectValidationError( - () => executeScrollIntoView(adapter, undefined as unknown as ScrollIntoViewInput), - 'INVALID_INPUT', - 'non-null object', - ); - }); - - it('rejects inputs with unknown fields', async () => { - const adapter = makeAdapter(); - await expectValidationError( - () => - executeScrollIntoView(adapter, { - target: { kind: 'text', blockId: 'p1', range: { start: 0, end: 5 } }, - somethingElse: true, - } as unknown as ScrollIntoViewInput), - 'INVALID_INPUT', - 'Unknown field', - ); - }); - - it('rejects when target is missing', async () => { - const adapter = makeAdapter(); - await expectValidationError( - () => executeScrollIntoView(adapter, {} as unknown as ScrollIntoViewInput), - 'INVALID_TARGET', - 'requires a target', - ); - }); - - it('rejects when target is malformed', async () => { - const adapter = makeAdapter(); - await expectValidationError( - () => executeScrollIntoView(adapter, { target: { kind: 'nope' } } as unknown as ScrollIntoViewInput), - 'INVALID_TARGET', - 'TextAddress, TextTarget, or EntityAddress', - ); - }); - - it('rejects entity targets with unknown entityType', async () => { - const adapter = makeAdapter(); - await expectValidationError( - () => - executeScrollIntoView(adapter, { - target: { kind: 'entity', entityType: 'mystery', entityId: 'x_1' }, - } as unknown as ScrollIntoViewInput), - 'INVALID_TARGET', - 'TextAddress, TextTarget, or EntityAddress', - ); - }); - - it('rejects entity targets with empty entityId', async () => { - const adapter = makeAdapter(); - await expectValidationError( - () => - executeScrollIntoView(adapter, { - target: { kind: 'entity', entityType: 'comment', entityId: '' }, - } as unknown as ScrollIntoViewInput), - 'INVALID_TARGET', - 'TextAddress, TextTarget, or EntityAddress', - ); - }); - - it('rejects block outside the allowed enum', async () => { - const adapter = makeAdapter(); - await expectValidationError( - () => - executeScrollIntoView(adapter, { - target: { kind: 'text', blockId: 'p1', range: { start: 0, end: 5 } }, - block: 'top' as 'start', - }), - 'INVALID_INPUT', - 'block must be', - ); - }); - - it('rejects behavior outside the allowed enum', async () => { - const adapter = makeAdapter(); - await expectValidationError( - () => - executeScrollIntoView(adapter, { - target: { kind: 'text', blockId: 'p1', range: { start: 0, end: 5 } }, - behavior: 'instant' as 'auto', - }), - 'INVALID_INPUT', - 'behavior must be', - ); - }); -}); - -describe('executeScrollIntoView — delegation', () => { - it('accepts a TextAddress target and forwards it unchanged', async () => { - const adapter = makeAdapter(); - const input: ScrollIntoViewInput = { - target: { kind: 'text', blockId: 'p1', range: { start: 3, end: 9 } }, - }; - const out = await executeScrollIntoView(adapter, input); - expect(out).toEqual({ success: true }); - expect(adapter.scrollIntoView).toHaveBeenCalledWith(input); - }); - - it('accepts a multi-segment TextTarget target', async () => { - const adapter = makeAdapter(); - const input: ScrollIntoViewInput = { - target: { - kind: 'text', - segments: [ - { blockId: 'p1', range: { start: 0, end: 5 } }, - { blockId: 'p2', range: { start: 0, end: 3 } }, - ], - }, - block: 'start', - behavior: 'auto', - }; - const out = await executeScrollIntoView(adapter, input); - expect(out).toEqual({ success: true }); - expect(adapter.scrollIntoView).toHaveBeenCalledWith(input); - }); - - it('accepts an EntityAddress (comment) target', async () => { - const adapter = makeAdapter(); - const input: ScrollIntoViewInput = { - target: { kind: 'entity', entityType: 'comment', entityId: 'c_1' }, - }; - await executeScrollIntoView(adapter, input); - expect(adapter.scrollIntoView).toHaveBeenCalledWith(input); - }); - - it('accepts an EntityAddress (trackedChange) target', async () => { - const adapter = makeAdapter(); - const input: ScrollIntoViewInput = { - target: { kind: 'entity', entityType: 'trackedChange', entityId: 'tc_1' }, - }; - await executeScrollIntoView(adapter, input); - expect(adapter.scrollIntoView).toHaveBeenCalledWith(input); - }); - - it('returns whatever the adapter returns (e.g. success: false)', async () => { - const adapter = makeAdapter({ success: false }); - const out = await executeScrollIntoView(adapter, { - target: { kind: 'text', blockId: 'p1', range: { start: 0, end: 5 } }, - }); - expect(out).toEqual({ success: false }); - }); -}); diff --git a/packages/document-api/src/ranges/scroll-into-view.ts b/packages/document-api/src/ranges/scroll-into-view.ts deleted file mode 100644 index 0f58196719..0000000000 --- a/packages/document-api/src/ranges/scroll-into-view.ts +++ /dev/null @@ -1,73 +0,0 @@ -/** - * `ranges.scrollIntoView` operation — scrolls the editor so the target - * text range is visible. Handles paginated, virtualized layouts by mounting - * the target page on demand. - * - * Primitive for custom sidebars (comments, track changes, mentions) that - * need to navigate the document to a specific range on user interaction. - */ - -import type { RangeScrollAdapter, ScrollIntoViewInput, ScrollIntoViewOutput } from './ranges.types.js'; -import { DocumentApiValidationError } from '../errors.js'; -import { isRecord, isTextAddress, isTextTarget, assertNoUnknownFields } from '../validation-primitives.js'; - -const VALID_ENTITY_TYPES: ReadonlySet = new Set(['comment', 'trackedChange']); - -function isEntityAddress(value: unknown): boolean { - if (!isRecord(value)) return false; - if (value.kind !== 'entity') return false; - if (typeof value.entityType !== 'string' || !VALID_ENTITY_TYPES.has(value.entityType)) return false; - if (typeof value.entityId !== 'string' || value.entityId.length === 0) return false; - return true; -} - -const SCROLL_INTO_VIEW_ALLOWED_KEYS = new Set(['target', 'block', 'behavior']); -const VALID_BLOCK_VALUES: ReadonlySet = new Set(['start', 'center', 'end', 'nearest']); -const VALID_BEHAVIOR_VALUES: ReadonlySet = new Set(['auto', 'smooth']); - -function validateScrollIntoViewInput(input: unknown): asserts input is ScrollIntoViewInput { - if (!isRecord(input)) { - throw new DocumentApiValidationError('INVALID_INPUT', 'ranges.scrollIntoView input must be a non-null object.'); - } - - assertNoUnknownFields(input, SCROLL_INTO_VIEW_ALLOWED_KEYS, 'ranges.scrollIntoView'); - - const { target, block, behavior } = input; - - if (target === undefined || target === null) { - throw new DocumentApiValidationError('INVALID_TARGET', 'ranges.scrollIntoView requires a target.', { - field: 'target', - }); - } - if (!isTextAddress(target) && !isTextTarget(target) && !isEntityAddress(target)) { - throw new DocumentApiValidationError( - 'INVALID_TARGET', - 'target must be a TextAddress, TextTarget, or EntityAddress object.', - { field: 'target', value: target }, - ); - } - - if (block !== undefined && (typeof block !== 'string' || !VALID_BLOCK_VALUES.has(block))) { - throw new DocumentApiValidationError( - 'INVALID_INPUT', - `block must be one of "start" | "center" | "end" | "nearest", got ${JSON.stringify(block)}.`, - { field: 'block', value: block }, - ); - } - - if (behavior !== undefined && (typeof behavior !== 'string' || !VALID_BEHAVIOR_VALUES.has(behavior))) { - throw new DocumentApiValidationError( - 'INVALID_INPUT', - `behavior must be "auto" or "smooth", got ${JSON.stringify(behavior)}.`, - { field: 'behavior', value: behavior }, - ); - } -} - -export async function executeScrollIntoView( - adapter: RangeScrollAdapter, - input: ScrollIntoViewInput, -): Promise { - validateScrollIntoViewInput(input); - return adapter.scrollIntoView(input); -} diff --git a/packages/document-api/src/selection/selection.ts b/packages/document-api/src/selection/selection.ts index 05e48608d9..d1ee218918 100644 --- a/packages/document-api/src/selection/selection.ts +++ b/packages/document-api/src/selection/selection.ts @@ -13,25 +13,12 @@ import { isRecord, assertNoUnknownFields } from '../validation-primitives.js'; export type { SelectionCurrentInput, SelectionInfo } from './selection.types.js'; -/** - * Callback invoked whenever the editor's selection changes, with the - * current {@link SelectionInfo}. Consumers may dedupe on `target` / - * `activeMarks` identity if their UI doesn't need every transaction. - */ -export type SelectionChangeListener = (info: SelectionInfo) => void; - /** * Engine-specific adapter for the selection API. */ export interface SelectionAdapter { /** Read the editor's current selection. */ current(input: SelectionCurrentInput): SelectionInfo; - /** - * Subscribe to selection changes. Returns an unsubscribe function. - * Implementations should debounce to at most once per tick to avoid - * storms during multi-step transactions. - */ - onChange(listener: SelectionChangeListener): () => void; } /** @@ -46,16 +33,6 @@ export interface SelectionApi { * pass the resulting `target` directly to `comments.create`. */ current(input?: SelectionCurrentInput): SelectionInfo; - - /** - * Subscribe to selection changes. The listener fires with a fresh - * {@link SelectionInfo} (as `includeText: false`) whenever the user's - * selection, stored marks, or containing block changes. - * - * Returns an unsubscribe function — call it in cleanup (React - * `useEffect` return, Vue `onUnmounted`, etc.). - */ - onChange(listener: SelectionChangeListener): () => void; } const SELECTION_CURRENT_ALLOWED_KEYS = new Set(['includeText']); diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/assemble-adapters.test.ts b/packages/super-editor/src/editors/v1/document-api-adapters/assemble-adapters.test.ts index bbde44f7f0..9aff5d2d76 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/assemble-adapters.test.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/assemble-adapters.test.ts @@ -99,9 +99,7 @@ describe('assembleDocumentApiAdapters', () => { expect(adapters).toHaveProperty('toc.update'); expect(adapters).toHaveProperty('toc.remove'); expect(adapters).toHaveProperty('ranges.resolve'); - expect(adapters).toHaveProperty('ranges.scrollIntoView'); expect(adapters).toHaveProperty('selection.current'); - expect(adapters).toHaveProperty('selection.onChange'); }); it('returns functions for all adapter methods', () => { @@ -132,8 +130,6 @@ describe('assembleDocumentApiAdapters', () => { expect(typeof adapters.toc.update).toBe('function'); expect(typeof adapters.toc.remove).toBe('function'); expect(typeof adapters.ranges.resolve).toBe('function'); - expect(typeof adapters.ranges.scrollIntoView).toBe('function'); expect(typeof adapters.selection!.current).toBe('function'); - expect(typeof adapters.selection!.onChange).toBe('function'); }); }); diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/assemble-adapters.ts b/packages/super-editor/src/editors/v1/document-api-adapters/assemble-adapters.ts index c08bf28159..760b0b615b 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/assemble-adapters.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/assemble-adapters.ts @@ -99,8 +99,7 @@ import { executePlan } from './plan-engine/executor.js'; import { previewPlan } from './plan-engine/preview.js'; import { queryMatchAdapter } from './plan-engine/query-match-adapter.js'; import { resolveRange } from './helpers/range-resolver.js'; -import { scrollRangeIntoView } from './helpers/scroll-into-view-adapter.js'; -import { resolveCurrentSelectionInfo, subscribeToSelection } from './helpers/selection-info-resolver.js'; +import { resolveCurrentSelectionInfo } from './helpers/selection-info-resolver.js'; import { initRevision, trackRevisions } from './plan-engine/revision-tracker.js'; import { initStoryRevisionStore } from './story-runtime/story-revision-store.js'; import { registerBuiltInExecutors } from './plan-engine/register-executors.js'; @@ -723,11 +722,9 @@ export function assembleDocumentApiAdapters(editor: Editor): DocumentApiAdapters }, ranges: { resolve: (input) => resolveRange(editor, input), - scrollIntoView: (input) => scrollRangeIntoView(editor, input), }, selection: { current: (input) => resolveCurrentSelectionInfo(editor, input), - onChange: (listener) => subscribeToSelection(editor, listener), }, query: { match: (input) => queryMatchAdapter(editor, input), diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/scroll-into-view-adapter.test.ts b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/scroll-into-view-adapter.test.ts deleted file mode 100644 index efb04b225c..0000000000 --- a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/scroll-into-view-adapter.test.ts +++ /dev/null @@ -1,218 +0,0 @@ -import { describe, expect, it, vi, beforeEach } from 'vitest'; -import type { Editor } from '../../core/Editor.js'; -import type { ScrollIntoViewInput } from '@superdoc/document-api'; - -vi.mock('./adapter-utils.js', async () => { - const actual = await vi.importActual('./adapter-utils.js'); - return { ...actual, resolveTextTarget: vi.fn() }; -}); - -import { resolveTextTarget } from './adapter-utils.js'; -import { scrollRangeIntoView } from './scroll-into-view-adapter.js'; - -function makeEditor( - presentationStub: { - scrollToPositionAsync?: ReturnType; - navigateTo?: ReturnType; - } | null = {}, -): Editor { - const presentation = presentationStub - ? { - scrollToPositionAsync: presentationStub.scrollToPositionAsync ?? vi.fn().mockResolvedValue(true), - navigateTo: presentationStub.navigateTo ?? vi.fn().mockResolvedValue(true), - } - : null; - return { presentationEditor: presentation } as unknown as Editor; -} - -describe('scrollRangeIntoView — TextAddress / TextTarget', () => { - beforeEach(() => { - vi.clearAllMocks(); - }); - - it('resolves a TextAddress target and delegates to scrollToPositionAsync', async () => { - vi.mocked(resolveTextTarget).mockReturnValue({ from: 42, to: 48 }); - const editor = makeEditor(); - const scroll = editor.presentationEditor!.scrollToPositionAsync as ReturnType; - - const out = await scrollRangeIntoView(editor, { - target: { kind: 'text', blockId: 'p1', range: { start: 3, end: 9 } }, - }); - - expect(out).toEqual({ success: true }); - expect(resolveTextTarget).toHaveBeenCalledWith(editor, { - kind: 'text', - blockId: 'p1', - range: { start: 3, end: 9 }, - }); - expect(scroll).toHaveBeenCalledWith(42, { block: 'center', behavior: 'smooth' }); - }); - - it('resolves a multi-segment TextTarget using the first segment', async () => { - vi.mocked(resolveTextTarget).mockReturnValue({ from: 100, to: 110 }); - const editor = makeEditor(); - const scroll = editor.presentationEditor!.scrollToPositionAsync as ReturnType; - - await scrollRangeIntoView(editor, { - target: { - kind: 'text', - segments: [ - { blockId: 'p1', range: { start: 2, end: 10 } }, - { blockId: 'p2', range: { start: 0, end: 5 } }, - ], - }, - }); - - // Only the FIRST segment is passed to resolveTextTarget — the helper - // scrolls to where the selection begins. - expect(resolveTextTarget).toHaveBeenCalledWith(editor, { - kind: 'text', - blockId: 'p1', - range: { start: 2, end: 10 }, - }); - expect(scroll).toHaveBeenCalledWith(100, { block: 'center', behavior: 'smooth' }); - }); - - it('passes through block and behavior options when provided', async () => { - vi.mocked(resolveTextTarget).mockReturnValue({ from: 1, to: 2 }); - const editor = makeEditor(); - const scroll = editor.presentationEditor!.scrollToPositionAsync as ReturnType; - - await scrollRangeIntoView(editor, { - target: { kind: 'text', blockId: 'p1', range: { start: 0, end: 1 } }, - block: 'start', - behavior: 'auto', - }); - - expect(scroll).toHaveBeenCalledWith(1, { block: 'start', behavior: 'auto' }); - }); - - it('returns { success: false } when the text target cannot be resolved', async () => { - vi.mocked(resolveTextTarget).mockReturnValue(null); - const editor = makeEditor(); - const scroll = editor.presentationEditor!.scrollToPositionAsync as ReturnType; - - const out = await scrollRangeIntoView(editor, { - target: { kind: 'text', blockId: 'missing', range: { start: 0, end: 1 } }, - }); - - expect(out).toEqual({ success: false }); - expect(scroll).not.toHaveBeenCalled(); - }); - - it('returns { success: false } when the presentation editor reports failure', async () => { - vi.mocked(resolveTextTarget).mockReturnValue({ from: 7, to: 9 }); - const editor = makeEditor({ scrollToPositionAsync: vi.fn().mockResolvedValue(false) }); - - const out = await scrollRangeIntoView(editor, { - target: { kind: 'text', blockId: 'p1', range: { start: 0, end: 2 } }, - }); - - expect(out).toEqual({ success: false }); - }); - - it('returns { success: false } when resolveTextTarget throws (ambiguous block id)', async () => { - // Production resolver throws `DocumentApiAdapterError` for ambiguous - // block IDs. The adapter must catch and convert to success: false - // rather than leak the error to the caller. - vi.mocked(resolveTextTarget).mockImplementation(() => { - throw new Error('Ambiguous block id'); - }); - const editor = makeEditor(); - - const out = await scrollRangeIntoView(editor, { - target: { kind: 'text', blockId: 'duplicated', range: { start: 0, end: 5 } }, - }); - - expect(out).toEqual({ success: false }); - }); - - it('returns { success: false } when scrollToPositionAsync rejects', async () => { - vi.mocked(resolveTextTarget).mockReturnValue({ from: 10, to: 15 }); - const editor = makeEditor({ - scrollToPositionAsync: vi.fn().mockRejectedValue(new Error('layout not ready')), - }); - - const out = await scrollRangeIntoView(editor, { - target: { kind: 'text', blockId: 'p1', range: { start: 0, end: 5 } }, - }); - - expect(out).toEqual({ success: false }); - }); -}); - -describe('scrollRangeIntoView — EntityAddress', () => { - beforeEach(() => { - vi.clearAllMocks(); - }); - - it('delegates a comment EntityAddress to presentation.navigateTo', async () => { - const navigateTo = vi.fn().mockResolvedValue(true); - const editor = makeEditor({ navigateTo }); - const input: ScrollIntoViewInput = { - target: { kind: 'entity', entityType: 'comment', entityId: 'c_1' }, - }; - - const out = await scrollRangeIntoView(editor, input); - - expect(out).toEqual({ success: true }); - expect(navigateTo).toHaveBeenCalledWith(input.target); - // Text-path helpers must not be invoked for entity targets. - expect(resolveTextTarget).not.toHaveBeenCalled(); - }); - - it('delegates a trackedChange EntityAddress (including story) to presentation.navigateTo', async () => { - const navigateTo = vi.fn().mockResolvedValue(true); - const editor = makeEditor({ navigateTo }); - const input: ScrollIntoViewInput = { - // trackedChange in a footnote story — story must reach navigateTo - // unchanged so it can activate the right editor surface before - // scrolling. - target: { - kind: 'entity', - entityType: 'trackedChange', - entityId: 'tc_42', - story: { storyType: 'footnote', refId: 'fn_1' }, - } as ScrollIntoViewInput['target'], - }; - - await scrollRangeIntoView(editor, input); - - expect(navigateTo).toHaveBeenCalledWith(input.target); - }); - - it('returns whatever navigateTo returns (e.g. success: false)', async () => { - const navigateTo = vi.fn().mockResolvedValue(false); - const editor = makeEditor({ navigateTo }); - - const out = await scrollRangeIntoView(editor, { - target: { kind: 'entity', entityType: 'comment', entityId: 'c_missing' }, - }); - - expect(out).toEqual({ success: false }); - }); - - it('returns { success: false } when navigateTo throws', async () => { - const navigateTo = vi.fn().mockRejectedValue(new Error('boom')); - const editor = makeEditor({ navigateTo }); - - const out = await scrollRangeIntoView(editor, { - target: { kind: 'entity', entityType: 'trackedChange', entityId: 'tc_boom' }, - }); - - expect(out).toEqual({ success: false }); - }); -}); - -describe('scrollRangeIntoView — presentation unavailable', () => { - it('returns { success: false } when the editor has no presentationEditor', async () => { - vi.mocked(resolveTextTarget).mockReturnValue({ from: 5, to: 10 }); - const editor = makeEditor(null); - - const out = await scrollRangeIntoView(editor, { - target: { kind: 'text', blockId: 'p1', range: { start: 0, end: 1 } }, - }); - - expect(out).toEqual({ success: false }); - }); -}); diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/scroll-into-view-adapter.ts b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/scroll-into-view-adapter.ts deleted file mode 100644 index b5d6b26260..0000000000 --- a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/scroll-into-view-adapter.ts +++ /dev/null @@ -1,87 +0,0 @@ -import type { ScrollIntoViewInput, ScrollIntoViewOutput } from '@superdoc/document-api'; -import type { Editor } from '../../core/Editor.js'; -import { resolveTextTarget } from './adapter-utils.js'; - -/** - * Implementation of `editor.doc.ranges.scrollIntoView`. - * - * Two paths: - * - EntityAddress (comment / tracked change by id) → delegates to - * `presentation.navigateTo(target)`, which handles paginated layouts, - * virtualized page mounting, AND story activation for entities in - * header/footer/footnote/endnote stories. `block` and `behavior` - * options are not applied here — `navigateTo` picks sensible viewport - * alignment per entity type. - * - TextAddress / TextTarget → resolves the first segment to a PM - * position and calls `scrollToPositionAsync` with caller-provided - * `block` / `behavior` options. This path is body-only today; text - * targets that reference non-body stories are out of scope for this - * operation. - * - * Both paths honor the `{ success: boolean }` contract: - * thrown errors from resolvers (e.g. ambiguous block IDs) and rejected - * scroll promises are caught and converted into `{ success: false }` - * rather than propagating to the caller. - * - * Known limitation: for a tracked change that lives in a non-body story - * (header, footer, footnote, endnote) on a page that is not currently - * mounted in the DOM (virtualized), `presentation.navigateTo` returns - * `false` — the non-body navigation path activates the story surface via - * rendered DOM candidates, and offscreen pages have none. This returns - * `{ success: false }`. A fix needs body-side reference resolution so the - * containing page can be pre-mounted; tracked as a follow-up. - */ -export async function scrollRangeIntoView(editor: Editor, input: ScrollIntoViewInput): Promise { - const presentation = editor.presentationEditor; - if (!presentation) { - return { success: false }; - } - - // EntityAddress path — hand off to the presentation editor so it can - // activate the right story (footnotes, header/footer) before scrolling. - if ('kind' in input.target && input.target.kind === 'entity') { - if (typeof presentation.navigateTo !== 'function') { - return { success: false }; - } - try { - const ok = await presentation.navigateTo(input.target); - return { success: ok }; - } catch { - return { success: false }; - } - } - - // TextAddress / TextTarget path — resolve to a PM position in the body - // and scroll directly. TextTarget resolves the FIRST segment, so a - // multi-block selection scrolls to where the selection begins. - if (typeof presentation.scrollToPositionAsync !== 'function') { - return { success: false }; - } - - try { - const firstSegment = - 'segments' in input.target - ? input.target.segments[0] - : { blockId: input.target.blockId, range: input.target.range }; - if (!firstSegment) return { success: false }; - - const resolved = resolveTextTarget(editor, { - kind: 'text', - blockId: firstSegment.blockId, - range: firstSegment.range, - }); - if (!resolved) return { success: false }; - - const ok = await presentation.scrollToPositionAsync(resolved.from, { - block: input.block ?? 'center', - behavior: input.behavior ?? 'smooth', - }); - return { success: ok }; - } catch { - // `resolveTextTarget` throws `DocumentApiAdapterError` for ambiguous - // block IDs; `scrollToPositionAsync` can reject on layout or mount - // failures. Convert either into `{ success: false }` so the caller - // sees a single predictable result type. - return { success: false }; - } -} diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/selection-info-resolver.test.ts b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/selection-info-resolver.test.ts index 2428c56479..b22d4bf9bb 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/selection-info-resolver.test.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/selection-info-resolver.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it, vi } from 'vitest'; import type { Node as ProseMirrorNode } from 'prosemirror-model'; import type { Editor } from '../../core/Editor.js'; -import { resolveCurrentSelectionInfo, subscribeToSelection } from './selection-info-resolver.js'; +import { resolveCurrentSelectionInfo } from './selection-info-resolver.js'; // --------------------------------------------------------------------------- // PM node stub builder @@ -324,114 +324,3 @@ describe('resolveCurrentSelectionInfo', () => { expect(elapsed).toBeLessThan(500); }); }); - -// --------------------------------------------------------------------------- -// subscribeToSelection -// --------------------------------------------------------------------------- - -describe('subscribeToSelection', () => { - it('fires the listener once per tick when selection updates', async () => { - const docNode = doc([textBlock('p1', 'Hi')]); - const editor = makeEditor(docNode, { from: 1, to: 3 }) as Editor & { __fire(event: string): void }; - const listener = vi.fn(); - - const unsubscribe = subscribeToSelection(editor, listener); - editor.__fire('selectionUpdate'); - editor.__fire('selectionUpdate'); - // Multiple events in one tick coalesce via queueMicrotask. - - await Promise.resolve(); // flush the microtask - expect(listener).toHaveBeenCalledTimes(1); - - unsubscribe(); - }); - - it('stops firing after unsubscribe', async () => { - const docNode = doc([textBlock('p1', 'Hi')]); - const editor = makeEditor(docNode, { from: 1, to: 3 }) as Editor & { __fire(event: string): void }; - const listener = vi.fn(); - - const unsubscribe = subscribeToSelection(editor, listener); - unsubscribe(); - editor.__fire('selectionUpdate'); - await Promise.resolve(); - - expect(listener).not.toHaveBeenCalled(); - }); - - it('cancels a microtask queued just before unsubscribe (no stale fire)', async () => { - // Regression: before the cancel flag, a microtask queued by the last - // pre-unmount event could still invoke the listener after unsubscribe - // returned — a classic source of stale state updates during React - // component unmount. - const docNode = doc([textBlock('p1', 'Hi')]); - const editor = makeEditor(docNode, { from: 1, to: 3 }) as Editor & { __fire(event: string): void }; - const listener = vi.fn(); - - const unsubscribe = subscribeToSelection(editor, listener); - editor.__fire('selectionUpdate'); // queues a microtask - unsubscribe(); // must mark the queued microtask as no-op - await Promise.resolve(); - - expect(listener).not.toHaveBeenCalled(); - }); - - it('dedupes events that produce identical SelectionInfo (typing without moving caret)', async () => { - // Regression: the `transaction` subscription is needed to catch - // programmatic selection changes that don't emit `selectionUpdate`, - // but it ALSO fires on every keystroke. Without content dedupe the - // listener ran per character even when the projected SelectionInfo - // was unchanged. Multiple ticks emitting the same selection state - // should fire the listener exactly once. - const docNode = doc([textBlock('p1', 'Hi')]); - const editor = makeEditor(docNode, { from: 1, to: 3 }) as Editor & { __fire(event: string): void }; - const listener = vi.fn(); - - const unsubscribe = subscribeToSelection(editor, listener); - - // First tick: a transaction event with the selection at [1, 3]. - editor.__fire('transaction'); - await Promise.resolve(); - expect(listener).toHaveBeenCalledTimes(1); - - // Second tick: another transaction with no selection change. - // Without dedupe this would re-fire; with dedupe it skips. - editor.__fire('transaction'); - await Promise.resolve(); - expect(listener).toHaveBeenCalledTimes(1); - - // Third tick: same again — still one call. - editor.__fire('transaction'); - await Promise.resolve(); - expect(listener).toHaveBeenCalledTimes(1); - - unsubscribe(); - }); - - it('fires again when SelectionInfo changes after a deduped tick', async () => { - // Dedupe must not become sticky: a real selection change after a - // deduped tick must still invoke the listener. - const docNode = doc([textBlock('p1', 'Hello')]); - const editor = makeEditor(docNode, { from: 1, to: 3 }) as Editor & { - __fire(event: string): void; - }; - const listener = vi.fn(); - - const unsubscribe = subscribeToSelection(editor, listener); - editor.__fire('transaction'); - await Promise.resolve(); - expect(listener).toHaveBeenCalledTimes(1); - - // Change the selection on the editor stub, then fire again. - (editor.state as { selection: { from: number; to: number; empty: boolean } }).selection = { - from: 2, - to: 4, - empty: false, - }; - editor.__fire('transaction'); - await Promise.resolve(); - expect(listener).toHaveBeenCalledTimes(2); - - unsubscribe(); - }); -}); diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/selection-info-resolver.ts b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/selection-info-resolver.ts index 33cead0642..60201cc8e9 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/selection-info-resolver.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/selection-info-resolver.ts @@ -1,11 +1,5 @@ import type { Node as ProseMirrorNode } from 'prosemirror-model'; -import type { - SelectionCurrentInput, - SelectionInfo, - SelectionChangeListener, - TextTarget, - TextSegment, -} from '@superdoc/document-api'; +import type { SelectionCurrentInput, SelectionInfo, TextTarget, TextSegment } from '@superdoc/document-api'; import type { Editor } from '../../core/Editor.js'; import { pmPositionToTextOffset } from './text-offset-resolver.js'; @@ -138,66 +132,6 @@ function collectActiveMarks( return Array.from(names); } -/** - * Subscribe to selection changes on the editor. - * - * - Microtask coalescing so a single user action that dispatches multiple - * PM transactions fires the listener at most once per tick. - * - Content dedupe so doc-only transactions (typing without moving the - * caret) don't re-fire the listener with an identical SelectionInfo. - * The transaction event is needed to catch programmatic selection - * changes that don't emit `selectionUpdate`, but it also fires on - * every keystroke; without dedupe the listener runs per character. - */ -export function subscribeToSelection(editor: Editor, listener: SelectionChangeListener): () => void { - let scheduled = false; - let cancelled = false; - let lastEmittedKey: string | null = null; - const flush = () => { - scheduled = false; - if (cancelled) return; - const info = resolveCurrentSelectionInfo(editor, {}); - const key = selectionInfoKey(info); - if (key === lastEmittedKey) return; - lastEmittedKey = key; - listener(info); - }; - const schedule = () => { - if (scheduled || cancelled) return; - scheduled = true; - queueMicrotask(flush); - }; - - editor.on('selectionUpdate', schedule); - editor.on('transaction', schedule); - - return () => { - // Mark cancelled first so a microtask already queued by `schedule` - // no-ops when it finally fires — otherwise the listener can be invoked - // after unsubscribe returns (stale state updates during unmount). - cancelled = true; - editor.off?.('selectionUpdate', schedule); - editor.off?.('transaction', schedule); - }; -} - -/** - * Build a stable string key from a SelectionInfo for content-dedupe. - * Two infos that produce the same key represent the same observable - * selection state — the listener can skip the second one. - */ -function selectionInfoKey(info: SelectionInfo): string { - const target = info.target; - let targetKey: string; - if (!target) { - targetKey = 'null'; - } else { - targetKey = target.segments.map((s) => `${s.blockId}:${s.range.start}-${s.range.end}`).join('|'); - } - const marks = [...info.activeMarks].sort().join(','); - return `${info.empty ? '1' : '0'}:${targetKey}:${marks}`; -} - function markTypesPresentEverywhere(doc: ProseMirrorNode, from: number, to: number): Set { // Intersect mark-name sets per text node, not per character. `selection. // onChange` fires frequently during editing, so allocating one Set per diff --git a/packages/super-editor/src/index.ts b/packages/super-editor/src/index.ts index 4b1d2c69be..6a776ec8e9 100644 --- a/packages/super-editor/src/index.ts +++ b/packages/super-editor/src/index.ts @@ -22,7 +22,6 @@ export type { SelectionApi, SelectionInfo, SelectionCurrentInput, - SelectionChangeListener, ScrollIntoViewInput, ScrollIntoViewOutput, TextAddress, diff --git a/packages/super-editor/src/ui/comments.test.ts b/packages/super-editor/src/ui/comments.test.ts index e7f19e170c..becef18afb 100644 --- a/packages/super-editor/src/ui/comments.test.ts +++ b/packages/super-editor/src/ui/comments.test.ts @@ -42,9 +42,17 @@ function makeStubs( })), page: { limit: 50, offset: 0, returned: commentsList.length }, })); - const scrollIntoView = vi.fn(async (_input: unknown) => ({ success: true as const })); - - const editor = { + const navigateTo = vi.fn(async (_target: unknown) => true); + + const editor: { + on: ReturnType; + off: ReturnType; + doc: unknown; + presentationEditor: { + navigateTo: typeof navigateTo; + getActiveEditor: () => unknown; + }; + } = { on: vi.fn((event: string, handler: (...args: unknown[]) => void) => { if (!editorListeners.has(event)) editorListeners.set(event, new Set()); editorListeners.get(event)!.add(handler); @@ -63,9 +71,12 @@ function makeStubs( })), }, comments: { create, patch, delete: del, list }, - ranges: { scrollIntoView }, }, + // Self-reference assigned below so toolbar source resolution sees + // the same routed editor as the rest of the stub. + presentationEditor: undefined as never, }; + editor.presentationEditor = { navigateTo, getActiveEditor: () => editor }; const superdoc: SuperDocLike & { fireEditor(event: string, ...args: unknown[]): void; @@ -90,7 +101,7 @@ function makeStubs( }, }; - return { superdoc, editor, mocks: { create, patch, delete: del, list, scrollIntoView } }; + return { superdoc, editor, mocks: { create, patch, delete: del, list, navigateTo } }; } const flushMicrotasks = () => Promise.resolve(); @@ -328,17 +339,15 @@ describe('ui.comments — actions route through editor.doc.*', () => { ui.destroy(); }); - it('scrollTo forwards to ranges.scrollIntoView with an EntityAddress', async () => { + it('scrollTo navigates to the comment EntityAddress via the presentation editor', async () => { const { superdoc, mocks } = makeStubs(); const ui = createSuperDocUI({ superdoc }); await ui.comments.scrollTo('c-42'); - expect(mocks.scrollIntoView).toHaveBeenCalledTimes(1); - const arg = mocks.scrollIntoView.mock.calls[0][0] as { - target: { kind: string; entityType: string; entityId: string }; - }; - expect(arg.target).toEqual({ kind: 'entity', entityType: 'comment', entityId: 'c-42' }); + expect(mocks.navigateTo).toHaveBeenCalledTimes(1); + const target = mocks.navigateTo.mock.calls[0][0] as { kind: string; entityType: string; entityId: string }; + expect(target).toEqual({ kind: 'entity', entityType: 'comment', entityId: 'c-42' }); ui.destroy(); }); diff --git a/packages/super-editor/src/ui/create-super-doc-ui.ts b/packages/super-editor/src/ui/create-super-doc-ui.ts index eb0a2aa82d..22c4ad491c 100644 --- a/packages/super-editor/src/ui/create-super-doc-ui.ts +++ b/packages/super-editor/src/ui/create-super-doc-ui.ts @@ -15,6 +15,7 @@ import type { TrackChangesListResult, } from '@superdoc/document-api'; import { shallowEqual } from './equality.js'; +import { scrollRangeIntoView } from './scroll-into-view.js'; import type { CommandHandle, CommandsHandle, @@ -671,13 +672,16 @@ export function createSuperDocUI(options: SuperDocUIOptions): SuperDocUI { return api; }; - const requireDocRanges = () => { + /** + * Run `scrollRangeIntoView` against whichever editor + * PresentationEditor currently routes to (body / header / footer / + * note). Returns `{ success: false }` when no routed editor is + * mounted. + */ + const runScrollIntoView = async (input: ScrollIntoViewInput): Promise => { const editor = resolveRoutedEditor(superdoc); - const api = editor?.doc?.ranges; - if (!api?.scrollIntoView) { - throw new Error('ui.comments.scrollTo: no active editor / ranges API.'); - } - return api; + if (!editor) return { success: false }; + return scrollRangeIntoView(editor as unknown as Parameters[0], input); }; const comments: CommentsHandle = { @@ -743,8 +747,7 @@ export function createSuperDocUI(options: SuperDocUIOptions): SuperDocUI { return receipt; }, async scrollTo(commentId) { - const api = requireDocRanges(); - return (api.scrollIntoView as (input: unknown) => Promise).call(api, { + return runScrollIntoView({ target: { kind: 'entity', entityType: 'comment', entityId: commentId }, block: 'center', behavior: 'smooth', @@ -862,10 +865,9 @@ export function createSuperDocUI(options: SuperDocUIOptions): SuperDocUI { async scrollTo(id) { const kind = entityKindForId(id); const entityType = kind === 'change' ? 'trackedChange' : 'comment'; - const api = requireDocRanges(); activeReviewId = id; scheduleNotify(); - return (api.scrollIntoView as (input: unknown) => Promise).call(api, { + return runScrollIntoView({ target: { kind: 'entity', entityType, entityId: id }, block: 'center', behavior: 'smooth', @@ -974,12 +976,7 @@ export function createSuperDocUI(options: SuperDocUIOptions): SuperDocUI { }, async scrollIntoView(input: ScrollIntoViewInput): Promise { - const editor = resolveRoutedEditor(superdoc); - const api = editor?.doc?.ranges; - if (!api?.scrollIntoView) { - return { success: false }; - } - return (api.scrollIntoView as (input: unknown) => Promise).call(api, input); + return runScrollIntoView(input); }, }; diff --git a/packages/super-editor/src/ui/review.test.ts b/packages/super-editor/src/ui/review.test.ts index 0f2a943bfe..33243853fa 100644 --- a/packages/super-editor/src/ui/review.test.ts +++ b/packages/super-editor/src/ui/review.test.ts @@ -64,10 +64,18 @@ function makeStubs( page: { limit: 50, offset: 0, returned: changesList.length }, })); const decide = vi.fn((_input: unknown) => ({ success: true as const })); - const scrollIntoView = vi.fn(async (_input: unknown) => ({ success: true as const })); + const navigateTo = vi.fn(async (_target: unknown) => true); const setDocumentMode = vi.fn(); - const editor = { + const editor: { + on: ReturnType; + off: ReturnType; + doc: unknown; + presentationEditor: { + navigateTo: typeof navigateTo; + getActiveEditor: () => unknown; + }; + } = { on: vi.fn((event: string, handler: (...args: unknown[]) => void) => { if (!editorListeners.has(event)) editorListeners.set(event, new Set()); editorListeners.get(event)!.add(handler); @@ -87,9 +95,12 @@ function makeStubs( }, comments: { list: listComments, create: vi.fn(), patch: vi.fn(), delete: vi.fn() }, trackChanges: { list: listChanges, decide }, - ranges: { scrollIntoView }, }, + // Self-reference assigned below so toolbar source resolution sees + // the same routed editor as the rest of the stub. + presentationEditor: undefined as never, }; + editor.presentationEditor = { navigateTo, getActiveEditor: () => editor }; const superdoc: SuperDocLike & { fireEditor(event: string, ...args: unknown[]): void; @@ -129,7 +140,7 @@ function makeStubs( }, }; - return { superdoc, editor, mocks: { listComments, listChanges, decide, scrollIntoView, setDocumentMode } }; + return { superdoc, editor, mocks: { listComments, listChanges, decide, navigateTo, setDocumentMode } }; } describe('ui.review — snapshot', () => { @@ -314,7 +325,7 @@ describe('ui.review — next/previous navigation', () => { }); describe('ui.review — scrollTo + setRecording', () => { - it('scrollTo(id) routes to ranges.scrollIntoView with the right entity type', async () => { + it('scrollTo(id) navigates to the right EntityAddress via the presentation editor', async () => { const { superdoc, mocks } = makeStubs({ comments: [{ id: 'c1', commentId: 'c1' }], trackedChanges: [{ id: 'tc1' }], @@ -322,12 +333,12 @@ describe('ui.review — scrollTo + setRecording', () => { const ui = createSuperDocUI({ superdoc }); await ui.review.scrollTo('c1'); - let arg = mocks.scrollIntoView.mock.calls[0][0] as { target: { entityType: string; entityId: string } }; - expect(arg.target).toEqual({ kind: 'entity', entityType: 'comment', entityId: 'c1' }); + let target = mocks.navigateTo.mock.calls[0][0] as { kind: string; entityType: string; entityId: string }; + expect(target).toEqual({ kind: 'entity', entityType: 'comment', entityId: 'c1' }); await ui.review.scrollTo('tc1'); - arg = mocks.scrollIntoView.mock.calls[1][0] as { target: { entityType: string; entityId: string } }; - expect(arg.target).toEqual({ kind: 'entity', entityType: 'trackedChange', entityId: 'tc1' }); + target = mocks.navigateTo.mock.calls[1][0] as { kind: string; entityType: string; entityId: string }; + expect(target).toEqual({ kind: 'entity', entityType: 'trackedChange', entityId: 'tc1' }); ui.destroy(); }); diff --git a/packages/super-editor/src/ui/scroll-into-view.ts b/packages/super-editor/src/ui/scroll-into-view.ts new file mode 100644 index 0000000000..42168438aa --- /dev/null +++ b/packages/super-editor/src/ui/scroll-into-view.ts @@ -0,0 +1,84 @@ +/** + * Viewport-scroll helper for `superdoc/ui`. Drives + * `presentation.navigateTo()` for entity targets (comment / + * tracked-change ids — story-aware) and + * `presentation.scrollToPositionAsync()` for text targets (body-only + * today). Used by `ui.viewport.scrollIntoView`, `ui.comments.scrollTo`, + * and `ui.review.scrollTo`. + */ + +import type { ScrollIntoViewInput, ScrollIntoViewOutput, TextAddress, TextTarget } from '@superdoc/document-api'; +import type { Editor } from '../editors/v1/core/Editor.js'; +import { resolveTextTarget } from '../editors/v1/document-api-adapters/helpers/adapter-utils.js'; + +/** + * Two paths: + * - EntityAddress (comment / tracked change by id) → delegates to + * `presentation.navigateTo(target)`, which handles paginated layouts, + * virtualized page mounting, AND story activation for entities in + * header/footer/footnote/endnote stories. `block` and `behavior` + * options are not applied here — `navigateTo` picks sensible viewport + * alignment per entity type. + * - TextAddress / TextTarget → resolves the first segment to a PM + * position and calls `scrollToPositionAsync` with caller-provided + * `block` / `behavior` options. This path is body-only today; text + * targets that reference non-body stories are out of scope. + * + * Both paths honor the `{ success: boolean }` contract: thrown errors + * from resolvers (e.g. ambiguous block IDs) and rejected scroll + * promises are caught and converted into `{ success: false }` rather + * than propagating. + * + * Known limitation: for a tracked change that lives in a non-body + * story (header, footer, footnote, endnote) on a page that is not + * currently mounted in the DOM (virtualized), + * `presentation.navigateTo` returns `false` — the non-body navigation + * path activates the story surface via rendered DOM candidates, and + * offscreen pages have none. Returns `{ success: false }` in that case. + */ +export async function scrollRangeIntoView(editor: Editor, input: ScrollIntoViewInput): Promise { + const presentation = editor.presentationEditor; + if (!presentation) { + return { success: false }; + } + + if ('kind' in input.target && (input.target as { kind?: unknown }).kind === 'entity') { + if (typeof presentation.navigateTo !== 'function') { + return { success: false }; + } + try { + const ok = await presentation.navigateTo(input.target); + return { success: Boolean(ok) }; + } catch { + return { success: false }; + } + } + + if (typeof presentation.scrollToPositionAsync !== 'function') { + return { success: false }; + } + + try { + const target = input.target as TextAddress | TextTarget; + const firstSegment = + 'segments' in target + ? target.segments[0] + : { blockId: (target as TextAddress).blockId, range: (target as TextAddress).range }; + if (!firstSegment) return { success: false }; + + const resolved = resolveTextTarget(editor, { + kind: 'text', + blockId: firstSegment.blockId, + range: firstSegment.range, + }); + if (!resolved) return { success: false }; + + const ok = await presentation.scrollToPositionAsync(resolved.from, { + block: input.block ?? 'center', + behavior: input.behavior ?? 'smooth', + }); + return { success: Boolean(ok) }; + } catch { + return { success: false }; + } +} diff --git a/packages/super-editor/src/ui/types.ts b/packages/super-editor/src/ui/types.ts index a86856bb45..176df083ab 100644 --- a/packages/super-editor/src/ui/types.ts +++ b/packages/super-editor/src/ui/types.ts @@ -79,10 +79,6 @@ export interface SuperDocEditorLike { patch?(input: unknown, options?: unknown): unknown; delete?(input: unknown, options?: unknown): unknown; }; - /** Ranges member on the Document API. Used for `ui.comments.scrollTo`. */ - ranges?: { - scrollIntoView?(input: unknown): Promise; - }; /** * Tracked-changes member on the Document API. Used by * `ui.review.*` for accept/reject and the merged feed. @@ -416,8 +412,8 @@ export interface CommentsHandle { delete(commentId: string): import('@superdoc/document-api').Receipt; /** * Scroll the viewport to the comment's anchor via - * `editor.doc.ranges.scrollIntoView({ target: EntityAddress })`. - * Resolves to the receipt the doc-API returns. + * `ui.viewport.scrollIntoView({ target: EntityAddress })`. Resolves + * to a `{ success: boolean }` receipt. */ scrollTo(commentId: string): Promise; } @@ -461,7 +457,7 @@ export interface ReviewHandle { /** * Scroll the viewport to the given item (comment or tracked * change) and set it as `activeId`. Routes through - * `editor.doc.ranges.scrollIntoView({ target: EntityAddress })`. + * `ui.viewport.scrollIntoView({ target: EntityAddress })`. */ scrollTo(id: string): Promise; /** @@ -494,8 +490,7 @@ export interface ViewportRect { export interface ViewportGetRectInput { /** - * The thing to look up. Mirrors `editor.doc.ranges.scrollIntoView`'s - * target shapes: + * The thing to look up. Same target shapes as `viewport.scrollIntoView`: * - `EntityAddress` (comment / tracked change by id) * - `TextAddress` (single-block text range) — deferred * - `TextTarget` (multi-segment text target) — deferred @@ -553,8 +548,8 @@ export type ViewportRectResult = * Valid target but currently virtualized / offscreen — the * page or story isn't painted in the DOM. Caller can call * `viewport.scrollIntoView` first to mount it, then retry. - * Same posture as `editor.doc.ranges.scrollIntoView` for - * non-body stories on virtualized pages (SD-2750). + * Same posture as the underlying scroll path for non-body + * stories on virtualized pages (SD-2750). */ | 'not-mounted'; }; @@ -572,10 +567,12 @@ export interface ViewportHandle { */ getRect(input: ViewportGetRectInput): ViewportRectResult; /** - * Scroll the viewport so the target is visible. Thin pass-through - * to `editor.doc.ranges.scrollIntoView` for parity with the rest - * of the controller surface, so consumers don't have to dip into - * `editor.doc` for scroll geometry that pairs with `getRect`. + * Scroll the viewport so the target is visible. Browser-only by + * definition: drives `presentation.navigateTo()` for entity targets + * (story-aware) and `presentation.scrollToPositionAsync()` for text + * targets. Lives on `ui.*` rather than `editor.doc.*` because + * viewport scroll is a UI side-effect, not a request/response + * Document API operation. */ scrollIntoView( input: import('@superdoc/document-api').ScrollIntoViewInput, diff --git a/packages/super-editor/src/ui/viewport.test.ts b/packages/super-editor/src/ui/viewport.test.ts index 2fa2e72c89..8a37dbb3a5 100644 --- a/packages/super-editor/src/ui/viewport.test.ts +++ b/packages/super-editor/src/ui/viewport.test.ts @@ -5,9 +5,8 @@ import type { SuperDocLike } from './types.js'; /** * Stub for `ui.viewport` tests. Models the minimal surface the - * controller calls: `editor.presentationEditor.getEntityRects` for - * geometry lookups, and `editor.doc.ranges.scrollIntoView` for the - * thin pass-through. + * controller calls: `presentationEditor.getEntityRects` for geometry + * lookups and `presentationEditor.navigateTo` for entity scroll. */ function makeStubs( initial: { @@ -31,7 +30,7 @@ function makeStubs( if (typeof target.entityId !== 'string') return []; return rectsById[target.entityId] ?? []; }); - const scrollIntoView = vi.fn(async (_input: unknown) => ({ success: true as const })); + const navigateTo = vi.fn(async (_target: unknown) => true); const editor: { on: ReturnType; @@ -40,6 +39,7 @@ function makeStubs( presentationEditor: | { getEntityRects: typeof getEntityRects; + navigateTo: typeof navigateTo; getActiveEditor: () => unknown; } | undefined; @@ -64,7 +64,6 @@ function makeStubs( page: { limit: 0, offset: 0, returned: 0 }, })), }, - ranges: { scrollIntoView }, }, presentationEditor: undefined, }; @@ -72,6 +71,7 @@ function makeStubs( // same stub editor the toolbar source resolver expects when present. editor.presentationEditor = { getEntityRects, + navigateTo, getActiveEditor: () => editor, }; @@ -82,7 +82,7 @@ function makeStubs( off: vi.fn(), }; - return { superdoc, editor, mocks: { getEntityRects, scrollIntoView } }; + return { superdoc, editor, mocks: { getEntityRects, navigateTo } }; } describe('ui.viewport.getRect — entity targets', () => { @@ -240,7 +240,7 @@ describe('ui.viewport.getRect — entity targets', () => { }); describe('ui.viewport.scrollIntoView', () => { - it('passes the input straight through to editor.doc.ranges.scrollIntoView', async () => { + it('navigates entity targets through the presentation editor', async () => { const { superdoc, mocks } = makeStubs(); const ui = createSuperDocUI({ superdoc }); @@ -252,13 +252,13 @@ describe('ui.viewport.scrollIntoView', () => { const result = await ui.viewport.scrollIntoView(input); expect(result).toEqual({ success: true }); - expect(mocks.scrollIntoView).toHaveBeenCalledWith(input); + expect(mocks.navigateTo).toHaveBeenCalledWith(input.target); ui.destroy(); }); - it('returns { success: false } when no editor / ranges API is available', async () => { + it('returns { success: false } when no presentation editor is mounted', async () => { const { superdoc } = makeStubs(); - (superdoc.activeEditor as unknown as { doc: { ranges: unknown } }).doc.ranges = undefined as never; + (superdoc.activeEditor as unknown as { presentationEditor: unknown }).presentationEditor = undefined; const ui = createSuperDocUI({ superdoc }); const result = await ui.viewport.scrollIntoView({ diff --git a/packages/superdoc/src/index.js b/packages/superdoc/src/index.js index f1fa825179..8316f0ec47 100644 --- a/packages/superdoc/src/index.js +++ b/packages/superdoc/src/index.js @@ -110,7 +110,6 @@ import { getSchemaIntrospection } from './helpers/schema-introspection.js'; * @typedef {import('@superdoc/super-editor').SelectionApi} SelectionApi * @typedef {import('@superdoc/super-editor').SelectionInfo} SelectionInfo * @typedef {import('@superdoc/super-editor').SelectionCurrentInput} SelectionCurrentInput - * @typedef {import('@superdoc/super-editor').SelectionChangeListener} SelectionChangeListener * @typedef {import('@superdoc/super-editor').ScrollIntoViewInput} ScrollIntoViewInput * @typedef {import('@superdoc/super-editor').ScrollIntoViewOutput} ScrollIntoViewOutput * @typedef {import('@superdoc/super-editor').TextTarget} TextTarget diff --git a/tests/consumer-typecheck/src/customer-scenario.ts b/tests/consumer-typecheck/src/customer-scenario.ts index aa3120a724..3a4b443a16 100644 --- a/tests/consumer-typecheck/src/customer-scenario.ts +++ b/tests/consumer-typecheck/src/customer-scenario.ts @@ -122,12 +122,11 @@ import type { SelectionApi, SelectionInfo, SelectionCurrentInput, - SelectionChangeListener, TextTarget, TextAddress, TextSegment, - // Ranges — scrollIntoView + // Viewport scroll (now exposed via ui.viewport.scrollIntoView) ScrollIntoViewInput, ScrollIntoViewOutput, EntityAddress, @@ -481,23 +480,21 @@ function testSelectionAPI(pe: PresentationEditor) { } // ============================================ -// SECTION 8c: Document API — ranges.scrollIntoView +// SECTION 8c: Viewport scroll — `ui.viewport.scrollIntoView` // ============================================ /** - * Smoke test for `editor.doc.ranges.scrollIntoView`. Consumers need to - * construct all three target shapes (TextAddress, TextTarget, EntityAddress) - * and await the returned Promise. The function is type-checked only — - * it is not called at runtime. + * Type-only smoke test for `ui.viewport.scrollIntoView`. Consumers + * construct `ScrollIntoViewInput` (TextAddress, TextTarget, or + * EntityAddress) and pass it to the viewport handle, which returns + * `Promise`. */ -async function testRangesScrollIntoView(editor: Editor) { - const api = (editor as any).doc.ranges as { - scrollIntoView(input: ScrollIntoViewInput): Promise; - }; - +async function testViewportScrollIntoView(viewport: { + scrollIntoView(input: ScrollIntoViewInput): Promise; +}) { // TextAddress — single-block target. const textAddress: TextAddress = { kind: 'text', blockId: 'p1', range: { start: 0, end: 10 } }; - const resTextAddr: ScrollIntoViewOutput = await api.scrollIntoView({ target: textAddress }); + const resTextAddr: ScrollIntoViewOutput = await viewport.scrollIntoView({ target: textAddress }); const successA: boolean = resTextAddr.success; void successA; @@ -507,7 +504,7 @@ async function testRangesScrollIntoView(editor: Editor) { kind: 'text', segments: [seg, { blockId: 'p2', range: { start: 0, end: 3 } }], }; - const resTextTarget: ScrollIntoViewOutput = await api.scrollIntoView({ + const resTextTarget: ScrollIntoViewOutput = await viewport.scrollIntoView({ target: textTarget, block: 'start', behavior: 'auto', @@ -521,8 +518,8 @@ async function testRangesScrollIntoView(editor: Editor) { entityType: 'trackedChange', entityId: 'tc_1', }; - await api.scrollIntoView({ target: commentAddr, behavior: 'smooth' }); - await api.scrollIntoView({ target: trackedAddr, block: 'center' }); + await viewport.scrollIntoView({ target: commentAddr, behavior: 'smooth' }); + await viewport.scrollIntoView({ target: trackedAddr, block: 'center' }); // Construct a full input object and pass it through — verifies the // combined type compiles for consumers who build inputs programmatically. @@ -531,7 +528,7 @@ async function testRangesScrollIntoView(editor: Editor) { block: 'nearest', behavior: 'auto', }; - await api.scrollIntoView(fullInput); + await viewport.scrollIntoView(fullInput); } // ============================================ @@ -593,19 +590,10 @@ function testDocSelectionPrimitives(editor: Editor) { void ta; void tt; - // Subscription: onChange returns an unsubscribe. Listener receives - // a SelectionInfo. The parameter is annotated explicitly because - // document-api types are surfaced via ambient `any` shims in the - // published package (workspace-package privacy), so type inference - // through SelectionChangeListener collapses to implicit any. - const listener: SelectionChangeListener = (next: SelectionInfo) => { - const nextTarget: TextTarget | null = next.target; - void nextTarget; - }; - const unsubscribe: () => void = api.onChange(listener); - unsubscribe(); - - // The exported SelectionCurrentInput type is the argument shape. + // The Document API contract is request/response: `current()` is + // the read primitive. For change subscriptions, use the + // `superdoc/ui` selector substrate + // (`createSuperDocUI({ superdoc }).select(s => s.selection, ...)`). const input: SelectionCurrentInput = { includeText: true }; api.current(input); } @@ -856,6 +844,7 @@ export { testReplaceFile, testPresentationEditorMethods, testSelectionAPI, + testViewportScrollIntoView, testEditorEvents, testPresentationEditorEvents, testToolbar, From 1fdd03bf0f407e1db0d2f76cc1c92509d925f1e8 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Wed, 29 Apr 2026 07:18:37 -0300 Subject: [PATCH 2/2] fix(superdoc/ui): narrow scroll target via discriminated kind (PR #2988 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `scroll-into-view.ts` checked `kind === 'entity'` through a cast, which left `input.target` widened as `TextAddress | TextTarget | EntityAddress` inside the entity branch — `presentation.navigateTo` expects `NavigableAddress`, so `tsc` flagged TS2345. Switch to a regular discriminated-union check on `target.kind`. All three target shapes carry a `kind` field, so the equality check narrows directly: `EntityAddress` inside the block, `TextAddress | TextTarget` after the early return. Drops the now-unused `TextAddress` / `TextTarget` named imports. --- .../super-editor/src/ui/scroll-into-view.ts | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/packages/super-editor/src/ui/scroll-into-view.ts b/packages/super-editor/src/ui/scroll-into-view.ts index 42168438aa..97650c194d 100644 --- a/packages/super-editor/src/ui/scroll-into-view.ts +++ b/packages/super-editor/src/ui/scroll-into-view.ts @@ -7,7 +7,7 @@ * and `ui.review.scrollTo`. */ -import type { ScrollIntoViewInput, ScrollIntoViewOutput, TextAddress, TextTarget } from '@superdoc/document-api'; +import type { ScrollIntoViewInput, ScrollIntoViewOutput } from '@superdoc/document-api'; import type { Editor } from '../editors/v1/core/Editor.js'; import { resolveTextTarget } from '../editors/v1/document-api-adapters/helpers/adapter-utils.js'; @@ -42,12 +42,18 @@ export async function scrollRangeIntoView(editor: Editor, input: ScrollIntoViewI return { success: false }; } - if ('kind' in input.target && (input.target as { kind?: unknown }).kind === 'entity') { + // Narrow to the entity branch via discriminated-union check on + // `kind`. `TextAddress`, `TextTarget`, and `EntityAddress` all have + // a `kind` field, so the equality check narrows `target` directly + // without a cast — `target` is `EntityAddress` inside the block and + // `TextAddress | TextTarget` after the early return. + const target = input.target; + if (target.kind === 'entity') { if (typeof presentation.navigateTo !== 'function') { return { success: false }; } try { - const ok = await presentation.navigateTo(input.target); + const ok = await presentation.navigateTo(target); return { success: Boolean(ok) }; } catch { return { success: false }; @@ -59,11 +65,11 @@ export async function scrollRangeIntoView(editor: Editor, input: ScrollIntoViewI } try { - const target = input.target as TextAddress | TextTarget; - const firstSegment = - 'segments' in target - ? target.segments[0] - : { blockId: (target as TextAddress).blockId, range: (target as TextAddress).range }; + // After the entity early-return, `target` narrows to + // `TextAddress | TextTarget`. `TextTarget` has `segments`; + // `TextAddress` has `blockId` + `range`. Resolve the first + // segment and hand it to the text resolver. + const firstSegment = 'segments' in target ? target.segments[0] : { blockId: target.blockId, range: target.range }; if (!firstSegment) return { success: false }; const resolved = resolveTextTarget(editor, {