diff --git a/apps/web/core/state/editor/make-block-position.test.ts b/apps/web/core/state/editor/make-block-position.test.ts new file mode 100644 index 0000000000..dd261dbdb8 --- /dev/null +++ b/apps/web/core/state/editor/make-block-position.test.ts @@ -0,0 +1,51 @@ +import { describe, expect, it } from 'vitest'; + +import { makeBlockPosition } from './make-block-position'; + +const blockRelations = [ + { block: { id: 'first' }, position: 'a0' }, + { block: { id: 'middle' }, position: 'a1' }, + { block: { id: 'last' }, position: 'a2' }, +]; + +describe('makeBlockPosition', () => { + it('places an existing last block before the current first block', () => { + const position = makeBlockPosition({ + blockId: 'last', + nextBlockIds: ['last', 'first', 'middle'], + blockRelations, + newBlocks: [], + }); + + expect(position < 'a0').toBe(true); + }); + + it('places an existing first block after the current last block', () => { + const position = makeBlockPosition({ + blockId: 'first', + nextBlockIds: ['middle', 'last', 'first'], + blockRelations, + newBlocks: [], + }); + + expect(position > 'a2').toBe(true); + }); + + it('places multiple adjacent new blocks before the existing first block', () => { + const firstNewPosition = makeBlockPosition({ + blockId: 'new-1', + nextBlockIds: ['new-1', 'new-2', 'first', 'middle', 'last'], + blockRelations, + newBlocks: [], + }); + const secondNewPosition = makeBlockPosition({ + blockId: 'new-2', + nextBlockIds: ['new-1', 'new-2', 'first', 'middle', 'last'], + blockRelations, + newBlocks: [{ toEntity: { id: 'new-1' }, position: firstNewPosition }], + }); + + expect(firstNewPosition < secondNewPosition).toBe(true); + expect(secondNewPosition < 'a0').toBe(true); + }); +}); diff --git a/apps/web/core/state/editor/make-block-position.ts b/apps/web/core/state/editor/make-block-position.ts new file mode 100644 index 0000000000..f8a3c39a03 --- /dev/null +++ b/apps/web/core/state/editor/make-block-position.ts @@ -0,0 +1,54 @@ +import { Position } from '@geoprotocol/geo-sdk/lite'; + +type ExistingBlockRelation = { + position?: string | null; + block: { id: string }; +}; + +type NewBlockRelation = { + position?: string | null; + toEntity: { id: string }; +}; + +/** Generates a block position from its neighbours in the requested document order. */ +export function makeBlockPosition({ + blockId, + nextBlockIds, + blockRelations, + newBlocks, +}: { + blockId: string; + nextBlockIds: string[]; + blockRelations: ExistingBlockRelation[]; + newBlocks: NewBlockRelation[]; +}) { + const position = nextBlockIds.indexOf(blockId); + const beforeBlockId = nextBlockIds[position - 1]; + const afterBlockId = nextBlockIds[position + 1]; + + // Insertions are absent from these collections. Reorders are present with + // their stale position, so exclude the moved block before finding fallback + // neighbours at the start or end of the list. + const allRelations = [ + ...blockRelations.map(relation => ({ + blockId: relation.block.id, + // @TODO(migration): default position + position: relation.position ?? 'a0', + })), + ...newBlocks.map(relation => ({ + blockId: relation.toEntity.id, + // @TODO(migration): default position + position: relation.position ?? 'a0', + })), + ] + .filter(relation => relation.blockId !== blockId) + .sort((a, b) => (a.position < b.position ? -1 : 1)); + + const beforePosition = allRelations.find(relation => relation.blockId === beforeBlockId)?.position; + const beforeRelationIndex = allRelations.findIndex(relation => relation.blockId === beforeBlockId); + const afterPosition = + allRelations.find(relation => relation.blockId === afterBlockId)?.position ?? + (beforeRelationIndex >= 0 ? allRelations[beforeRelationIndex + 1]?.position : allRelations[0]?.position); + + return Position.generateBetween(beforePosition ?? null, afterPosition ?? null); +} diff --git a/apps/web/core/state/editor/use-editor.tsx b/apps/web/core/state/editor/use-editor.tsx index e1e3471e19..c2aead348f 100644 --- a/apps/web/core/state/editor/use-editor.tsx +++ b/apps/web/core/state/editor/use-editor.tsx @@ -35,6 +35,7 @@ import { EntityId } from '../../io/substream-schema'; import { getRelationForBlockType } from './block-types'; import { useActiveTabIdForEditor, useEditorBlocks, useEditorInstance } from './editor-provider'; import { getBlockPositionChanges } from './get-block-position-changes'; +import { makeBlockPosition } from './make-block-position'; import { markdownToEditorJson } from './markdown-adapter'; import { PROFILE_OVERVIEW_TAIL_BLOCK_SENTINEL, @@ -67,40 +68,12 @@ function makeNewBlockRelation({ }: MakeNewBlockArgs) { const newRelationId = ID.createEntityId(); - const position = nextBlockIds.indexOf(addedBlock.id); - - // @TODO: noUncheckedIndexAccess - const beforeBlockIndex = nextBlockIds[position - 1] as string | undefined; - const afterBlockIndex = nextBlockIds[position + 1] as string | undefined; - - // Create a unified array with consistent structure for both blockRelations and newBlocks - const allRelations = [ - ...blockRelations.map(r => ({ - toEntity: { id: r.block.id }, - // @TODO(migration): default position - position: r.position ?? 'a0', - })), - ...newBlocks.map(b => ({ - toEntity: { id: b.toEntity.id }, - // @TODO(migration): default position - position: b.position ?? 'a0', - })), - ].sort((a, b) => (a.position < b.position ? -1 : 1)); - - // Check both the existing blocks and any that are created as part of this update - // tick. This is necessary as right now we don't update the Geo state until the - // user blurs the editor. See the comment earlier in this function. - const beforeCollectionItemIndex = allRelations.find(c => c.toEntity.id === beforeBlockIndex)?.position; - - // When the afterCollectionItemIndex is undefined, we need to use the next block of beforeBlockIndex - const afterCollectionItemIndex = - allRelations.find(c => c.toEntity.id === afterBlockIndex)?.position ?? - allRelations[allRelations.findIndex(c => c.position === beforeCollectionItemIndex) + 1]?.position; - - const newBlockOrdering = Position.generateBetween( - beforeCollectionItemIndex ?? null, - afterCollectionItemIndex ?? null - ); + const newBlockOrdering = makeBlockPosition({ + blockId: addedBlock.id, + nextBlockIds, + blockRelations, + newBlocks, + }); const renderableType = ((): RenderableEntityType => { switch (tiptapBlock.type) { @@ -124,8 +97,8 @@ function makeNewBlockRelation({ } })(); - const newRelation: Relation = { - spaceId: spaceId, + return { + spaceId, id: newRelationId, position: newBlockOrdering, verified: false, @@ -144,9 +117,7 @@ function makeNewBlockRelation({ id: entityPageId, name: null, }, - }; - - return newRelation; + } satisfies Relation; } interface UpsertBlocksRelationsArgs { @@ -161,7 +132,7 @@ interface UpsertBlocksRelationsArgs { // Helper function to create or update the block IDs on an entity // Since we don't currently support array value types, we store all ordered blocks as a single stringified array -const makeBlocksRelations = async ({ +const makeBlocksRelations = ({ nextBlocks, blockRelations, spaceId, @@ -207,22 +178,18 @@ const makeBlocksRelations = async ({ for (const movedBlock of movedBlocks) { const relationForMovedBlock = blockRelations.find(r => r.block.id === movedBlock.id); + if (!relationForMovedBlock) continue; - if (relationForMovedBlock) { - storage.relations.delete(relationForMovedBlock); - } - - const newRelation = makeNewBlockRelation({ - tiptapBlock: nextBlocks.find(b => b.id === movedBlock.id)!, - addedBlock: movedBlock, + const position = makeBlockPosition({ + blockId: movedBlock.id, nextBlockIds, blockRelations, - spaceId, newBlocks, - entityPageId, }); - storage.relations.set(newRelation); + storage.relations.update(relationForMovedBlock, draft => { + draft.position = position; + }); } }; diff --git a/apps/web/core/sync/relation-update.test.ts b/apps/web/core/sync/relation-update.test.ts new file mode 100644 index 0000000000..74772f1d55 --- /dev/null +++ b/apps/web/core/sync/relation-update.test.ts @@ -0,0 +1,96 @@ +import { describe, expect, it } from 'vitest'; + +import { Relation } from '../types'; +import { + getRelationUpdateUnsetFields, + isExistingRelationWithUnchangedIdentity, + requiresRelationIdentityReplacement, +} from './relation-update'; + +const existingRelation: Relation = { + id: 'existing-relation', + entityId: 'relation-entity', + type: { id: 'blocks', name: 'Blocks' }, + fromEntity: { id: 'page', name: 'Page' }, + toEntity: { id: 'block', name: 'Block', value: 'block' }, + renderableType: 'TEXT', + position: 'a0', + spaceId: 'space', +}; + +describe('isExistingRelationWithUnchangedIdentity', () => { + it('preserves an existing relation when its identity fields are unchanged', () => { + expect(isExistingRelationWithUnchangedIdentity(existingRelation, { ...existingRelation, position: 'a1' })).toBe( + true + ); + }); + + it('does not imply that non-identity fields are publishable', () => { + expect(isExistingRelationWithUnchangedIdentity(existingRelation, { ...existingRelation, verified: true })).toBe( + true + ); + }); + + it('keeps an unpublished local relation as a create', () => { + const localRelation = { ...existingRelation, isLocal: true, hasBeenPublished: false }; + + expect(isExistingRelationWithUnchangedIdentity(localRelation, { ...localRelation, position: 'a1' })).toBe(false); + }); + + it('does not use updateRelation for endpoint changes the SDK cannot update', () => { + expect( + isExistingRelationWithUnchangedIdentity(existingRelation, { + ...existingRelation, + toEntity: { id: 'different-block', name: 'Different block', value: 'different-block' }, + }) + ).toBe(false); + }); +}); + +describe('requiresRelationIdentityReplacement', () => { + it('replaces an existing relation when an endpoint changes', () => { + expect( + requiresRelationIdentityReplacement(existingRelation, { + ...existingRelation, + fromEntity: { id: 'different-page', name: 'Different page' }, + }) + ).toBe(true); + }); + + it('allows an unpublished relation identity to change before its first create', () => { + const localRelation = { ...existingRelation, isLocal: true, hasBeenPublished: false }; + + expect( + requiresRelationIdentityReplacement(localRelation, { + ...localRelation, + fromEntity: { id: 'different-page', name: 'Different page' }, + }) + ).toBe(false); + }); + + it('keeps position-only changes on the existing relation', () => { + expect(requiresRelationIdentityReplacement(existingRelation, { ...existingRelation, position: 'a1' })).toBe( + false + ); + }); +}); + +describe('getRelationUpdateUnsetFields', () => { + it('explicitly unsets a removed to-space reference', () => { + const relationWithSpace = { ...existingRelation, toSpaceId: 'target-space' }; + + expect(getRelationUpdateUnsetFields(relationWithSpace, { ...relationWithSpace, toSpaceId: undefined })).toEqual([ + 'toSpace', + ]); + }); + + it('preserves a pending unset until a to-space reference is selected again', () => { + const pendingUnset = { + ...existingRelation, + relationUpdateUnsetFields: ['toSpace'] as Array<'toSpace'>, + }; + + expect(getRelationUpdateUnsetFields(pendingUnset, pendingUnset)).toEqual(['toSpace']); + expect(getRelationUpdateUnsetFields(pendingUnset, { ...pendingUnset, toSpaceId: 'target-space' })).toEqual([]); + }); +}); diff --git a/apps/web/core/sync/relation-update.ts b/apps/web/core/sync/relation-update.ts new file mode 100644 index 0000000000..b6a19773e6 --- /dev/null +++ b/apps/web/core/sync/relation-update.ts @@ -0,0 +1,38 @@ +import { Relation } from '../types'; + +/** Whether a relation already has a remote identity that must not be reused. */ +export function isExistingRelation(relation: Relation) { + return relation.isLocal !== true || relation.hasBeenPublished === true || relation.isRelationUpdate === true; +} + +/** Whether a relation change requires deleting the old edge and creating a new one. */ +export function requiresRelationIdentityReplacement(base: Relation, changed: Relation) { + if (!isExistingRelation(base)) return false; + + return ( + base.id !== changed.id || + base.entityId !== changed.entityId || + base.type.id !== changed.type.id || + base.fromEntity.id !== changed.fromEntity.id || + base.toEntity.id !== changed.toEntity.id || + base.spaceId !== changed.spaceId + ); +} + +/** Whether a remote relation can retain its identity after a local change. */ +export function isExistingRelationWithUnchangedIdentity(base: Relation, changed: Relation) { + return isExistingRelation(base) && !requiresRelationIdentityReplacement(base, changed); +} + +/** Tracks optional relation fields that must be explicitly cleared remotely. */ +export function getRelationUpdateUnsetFields(base: Relation, changed: Relation) { + const unsetFields = new Set(base.relationUpdateUnsetFields ?? []); + + if (changed.toSpaceId) { + unsetFields.delete('toSpace'); + } else if (base.toSpaceId) { + unsetFields.add('toSpace'); + } + + return Array.from(unsetFields); +} diff --git a/apps/web/core/sync/use-mutate.tsx b/apps/web/core/sync/use-mutate.tsx index e7fd2dad06..203198ef43 100644 --- a/apps/web/core/sync/use-mutate.tsx +++ b/apps/web/core/sync/use-mutate.tsx @@ -18,6 +18,11 @@ import { DataType, Relation, Value } from '../types'; import { toHexId } from '../utils/hex-id'; import { extractValueString } from '../utils/value'; import { saveVideoKeyframe } from '../utils/video/save-keyframe'; +import { + getRelationUpdateUnsetFields, + isExistingRelationWithUnchangedIdentity, + requiresRelationIdentityReplacement, +} from './relation-update'; import { GeoStore } from './store'; import { store, useSyncEngine } from './use-sync-engine'; @@ -482,7 +487,32 @@ function createMutator(store: GeoStore): Mutator { store.setRelation(newRelation); }, update: (base, recipe) => { - const newRelation = produce(base, recipe); + const changedRelation = produce(base, recipe); + + // The SDK cannot update relation endpoints or other identity fields. + // Replace an existing edge with a fresh relation ID so the backend does + // not ignore a createRelation operation that reuses the committed ID. + if (requiresRelationIdentityReplacement(base, changedRelation)) { + store.deleteRelation(base); + store.setRelation({ + ...changedRelation, + id: ID.createEntityId(), + isRelationUpdate: undefined, + relationUpdateUnsetFields: undefined, + }); + return; + } + + // Relations created in the current local edit still need createRelation. + // Once a relation exists remotely and its identity is unchanged, retain + // that identity. The publish layer serializes its SDK-updateable fields. + const newRelation = produce(changedRelation, draft => { + const isRelationUpdate = isExistingRelationWithUnchangedIdentity(base, changedRelation); + draft.isRelationUpdate = isRelationUpdate; + draft.relationUpdateUnsetFields = isRelationUpdate + ? getRelationUpdateUnsetFields(base, changedRelation) + : undefined; + }); store.setRelation(newRelation); }, delete: newRelation => { diff --git a/apps/web/core/types.ts b/apps/web/core/types.ts index 5c6bb1c39c..a3a7ff66f4 100644 --- a/apps/web/core/types.ts +++ b/apps/web/core/types.ts @@ -247,6 +247,10 @@ export type Value = LocalMetadata & { // ============================================================================== export type Relation = LocalMetadata & { + /** Publish this local change with the SDK's updateRelation operation. */ + isRelationUpdate?: boolean; + /** Optional relation fields that the pending updateRelation operation clears. */ + relationUpdateUnsetFields?: Array<'toSpace'>; id: string; entityId: string; type: { diff --git a/apps/web/core/utils/publish/publish.test.ts b/apps/web/core/utils/publish/publish.test.ts index 3c8a10cecf..b1f467fcf7 100644 --- a/apps/web/core/utils/publish/publish.test.ts +++ b/apps/web/core/utils/publish/publish.test.ts @@ -90,6 +90,13 @@ type DeleteRelationOp = Op & { id: unknown; }; +type UpdateRelationOp = Op & { + type: 'updateRelation'; + id: unknown; + position?: string; + unset: string[]; +}; + describe('prepareLocalDataForPublishing', () => { describe('basic functionality', () => { it('should create updateEntity operation for valid values', () => { @@ -123,6 +130,48 @@ describe('prepareLocalDataForPublishing', () => { expect(createOp.position).toBe('1'); }); + it('should update an existing relation position without deleting or recreating it', () => { + const relationId = IdUtils.generate(); + const relations = [ + createMockRelation({ + id: relationId, + isRelationUpdate: true, + position: 'position-after-reorder', + }), + ]; + + const result = prepareLocalDataForPublishing([], relations, 'test-space'); + + expect(result).toHaveLength(1); + expect(result.filter(op => op.type === 'deleteRelation')).toHaveLength(0); + expect(result.filter(op => op.type === 'createRelation')).toHaveLength(0); + + const updateOp = result[0] as UpdateRelationOp; + expect(updateOp.type).toBe('updateRelation'); + expect(updateOp.position).toBe('position-after-reorder'); + expect(Array.from(updateOp.id as Uint8Array)).toEqual(Array.from(IdUtils.toBytes(relationId) as Uint8Array)); + }); + + it('should explicitly unset a cleared relation to-space reference', () => { + const relationId = IdUtils.generate(); + const relations = [ + createMockRelation({ + id: relationId, + isRelationUpdate: true, + toSpaceId: undefined, + relationUpdateUnsetFields: ['toSpace'], + }), + ]; + + const result = prepareLocalDataForPublishing([], relations, 'test-space'); + + expect(result).toHaveLength(1); + const updateOp = result[0] as UpdateRelationOp; + expect(updateOp.type).toBe('updateRelation'); + expect(updateOp.unset).toEqual(['toSpace']); + expect(updateOp).not.toHaveProperty('toSpace'); + }); + it('should create deleteRelation operation for deleted relations', async () => { const values: Value[] = []; const relations = [createMockRelation({ isDeleted: true, type: { id: SystemIds.BLOCKS, name: 'Blocks' } })]; diff --git a/apps/web/core/utils/publish/publish.ts b/apps/web/core/utils/publish/publish.ts index b80decd4e6..8ed84876d2 100644 --- a/apps/web/core/utils/publish/publish.ts +++ b/apps/web/core/utils/publish/publish.ts @@ -2,10 +2,13 @@ import { ContentIds, type DecimalMantissa, Graph, + IdUtils, Op, + Ops, type PropertyValueParam, SystemIds, } from '@geoprotocol/geo-sdk/lite'; +import { updateRelation as updateGrc20Relation } from '@geoprotocol/grc-20'; import { Effect } from 'effect'; @@ -93,6 +96,24 @@ function prepareOps(values: Value[], relations: Relation[], spaceId: string): Op if (r.isDeleted) { const { ops: deleteOps } = Graph.deleteRelation({ id: r.id }); ops.push(...deleteOps); + } else if (r.isRelationUpdate) { + if (r.relationUpdateUnsetFields?.length) { + ops.push( + updateGrc20Relation({ + id: IdUtils.toGrcId(r.id), + position: r.position, + ...(r.toSpaceId && { toSpace: IdUtils.toGrcId(r.toSpaceId) }), + unset: r.relationUpdateUnsetFields, + }) + ); + } else { + const { ops: updateOps } = Ops.relations.update({ + id: r.id, + position: r.position, + ...(r.toSpaceId && { toSpace: r.toSpaceId }), + }); + ops.push(...updateOps); + } } else { const { ops: createOps } = Graph.createRelation({ fromEntity: r.fromEntity.id, diff --git a/apps/web/partials/editor/block-reorder.test.ts b/apps/web/partials/editor/block-reorder.test.ts new file mode 100644 index 0000000000..764bad74e3 --- /dev/null +++ b/apps/web/partials/editor/block-reorder.test.ts @@ -0,0 +1,307 @@ +import { DndContext } from '@dnd-kit/core'; +import '@testing-library/jest-dom/vitest'; +import { cleanup, fireEvent, render, screen } from '@testing-library/react'; +import Document from '@tiptap/extension-document'; +import Paragraph from '@tiptap/extension-paragraph'; +import Text from '@tiptap/extension-text'; +import { Editor } from '@tiptap/react'; + +import React from 'react'; + +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import { + BlockDragHandle, + BlockGutterHoverArea, + getGutterHoveredChildIndex, + getNextKeyboardDropBoundary, + getTopLevelBlockElements, + isBlockDropNoOp, + makeDropZones, + moveTopLevelBlock, + releasePointerDragFocus, +} from './block-reorder'; + +const editors: Editor[] = []; + +afterEach(() => { + cleanup(); + for (const editor of editors.splice(0)) editor.destroy(); + vi.unstubAllGlobals(); +}); + +describe('BlockDragHandle', () => { + it('bridges the gap between the visible handle and the hovered block', () => { + render( + React.createElement( + DndContext, + null, + React.createElement(BlockDragHandle, { + childIndex: 0, + top: 12, + left: -32, + isDragging: false, + visible: true, + }) + ) + ); + + const button = screen.getByRole('button', { name: 'Drag block 1 to reorder' }); + const hoverBridge = button.parentElement; + + expect(button).toHaveClass('size-6'); + expect(hoverBridge).toHaveAttribute('data-block-drag-handle'); + expect(hoverBridge).toHaveClass('w-8'); + }); + + it('reveals a hidden handle when it receives keyboard focus', () => { + render( + React.createElement( + DndContext, + null, + React.createElement(BlockDragHandle, { + childIndex: 0, + top: 12, + left: -32, + isDragging: false, + visible: false, + }) + ) + ); + + const button = screen.getByRole('button', { name: 'Drag block 1 to reorder' }); + const handle = button.parentElement; + expect(handle).toHaveStyle({ opacity: '0', pointerEvents: 'none' }); + + fireEvent.focus(button); + + expect(handle).toHaveStyle({ opacity: '1', pointerEvents: 'auto' }); + }); + + it('keeps hidden handles available on coarse or hoverless pointers', () => { + vi.stubGlobal( + 'matchMedia', + vi.fn().mockReturnValue({ + matches: true, + media: '(hover: none), (pointer: coarse)', + onchange: null, + addListener: vi.fn(), + removeListener: vi.fn(), + addEventListener: vi.fn(), + removeEventListener: vi.fn(), + dispatchEvent: vi.fn(), + }) + ); + + render( + React.createElement( + DndContext, + null, + React.createElement(BlockDragHandle, { + childIndex: 0, + top: 12, + left: -32, + isDragging: false, + visible: false, + }) + ) + ); + + const button = screen.getByRole('button', { name: 'Drag block 1 to reorder' }); + expect(button.parentElement).toHaveStyle({ opacity: '1', pointerEvents: 'auto' }); + expect(button).toHaveClass('touch-none'); + }); + + it('releases pointer focus after a drag so the old block slot does not stay highlighted', () => { + render( + React.createElement( + DndContext, + null, + React.createElement(BlockDragHandle, { + childIndex: 0, + top: 12, + left: -32, + isDragging: false, + visible: true, + }) + ) + ); + + const button = screen.getByRole('button', { name: 'Drag block 1 to reorder' }); + button.focus(); + const pointerDown = new MouseEvent('pointerdown', { bubbles: true }); + button.dispatchEvent(pointerDown); + expect(button).toHaveFocus(); + + releasePointerDragFocus(pointerDown); + + expect(button).not.toHaveFocus(); + }); +}); + +describe('BlockGutterHoverArea', () => { + it('creates a real pointer target extending left beyond the handle position', () => { + const { container } = render( + React.createElement(BlockGutterHoverArea, { + editorLeft: 100, + blocks: [ + { childIndex: 0, top: 10, bottom: 30, center: 20 }, + { childIndex: 1, top: 50, bottom: 70, center: 60 }, + ], + }) + ); + + expect(container.querySelector('[data-block-drag-gutter]')).toHaveStyle({ + top: '10px', + left: '52px', + width: '48px', + height: '60px', + }); + }); +}); + +describe('moveTopLevelBlock', () => { + it('moves a block down to the selected document boundary', () => { + const editor = makeEditor(['A', 'B', 'C', 'D']); + + expect(moveTopLevelBlock(editor, 0, 3)).toBe(true); + + expect(blockText(editor)).toEqual(['B', 'C', 'A', 'D']); + }); + + it('moves a block up to the selected document boundary', () => { + const editor = makeEditor(['A', 'B', 'C', 'D']); + + expect(moveTopLevelBlock(editor, 3, 1)).toBe(true); + + expect(blockText(editor)).toEqual(['A', 'D', 'B', 'C']); + }); + + it('does not change the document when dropped beside its current position', () => { + const editor = makeEditor(['A', 'B', 'C']); + + expect(moveTopLevelBlock(editor, 1, 1)).toBe(false); + expect(moveTopLevelBlock(editor, 1, 2)).toBe(false); + expect(blockText(editor)).toEqual(['A', 'B', 'C']); + }); +}); + +describe('getGutterHoveredChildIndex', () => { + const blocks = [ + { childIndex: 0, top: 10, bottom: 30, center: 20 }, + { childIndex: 1, top: 50, bottom: 70, center: 60 }, + ]; + + it('shows the handle when hovering directly left of a block', () => { + expect(getGutterHoveredChildIndex(blocks, 60, 20, 100)).toBe(0); + expect(getGutterHoveredChildIndex(blocks, 60, 60, 100)).toBe(1); + }); + + it('keeps the gutter target continuous through the gap between blocks', () => { + expect(getGutterHoveredChildIndex(blocks, 60, 39, 100)).toBe(0); + expect(getGutterHoveredChildIndex(blocks, 60, 41, 100)).toBe(1); + }); + + it('ignores pointers outside the left gutter', () => { + expect(getGutterHoveredChildIndex(blocks, 51, 20, 100)).toBeNull(); + expect(getGutterHoveredChildIndex(blocks, 101, 20, 100)).toBeNull(); + }); +}); + +describe('getNextKeyboardDropBoundary', () => { + const boundaries = [0, 1, 2, 3, 4]; + + it('moves one block at a time and can return to the original position', () => { + expect(getNextKeyboardDropBoundary(1, null, 1, boundaries)).toBe(3); + expect(getNextKeyboardDropBoundary(1, 3, -1, boundaries)).toBe(1); + expect(getNextKeyboardDropBoundary(1, 1, -1, boundaries)).toBe(0); + }); + + it('stops at the first and last positions', () => { + expect(getNextKeyboardDropBoundary(0, null, -1, boundaries)).toBeNull(); + expect(getNextKeyboardDropBoundary(3, null, 1, boundaries)).toBeNull(); + }); + + it('moves by draggable rank when excluded nodes make child indexes non-contiguous', () => { + const boundariesWithExcludedNode = [0, 2, 3]; + + expect(getNextKeyboardDropBoundary(0, null, 1, boundariesWithExcludedNode)).toBe(3); + expect(getNextKeyboardDropBoundary(2, null, -1, boundariesWithExcludedNode)).toBe(0); + }); +}); + +describe('isBlockDropNoOp', () => { + it('recognizes both slots beside the source in a contiguous document', () => { + const boundaries = [0, 1, 2, 3]; + + expect(isBlockDropNoOp(1, 1, boundaries)).toBe(true); + expect(isBlockDropNoOp(1, 2, boundaries)).toBe(true); + expect(isBlockDropNoOp(1, 0, boundaries)).toBe(false); + expect(isBlockDropNoOp(1, 3, boundaries)).toBe(false); + }); + + it('uses draggable rank when excluded nodes make child indexes sparse', () => { + const boundariesWithExcludedNode = [0, 2, 3]; + + expect(isBlockDropNoOp(0, 2, boundariesWithExcludedNode)).toBe(true); + expect(isBlockDropNoOp(2, 2, boundariesWithExcludedNode)).toBe(true); + expect(isBlockDropNoOp(0, 3, boundariesWithExcludedNode)).toBe(false); + expect(isBlockDropNoOp(2, 0, boundariesWithExcludedNode)).toBe(false); + }); +}); + +describe('makeDropZones', () => { + it('creates a drop target before, between, and after every draggable block', () => { + expect( + makeDropZones([ + { childIndex: 0, top: 10, bottom: 30, center: 20 }, + { childIndex: 1, top: 40, bottom: 80, center: 60 }, + ]) + ).toEqual([ + { boundary: 0, top: 10, height: 10, indicatorTop: 10 }, + { boundary: 1, top: 20, height: 40, indicatorTop: 35 }, + { boundary: 2, top: 60, height: 20, indicatorTop: 80 }, + ]); + }); + + it('places the final drop boundary immediately after the last draggable block', () => { + const zones = makeDropZones([ + { childIndex: 0, top: 10, bottom: 30, center: 20 }, + { childIndex: 1, top: 50, bottom: 70, center: 60 }, + ]); + + expect(zones.map(zone => zone.boundary)).toEqual([0, 1, 2]); + }); +}); + +describe('getTopLevelBlockElements', () => { + it('maps document indexes without counting direct gap-cursor widgets', () => { + const editor = makeEditor(['A', 'B']); + const editorElement = editor.view.dom; + const firstBlock = editor.view.nodeDOM(0); + const gapCursor = document.createElement('div'); + gapCursor.className = 'ProseMirror-gapcursor'; + + expect(firstBlock).toBeInstanceOf(HTMLElement); + firstBlock?.parentNode?.insertBefore(gapCursor, firstBlock.nextSibling); + + expect(getTopLevelBlockElements(editor, editorElement).map(block => block.childIndex)).toEqual([0, 1]); + expect(getTopLevelBlockElements(editor, editorElement).map(block => block.element)).not.toContain(gapCursor); + }); +}); + +function makeEditor(labels: string[]) { + const editor = new Editor({ + extensions: [Document, Paragraph, Text], + content: { + type: 'doc', + content: labels.map(label => ({ type: 'paragraph', content: [{ type: 'text', text: label }] })), + }, + }); + editors.push(editor); + return editor; +} + +function blockText(editor: Editor) { + return Array.from({ length: editor.state.doc.childCount }, (_, index) => editor.state.doc.child(index).textContent); +} diff --git a/apps/web/partials/editor/block-reorder.tsx b/apps/web/partials/editor/block-reorder.tsx new file mode 100644 index 0000000000..98a7c6fc26 --- /dev/null +++ b/apps/web/partials/editor/block-reorder.tsx @@ -0,0 +1,571 @@ +'use client'; + +import { + DndContext, + DragCancelEvent, + DragEndEvent, + DragOverEvent, + DragOverlay, + DragStartEvent, + KeyboardCode, + KeyboardSensor, + PointerSensor, + closestCenter, + useDraggable, + useDroppable, + useSensor, + useSensors, +} from '@dnd-kit/core'; +import type { KeyboardCoordinateGetter } from '@dnd-kit/core'; +import type { Editor } from '@tiptap/react'; + +import * as React from 'react'; + +import { OrderDots } from '~/design-system/icons/order-dots'; + +import { ensureUniqueNodeIds } from './id-extension'; + +type BlockLayout = { + childIndex: number; + element?: HTMLElement; + top: number; + bottom: number; + center: number; +}; + +type DropZoneLayout = { + boundary: number; + top: number; + height: number; + indicatorTop: number; +}; + +const GUTTER_HOVER_WIDTH = 48; + +type Props = { + children: React.ReactNode; + editor: Editor; + editorWrapperRef: React.RefObject; + enabled: boolean; + onReorder: () => void; +}; + +/** Adds edit-mode drag controls around TipTap's top-level content blocks. */ +export function BlockReorder({ children, editor, editorWrapperRef, enabled, onReorder }: Props) { + const sensors = useSensors( + useSensor(PointerSensor, { + activationConstraint: { distance: 4 }, + }), + useSensor(KeyboardSensor, { + coordinateGetter: blockKeyboardCoordinates, + }) + ); + const [blockLayout, setBlockLayout] = React.useState([]); + const blockLayoutRef = React.useRef([]); + const [hoveredChildIndex, setHoveredChildIndex] = React.useState(null); + const hoveredChildIndexRef = React.useRef(null); + const [activeChildIndex, setActiveChildIndex] = React.useState(null); + const [activeBoundary, setActiveBoundary] = React.useState(null); + + const updateHoveredChildIndex = React.useCallback((childIndex: number | null) => { + hoveredChildIndexRef.current = childIndex; + setHoveredChildIndex(childIndex); + }, []); + + const measureBlocks = React.useCallback(() => { + const wrapper = editorWrapperRef.current; + const editorElement = wrapper?.querySelector('.ProseMirror'); + if (!wrapper || !editorElement) return; + + const wrapperRect = wrapper.getBoundingClientRect(); + const nextLayout = getTopLevelBlockElements(editor, editorElement).flatMap(({ childIndex, element }) => { + if (!isDraggableBlock(element)) return []; + + const rect = element.getBoundingClientRect(); + const top = rect.top - wrapperRect.top; + const bottom = rect.bottom - wrapperRect.top; + + return [{ childIndex, element, top, bottom, center: top + rect.height / 2 }]; + }); + + blockLayoutRef.current = nextLayout; + setBlockLayout(nextLayout); + }, [editor, editorWrapperRef]); + + React.useLayoutEffect(() => { + if (!enabled) return; + + const wrapper = editorWrapperRef.current; + const editorElement = wrapper?.querySelector('.ProseMirror'); + if (!wrapper || !editorElement) return; + + // Establish stable handle identities before rendering the controls. This + // lets focus follow a block when keyboard reordering changes its child index. + ensureUniqueNodeIds(editor); + measureBlocks(); + + const resizeObserver = new ResizeObserver(measureBlocks); + const mutationObserver = new MutationObserver(measureBlocks); + let measureFrame: number | null = null; + const scheduleMeasureBlocks = () => { + // The drop transaction already triggers the existing active-drag effect + // cycle. Only schedule document edits made outside an active drag. + if (activeChildIndex !== null || measureFrame !== null) return; + + measureFrame = requestAnimationFrame(() => { + measureFrame = null; + measureBlocks(); + }); + }; + resizeObserver.observe(editorElement); + mutationObserver.observe(editorElement, { childList: true }); + editor.on('update', scheduleMeasureBlocks); + + const handlePointerMove = (event: PointerEvent) => { + if (activeChildIndex !== null) return; + + const target = event.target; + if (!(target instanceof Element)) return; + if (target.closest('[data-block-drag-handle]')) return; + + const blockElement = target.closest('.ProseMirror > *'); + const hoveredContentIndex = blockLayoutRef.current.find(block => block.element === blockElement)?.childIndex; + const wrapperRect = wrapper.getBoundingClientRect(); + const editorRect = editorElement.getBoundingClientRect(); + const hoveredGutterIndex = getGutterHoveredChildIndex( + blockLayoutRef.current, + event.clientX - wrapperRect.left, + event.clientY - wrapperRect.top, + editorRect.left - wrapperRect.left + ); + const childIndex = hoveredContentIndex ?? hoveredGutterIndex ?? null; + + if (childIndex === null) { + updateHoveredChildIndex(null); + return; + } + + if (hoveredChildIndexRef.current === childIndex) return; + + measureBlocks(); + updateHoveredChildIndex(childIndex); + }; + + const handlePointerLeave = () => { + if (activeChildIndex === null) updateHoveredChildIndex(null); + }; + + wrapper.addEventListener('pointermove', handlePointerMove); + wrapper.addEventListener('pointerleave', handlePointerLeave); + + return () => { + resizeObserver.disconnect(); + mutationObserver.disconnect(); + editor.off('update', scheduleMeasureBlocks); + if (measureFrame !== null) cancelAnimationFrame(measureFrame); + wrapper.removeEventListener('pointermove', handlePointerMove); + wrapper.removeEventListener('pointerleave', handlePointerLeave); + }; + }, [activeChildIndex, editor, editorWrapperRef, enabled, measureBlocks, updateHoveredChildIndex]); + + React.useEffect(() => { + if (enabled) return; + + hoveredChildIndexRef.current = null; + setHoveredChildIndex(null); + setActiveChildIndex(null); + setActiveBoundary(null); + }, [enabled]); + + const editorRect = editorWrapperRef.current?.querySelector('.ProseMirror')?.getBoundingClientRect(); + const wrapperRect = editorWrapperRef.current?.getBoundingClientRect(); + const editorLeft = editorRect && wrapperRect ? editorRect.left - wrapperRect.left : 0; + const editorWidth = editorRect?.width ?? 0; + const visibleHandleIndex = activeChildIndex ?? hoveredChildIndex; + const handleLayout = blockLayout.find(block => block.childIndex === visibleHandleIndex); + const dropZones = makeDropZones(blockLayout); + const indicatorTop = dropZones.find(zone => zone.boundary === activeBoundary)?.indicatorTop; + + const resetDragState = () => { + setActiveChildIndex(null); + setActiveBoundary(null); + updateHoveredChildIndex(null); + }; + + const handleDragCancel = (event: DragCancelEvent) => { + releasePointerDragFocus(event.activatorEvent); + resetDragState(); + }; + + const handleDragStart = (event: DragStartEvent) => { + if (!enabled) return; + + const childIndex = event.active.data.current?.childIndex; + if (typeof childIndex !== 'number') return; + + // Continuation nodes loaded from markdown intentionally start without IDs. + // A drag can happen before blur, so assign/dedupe IDs before persisting it. + ensureUniqueNodeIds(editor); + measureBlocks(); + setActiveChildIndex(childIndex); + }; + + const handleDragOver = (event: DragOverEvent) => { + const boundary = event.over?.data.current?.boundary; + setActiveBoundary(typeof boundary === 'number' ? boundary : null); + }; + + const handleDragEnd = (event: DragEndEvent) => { + const sourceIndex = event.active.data.current?.childIndex; + const dropBoundary = event.over?.data.current?.boundary; + const boundaries = makeDropZones(blockLayoutRef.current).map(zone => zone.boundary); + + if ( + typeof sourceIndex === 'number' && + typeof dropBoundary === 'number' && + !isBlockDropNoOp(sourceIndex, dropBoundary, boundaries) && + moveTopLevelBlock(editor, sourceIndex, dropBoundary) + ) { + onReorder(); + } + + releasePointerDragFocus(event.activatorEvent); + resetDragState(); + }; + + return ( + + {children} + + {enabled && activeChildIndex === null && blockLayout.length > 0 ? ( + editor.commands.focus()} /> + ) : null} + + {enabled + ? blockLayout.map(layout => ( + + )) + : null} + + {enabled && activeChildIndex !== null + ? dropZones.map(zone => ) + : null} + + {enabled && activeChildIndex !== null && indicatorTop !== undefined ? ( +
+ ) : null} + + + {enabled && activeChildIndex !== null ? ( +
+ +
+ ) : null} + + + ); +} + +function getBlockDragHandleKey(editor: Editor, childIndex: number) { + if (childIndex < 0 || childIndex >= editor.state.doc.childCount) return `child-${childIndex}`; + + const blockId = editor.state.doc.child(childIndex).attrs.id; + return typeof blockId === 'string' && blockId.length > 0 ? blockId : `child-${childIndex}`; +} + +/** Pointer activation should not leave a handle visibly focused after drop. */ +export function releasePointerDragFocus(activatorEvent: Event) { + if (activatorEvent.type === 'keydown') return; + + const target = activatorEvent.target; + if (!(target instanceof Element)) return; + + target.closest('[data-block-drag-handle] button')?.blur(); +} + +export function BlockGutterHoverArea({ + blocks, + editorLeft, + onClick, +}: { + blocks: BlockLayout[]; + editorLeft: number; + onClick?: React.MouseEventHandler; +}) { + const firstBlock = blocks[0]; + const lastBlock = blocks[blocks.length - 1]; + if (!firstBlock || !lastBlock) return null; + + return ( +
+ ); +} + +export function BlockDragHandle({ + childIndex, + top, + left, + isDragging, + visible, +}: { + childIndex: number; + top: number; + left: number; + isDragging: boolean; + visible: boolean; +}) { + const [isFocused, setIsFocused] = React.useState(false); + const [isCoarseOrHoverlessPointer, setIsCoarseOrHoverlessPointer] = React.useState(false); + const { attributes, listeners, setNodeRef } = useDraggable({ + id: `content-block-${childIndex}`, + data: { childIndex }, + }); + const isAvailable = !isDragging && (visible || isFocused || isCoarseOrHoverlessPointer); + + React.useEffect(() => { + if (typeof window.matchMedia !== 'function') return; + + const pointerQuery = window.matchMedia('(hover: none), (pointer: coarse)'); + const updatePointerMode = () => setIsCoarseOrHoverlessPointer(pointerQuery.matches); + updatePointerMode(); + pointerQuery.addEventListener('change', updatePointerMode); + + return () => pointerQuery.removeEventListener('change', updatePointerMode); + }, []); + + return ( +
+ +
+ ); +} + +export const blockKeyboardCoordinates: KeyboardCoordinateGetter = (event, { context, currentCoordinates }) => { + if (event.code !== KeyboardCode.Up && event.code !== KeyboardCode.Down) return; + + const sourceIndex = context.active?.data.current?.childIndex; + if (typeof sourceIndex !== 'number') return; + + const dropZones = context.droppableContainers + .getEnabled() + .flatMap(container => { + const boundary = container.data.current?.boundary; + return typeof boundary === 'number' ? [{ boundary, container }] : []; + }) + .sort((a, b) => a.boundary - b.boundary); + const currentBoundary = context.over?.data.current?.boundary; + const targetBoundary = getNextKeyboardDropBoundary( + sourceIndex, + typeof currentBoundary === 'number' ? currentBoundary : null, + event.code === KeyboardCode.Down ? 1 : -1, + dropZones.map(zone => zone.boundary) + ); + const target = dropZones.find(zone => zone.boundary === targetBoundary); + const targetRect = target ? context.droppableRects.get(target.container.id) : null; + if (!targetRect) return; + + event.preventDefault(); + const collisionHeight = context.collisionRect?.height ?? 0; + + return { + x: currentCoordinates.x, + y: targetRect.top + (targetRect.height - collisionHeight) / 2, + }; +}; + +export function getNextKeyboardDropBoundary( + sourceIndex: number, + currentBoundary: number | null, + direction: -1 | 1, + boundaries: number[] +) { + const sourceRank = boundaries.indexOf(sourceIndex); + if (sourceRank === -1) return null; + + const currentRank = + currentBoundary === null ? sourceRank : getBlockRankAtBoundary(sourceIndex, currentBoundary, boundaries); + if (currentRank === null) return null; + + const targetRank = currentRank + direction; + const targetBoundaryRank = targetRank > sourceRank ? targetRank + 1 : targetRank; + + return boundaries[targetBoundaryRank] ?? null; +} + +/** Whether a drop boundary leaves the source in the same draggable-block slot. */ +export function isBlockDropNoOp(sourceIndex: number, dropBoundary: number, boundaries: number[]) { + const sourceRank = boundaries.indexOf(sourceIndex); + const dropRank = getBlockRankAtBoundary(sourceIndex, dropBoundary, boundaries); + + return sourceRank !== -1 && dropRank === sourceRank; +} + +function getBlockRankAtBoundary(sourceIndex: number, boundary: number, boundaries: number[]) { + const sourceRank = boundaries.indexOf(sourceIndex); + const boundaryRank = boundaries.indexOf(boundary); + if (sourceRank === -1 || boundaryRank === -1) return null; + + return boundaryRank > sourceRank ? boundaryRank - 1 : boundaryRank; +} + +function BlockDropZone({ zone, left, width }: { zone: DropZoneLayout; left: number; width: number }) { + const { setNodeRef } = useDroppable({ + id: `content-block-drop-${zone.boundary}`, + data: { boundary: zone.boundary }, + }); + + return ( +
+ ); +} + +function isDraggableBlock(element: HTMLElement) { + return ( + !element.classList.contains('paragraph-tail-placeholder') && + !element.matches('.paragraph-tail-placeholder, .is-empty') + ); +} + +/** Maps document children through ProseMirror positions so DOM widgets cannot shift block indexes. */ +export function getTopLevelBlockElements(editor: Editor, editorElement: HTMLElement) { + const blocks: Array<{ childIndex: number; element: HTMLElement }> = []; + let position = 0; + + for (let childIndex = 0; childIndex < editor.state.doc.childCount; childIndex += 1) { + const element = editor.view.nodeDOM(position); + if (element instanceof HTMLElement && element.parentElement === editorElement) { + blocks.push({ childIndex, element }); + } + position += editor.state.doc.child(childIndex).nodeSize; + } + + return blocks; +} + +export function makeDropZones(blocks: BlockLayout[]): DropZoneLayout[] { + if (blocks.length === 0) return []; + + return Array.from({ length: blocks.length + 1 }, (_, index) => { + const previous = blocks[index - 1]; + const next = blocks[index]; + const top = previous?.center ?? next.top; + const bottom = next?.center ?? previous.bottom; + const indicatorTop = previous && next ? (previous.bottom + next.top) / 2 : (next?.top ?? previous.bottom); + + return { + boundary: next?.childIndex ?? previous.childIndex + 1, + top, + height: Math.max(1, bottom - top), + indicatorTop, + }; + }); +} + +/** Finds the block beside a pointer in the editor's left gutter. */ +export function getGutterHoveredChildIndex( + blocks: BlockLayout[], + pointerX: number, + pointerY: number, + editorLeft: number +) { + if (pointerX < editorLeft - GUTTER_HOVER_WIDTH || pointerX > editorLeft) return null; + + const hoveredBlock = blocks.find((block, index) => { + const previous = blocks[index - 1]; + const next = blocks[index + 1]; + const hoverTop = previous ? (previous.bottom + block.top) / 2 : block.top; + const hoverBottom = next ? (block.bottom + next.top) / 2 : block.bottom; + + return pointerY >= hoverTop && pointerY <= hoverBottom; + }); + + return hoveredBlock?.childIndex ?? null; +} + +/** Moves one top-level document node to a boundary in the original document. */ +export function moveTopLevelBlock(editor: Editor, sourceIndex: number, dropBoundary: number): boolean { + const { doc } = editor.state; + if (sourceIndex < 0 || sourceIndex >= doc.childCount || dropBoundary < 0 || dropBoundary > doc.childCount) { + return false; + } + + // Dropping immediately before or after the source preserves its current order. + if (dropBoundary === sourceIndex || dropBoundary === sourceIndex + 1) return false; + + const sourceNode = doc.child(sourceIndex); + const sourcePosition = positionBeforeChild(doc, sourceIndex); + const boundaryPosition = positionBeforeChild(doc, dropBoundary); + const insertionPosition = + boundaryPosition > sourcePosition ? boundaryPosition - sourceNode.nodeSize : boundaryPosition; + const transaction = editor.state.tr + .delete(sourcePosition, sourcePosition + sourceNode.nodeSize) + .insert(insertionPosition, sourceNode) + .scrollIntoView(); + + editor.view.dispatch(transaction); + return true; +} + +function positionBeforeChild(doc: Editor['state']['doc'], childIndex: number) { + let position = 0; + for (let index = 0; index < childIndex; index += 1) { + position += doc.child(index).nodeSize; + } + return position; +} diff --git a/apps/web/partials/editor/editor.tsx b/apps/web/partials/editor/editor.tsx index ba8c5abafc..8ec7765e24 100644 --- a/apps/web/partials/editor/editor.tsx +++ b/apps/web/partials/editor/editor.tsx @@ -17,6 +17,7 @@ import { resolveGraphLinkHref } from '~/core/utils/graph-link'; import { Spacer } from '~/design-system/spacer'; +import { BlockReorder } from './block-reorder'; import { createCommandExtension } from './command-extension'; import { createEntityMentionExtension, entityMentionPluginKey } from './entity-mention-extension'; import { tiptapExtensions } from './extensions'; @@ -168,6 +169,15 @@ export function Editor({ shouldHandleOwnSpacing, spaceId, placeholder = null }: upsertEditorStateRef.current(json); }, [trackEditorDocument]); + // A deliberate drop is an explicit document change, so persist it immediately + // even if a suggestion popup happened to be active before the drag began. + const persistReorderedBlocks = React.useCallback(() => { + if (!editableRef.current || !editorRef.current) return; + const json = editorRef.current.getJSON(); + trackEditorDocument(json); + upsertEditorStateRef.current(json); + }, [trackEditorDocument]); + // When transitioning from edit → view mode, persist the editor content to the // store BEFORE useEditor destroys and recreates the editor. Without this, // content changes (like @mention links) would be lost because onBlur may not @@ -331,11 +341,22 @@ export function Editor({ shouldHandleOwnSpacing, spaceId, placeholder = null }:
- {editor ? : } + {editor ? ( + + + + ) : ( + + )} {shouldHandleOwnSpacing && editable && }
diff --git a/apps/web/partials/editor/id-extension.test.ts b/apps/web/partials/editor/id-extension.test.ts new file mode 100644 index 0000000000..dabbbaddc7 --- /dev/null +++ b/apps/web/partials/editor/id-extension.test.ts @@ -0,0 +1,41 @@ +import Document from '@tiptap/extension-document'; +import Paragraph from '@tiptap/extension-paragraph'; +import Text from '@tiptap/extension-text'; +import { Editor } from '@tiptap/react'; + +import { afterEach, describe, expect, it } from 'vitest'; + +import { createIdExtension, ensureUniqueNodeIds } from './id-extension'; + +let editor: Editor | null = null; + +afterEach(() => { + editor?.destroy(); + editor = null; +}); + +describe('ensureUniqueNodeIds', () => { + it('assigns IDs to missing and duplicate nodes before drag persistence', () => { + editor = new Editor({ + extensions: [Document, Paragraph, Text, createIdExtension('space-id')], + content: { + type: 'doc', + content: [ + { type: 'paragraph', attrs: { id: 'existing-id' } }, + { type: 'paragraph', attrs: { id: 'existing-id' } }, + { type: 'paragraph', attrs: { id: null } }, + ], + }, + }); + + expect(ensureUniqueNodeIds(editor)).toBe(2); + + const ids = Array.from({ length: editor.state.doc.childCount }, (_, index) => + String(editor?.state.doc.child(index).attrs.id) + ); + expect(ids[0]).toBe('existing-id'); + expect(new Set(ids).size).toBe(ids.length); + expect(ids).not.toContain('null'); + expect(ensureUniqueNodeIds(editor)).toBe(0); + }); +}); diff --git a/apps/web/partials/editor/id-extension.tsx b/apps/web/partials/editor/id-extension.tsx index a98314615b..ac84a5fd0a 100644 --- a/apps/web/partials/editor/id-extension.tsx +++ b/apps/web/partials/editor/id-extension.tsx @@ -1,4 +1,4 @@ -import { Extension, findChildren } from '@tiptap/core'; +import { Editor, Extension, findChildren } from '@tiptap/core'; import { ID } from '~/core/id'; @@ -35,40 +35,41 @@ export const createIdExtension = (spaceId: string) => { ]; }, onBlur() { - const { view, state } = this.editor; - const { tr, doc } = state; - - // If an editor adds content to the top of an editing with existing content we can - // end up with two blocks that have the same id. The below functionality de-dupes - // these ids and creates a new id for the second instance of the id. - // - // Functionally this means that adding a new block at the top keeps the id for the - // first block, but replaces the content, while creating a new block and id for - // what was the original content, but is now a new block with new content. - const nodeIds = new Set(); + ensureUniqueNodeIds(this.editor); + }, + }); +}; - const newNodes = findChildren(doc, node => { - // Check if we have two nodes with the same id, if we do, replace the second one - if (node.attrs.id !== null && nodeIds.has(node.attrs.id) && nodeTypes.includes(node.type.name)) { - return true; - } else { - nodeIds.add(node.attrs.id); - } +/** Assigns IDs to missing/duplicate editor nodes before their data is persisted. */ +export function ensureUniqueNodeIds(editor: Editor) { + const { view, state } = editor; + const { tr, doc } = state; - return node.attrs.id === null && nodeTypes.includes(node.type.name); - }); + // If an editor adds content to the top of an editing with existing content we can + // end up with two blocks that have the same id. Keep the first and replace later + // duplicates, while also assigning IDs to nodes that do not have one yet. + const nodeIds = new Set(); + const newNodes = findChildren(doc, node => { + if (!nodeTypes.includes(node.type.name)) return false; - newNodes.forEach(({ node, pos }) => { - tr.setNodeMarkup(pos, undefined, { - ...node.attrs, - id: ID.createEntityId(), - }); - }); + const nodeId = typeof node.attrs.id === 'string' ? node.attrs.id : null; + if (nodeId && nodeIds.has(nodeId)) return true; + if (nodeId) nodeIds.add(nodeId); - if (newNodes.length > 0) { - tr.setMeta('addToHistory', false); - view.dispatch(tr); - } - }, + return nodeId === null; }); -}; + + for (const { node, pos } of newNodes) { + tr.setNodeMarkup(pos, undefined, { + ...node.attrs, + id: ID.createEntityId(), + }); + } + + if (newNodes.length > 0) { + tr.setMeta('addToHistory', false); + view.dispatch(tr); + } + + return newNodes.length; +}