diff --git a/components/Blocks/TextBlock/index.tsx b/components/Blocks/TextBlock/index.tsx index f785e091..64f56758 100644 --- a/components/Blocks/TextBlock/index.tsx +++ b/components/Blocks/TextBlock/index.tsx @@ -251,7 +251,7 @@ function BaseTextBlock(props: BlockProps & { className?: string }) { handleMentionOpenChange, } = useMentionState(props.entityID, props); - let { mountRef, actionTimeout, overlay } = useMountProsemirror({ + let { mountRef, overlay } = useMountProsemirror({ props, openMentionAutocomplete, }); @@ -284,11 +284,6 @@ function BaseTextBlock(props: BlockProps & { className?: string }) { data: { type: "block-type-union", value: "horizontal-rule" }, }); } - if (actionTimeout.current) { - rep.undoManager.endGroup(); - window.clearTimeout(actionTimeout.current); - actionTimeout.current = null; - } }} onFocus={() => { handleMentionOpenChange(false); diff --git a/components/Blocks/TextBlock/keymap.ts b/components/Blocks/TextBlock/keymap.ts index 28818b34..cc724378 100644 --- a/components/Blocks/TextBlock/keymap.ts +++ b/components/Blocks/TextBlock/keymap.ts @@ -552,8 +552,10 @@ const enter = let index = siblings.findIndex( (sib) => sib.data.value === propsRef.current.entityID, ); + // Use the freshly-read sibling position, not the render-lagged + // propsRef snapshot, which can collide with the just-created item. position = generateKeyBetween( - propsRef.current.position, + siblings[index]?.data.position || propsRef.current.position, siblings[index + 1]?.data.position || null, ); } else { @@ -568,6 +570,14 @@ const enter = children[0]?.data.position || null, ); } + let [listStyle] = + (await repRef.current?.query((tx) => + scanIndex(tx).eav(propsRef.current.entityID, "block/list-style"), + )) || []; + let [checked] = + (await repRef.current?.query((tx) => + scanIndex(tx).eav(propsRef.current.entityID, "block/check-list"), + )) || []; await repRef.current?.mutate.addBlock({ newEntityID, factID: v7(), @@ -577,6 +587,14 @@ const enter = : propsRef.current.listData.parent, type: blockType, position, + list: { + listStyle: listStyle?.data.value, + checklist: checked + ? state.selection.anchor === 1 + ? checked.data.value + : false + : undefined, + }, }); if ( !createChild && @@ -591,38 +609,6 @@ const enter = after: null, }); } - await repRef.current?.mutate.assertFact({ - entity: newEntityID, - attribute: "block/is-list", - data: { type: "boolean", value: true }, - }); - // Copy list style (ordered/unordered) to new list item - let listStyle = await repRef.current?.query((tx) => - scanIndex(tx).eav(propsRef.current.entityID, "block/list-style"), - ); - if (listStyle?.[0]) { - await repRef.current?.mutate.assertFact({ - entity: newEntityID, - attribute: "block/list-style", - data: { - type: "list-style-union", - value: listStyle[0].data.value, - }, - }); - } - let checked = await repRef.current?.query((tx) => - scanIndex(tx).eav(propsRef.current.entityID, "block/check-list"), - ); - if (checked?.[0]) - await repRef.current?.mutate.assertFact({ - entity: newEntityID, - attribute: "block/check-list", - data: { - type: "boolean", - value: - state.selection.anchor === 1 ? checked?.[0].data.value : false, - }, - }); } // if the block is not a list, add a new text block after it if (!propsRef.current.listData) { @@ -681,26 +667,36 @@ const enter = }); } }; + // focusBlock silently no-ops until the new block's editor mounts, so a + // fixed timeout could drop focus and send the next keystrokes to the old + // block. Retry until it registers. let focusNewBlock = () => { - let block = useEditorStates.getState().editorStates[newEntityID]; - if (!block) return; - let tr = block.editor.tr; - if (newContent.content.size > 2) { - tr.replaceWith(0, tr.doc.content.size, newContent.content); - tr.setSelection(TextSelection.create(tr.doc, 0)); - let newState = block.editor.apply(tr); - setEditorState(newEntityID, { - editor: newState, - }); - } - focusBlock( - { - value: newEntityID, - parent: propsRef.current.parent, - type: "text", - }, - { type: "start" }, - ); + let attempts = 0; + let run = () => { + let block = useEditorStates.getState().editorStates[newEntityID]; + if (!block) { + if (attempts++ < 50) setTimeout(run, 10); + return; + } + let tr = block.editor.tr; + if (newContent.content.size > 2) { + tr.replaceWith(0, tr.doc.content.size, newContent.content); + tr.setSelection(TextSelection.create(tr.doc, 0)); + let newState = block.editor.apply(tr); + setEditorState(newEntityID, { + editor: newState, + }); + } + focusBlock( + { + value: newEntityID, + parent: propsRef.current.parent, + type: "text", + }, + { type: "start" }, + ); + }; + run(); }; asyncRun() diff --git a/components/Blocks/TextBlock/mountProsemirror.ts b/components/Blocks/TextBlock/mountProsemirror.ts index 3a3e2462..2e2fe659 100644 --- a/components/Blocks/TextBlock/mountProsemirror.ts +++ b/components/Blocks/TextBlock/mountProsemirror.ts @@ -55,7 +55,7 @@ export function useMountProsemirror({ props: BlockProps; openMentionAutocomplete: () => void; }) { - let { entityID, parent } = props; + let { entityID } = props; let rep = useReplicache(); let mountRef = useRef(null); const repRef = useRef | null>(null); @@ -66,8 +66,6 @@ export function useMountProsemirror({ let propsRef = useRef({ ...props, entity_set, alignment }); let handlePaste = useHandlePaste(entityID, propsRef); - const actionTimeout = useRef(null); - propsRef.current = { ...props, entity_set, alignment }; repRef.current = rep.rep; @@ -348,7 +346,7 @@ export function useMountProsemirror({ trackUndoRedo( tr, rep.undoManager, - actionTimeout, + entityID, setState(oldEditorState), setState(newState), ); @@ -366,38 +364,34 @@ export function useMountProsemirror({ }; }); } - }, [entityID, parent, value, cursorPlugin, handlePaste, rep]); - return { mountRef, actionTimeout, overlay }; + // `parent` is intentionally NOT a dependency: indent/outdent change a + // block's parent but keep the same entityID, and rebuilding the EditorView + // on reparent destroyed the caret (dropping focus to on every + // structural edit and its undo/redo). The view reads parent via propsRef. + }, [entityID, value, cursorPlugin, handlePaste, rep]); + return { mountRef, overlay }; } export function trackUndoRedo( tr: Transaction, undoManager: UndoManager, - actionTimeout: { current: number | null }, + coalesceKey: string, undo: () => void, redo: () => void, ) { let addToHistory = tr.getMeta("addToHistory"); let isBulkOp = tr.getMeta("bulkOp"); - // externalUndoGroup: the caller already has an undo group open and is keeping - // it open across async mutations (e.g. the Enter handler grouping a block - // split with the new block's creation), so skip the timeout-based group - // management here and just add the entry. Unlike bulkOp this does NOT suppress - // focus on undo (see setState above), so the cursor returns to the block. + // externalUndoGroup: the caller already holds a command group open across its + // async mutations (e.g. the Enter handler grouping a block split with the new + // block's creation), so just add to it. bulkOp likewise manages its own + // grouping. Otherwise coalesce consecutive text edits into one undo step + // (the undo manager owns the coalescing window, so a command can flush it and + // never merge with adjacent typing). let skipGroupManagement = isBulkOp || tr.getMeta("externalUndoGroup"); let docHasChanges = tr.steps.length !== 0 || tr.docChanged; if (addToHistory !== false && docHasChanges) { - if (actionTimeout.current) window.clearTimeout(actionTimeout.current); - else if (!skipGroupManagement) undoManager.startGroup(); - - if (!skipGroupManagement) { - actionTimeout.current = window.setTimeout(() => { - undoManager.endGroup(); - actionTimeout.current = null; - }, 200); - } - - undoManager.add({ undo, redo }); + if (skipGroupManagement) undoManager.add({ undo, redo }); + else undoManager.addGrouped({ undo, redo }, coalesceKey); } } diff --git a/components/Footnotes/FootnoteEditor.tsx b/components/Footnotes/FootnoteEditor.tsx index d5a850a7..5d877e00 100644 --- a/components/Footnotes/FootnoteEditor.tsx +++ b/components/Footnotes/FootnoteEditor.tsx @@ -71,7 +71,6 @@ function EditableFootnote(props: { cursorPlugin, overlay, } = useCollabText(props.footnoteEntityID); - let actionTimeout = useRef(null); let { pageID } = useFootnoteContext(); useLayoutEffect(() => { @@ -165,7 +164,7 @@ function EditableFootnote(props: { trackUndoRedo( tr, rep.undoManager, - actionTimeout, + props.footnoteEntityID, () => { this.focus(); this.updateState(oldState); diff --git a/docs/editor-behaviors.md b/docs/editor-behaviors.md new file mode 100644 index 00000000..b63e743c --- /dev/null +++ b/docs/editor-behaviors.md @@ -0,0 +1,26 @@ +# Text Editor Behaviors + +A living reference for how Leaflet's text editor is expected to behave. Add a +new section for each behavior as it is defined. Each entry specifies the +behavior from the user's point of view, independent of how it is implemented. + +## Enter on a blank list item + +When the cursor is in a **blank** list item (the item contains no text) and the +user presses **Enter**: + +1. **If the item is nested** — there are outer list layers above it — it + **outdents by one level**: the item moves up to its parent's level. No new + list item is created. +2. **If the item is already at the top level** — there are no more layers to + outdent — it is **converted into a normal, non-list text block**. The block + keeps its text type: a heading stays a heading (at the same heading level), a + blockquote stays a blockquote, and plain text stays plain text. Only the list + treatment (bullet / number / checkbox) is removed. + +This applies to every kind of list item — unordered, ordered, and checkbox +(to-do). + +**Not affected:** pressing Enter in a list item that has content behaves +normally — it splits the item / creates a new sibling list item at the same +depth. Only blank items outdent. diff --git a/src/replicache/clientMutationContext.ts b/src/replicache/clientMutationContext.ts index edc87fed..198e907e 100644 --- a/src/replicache/clientMutationContext.ts +++ b/src/replicache/clientMutationContext.ts @@ -113,7 +113,7 @@ export function clientMutationContext( } }, redo: () => { - rep.mutate.retractFact({ factID: id }); + rep.mutate.retractFact({ ignoreUndo: true, factID: id }); }, }); await tx.del(id); diff --git a/src/replicache/getBlocks.ts b/src/replicache/getBlocks.ts index fa3b27d4..a8cf20d0 100644 --- a/src/replicache/getBlocks.ts +++ b/src/replicache/getBlocks.ts @@ -52,7 +52,11 @@ export const getBlocksWithType = async ( ): Promise => { let children = ( await scan.eav(root.data.value, "card/block") - ).sort((a, b) => (a.data.position > b.data.position ? 1 : -1)); + ).sort((a, b) => { + if (a.data.position === b.data.position) + return a.id > b.id ? 1 : -1; + return a.data.position > b.data.position ? 1 : -1; + }); let type = (await scan.eav(root.data.value, "block/type"))[0]; let checklist = await scan.eav( root.data.value, @@ -131,7 +135,11 @@ export const getBlocksWithTypeLocal = ( ): Block[] => { let children = scan .eav(root.data.value, "card/block") - .sort((a, b) => (a.data.position > b.data.position ? 1 : -1)); + .sort((a, b) => { + if (a.data.position === b.data.position) + return a.id > b.id ? 1 : -1; + return a.data.position > b.data.position ? 1 : -1; + }); let type = scan.eav(root.data.value, "block/type")[0]; let listStyle = scan.eav(root.data.value, "block/list-style")[0]; let listNumber = scan.eav(root.data.value, "block/list-number")[0]; diff --git a/src/replicache/mutations.ts b/src/replicache/mutations.ts index fbc76803..761e0c02 100644 --- a/src/replicache/mutations.ts +++ b/src/replicache/mutations.ts @@ -76,6 +76,13 @@ const addBlock: Mutation<{ type: Fact<"block/type">["data"]["value"]; newEntityID: string; position: string; + // Set in the same mutation as the type so the block is never briefly a + // non-list block — otherwise a racing Enter sees no list data and inserts a + // plain paragraph, breaking the list. + list?: { + listStyle?: Fact<"block/list-style">["data"]["value"]; + checklist?: boolean; + }; }> = async (args, ctx) => { await ctx.createEntity({ entityID: args.newEntityID, @@ -96,6 +103,27 @@ const addBlock: Mutation<{ data: { type: "block-type-union", value: args.type }, attribute: "block/type", }); + if (args.list) { + await ctx.assertFact({ + entity: args.newEntityID, + attribute: "block/is-list", + data: { type: "boolean", value: true }, + }); + if (args.list.listStyle) { + await ctx.assertFact({ + entity: args.newEntityID, + attribute: "block/list-style", + data: { type: "list-style-union", value: args.list.listStyle }, + }); + } + if (args.list.checklist !== undefined) { + await ctx.assertFact({ + entity: args.newEntityID, + attribute: "block/check-list", + data: { type: "boolean", value: args.list.checklist }, + }); + } + } }; const addLastBlock: Mutation<{ @@ -227,6 +255,11 @@ const outdentBlock: Mutation<{ (f) => f.data.value === args.block, ); if (currentFactIndex === -1) return; + // Bail before the retraction below: a missing anchor here is a clean no-op, + // but checking after retracting would orphan the block and the siblings + // re-parented under it, dropping them from the document. + let index = newSiblings.findIndex((f) => f.data.value === args.after); + if (index === -1) return; // Filter out blocks that are being processed separately (e.g., in multi-select outdent) let excludeSet = new Set(args.excludeFromSiblings || []); let currentSiblingsAfter = currentSiblings @@ -253,8 +286,6 @@ const outdentBlock: Mutation<{ }); } - let index = newSiblings.findIndex((f) => f.data.value === args.after); - if (index === -1) return; let newPosition = generateKeyBetween( newSiblings[index]?.data.position, newSiblings[index + 1]?.data.position || null, diff --git a/src/undoManager.ts b/src/undoManager.ts index fc782632..7285d9d0 100644 --- a/src/undoManager.ts +++ b/src/undoManager.ts @@ -3,54 +3,123 @@ import { create } from "zustand"; export type UndoManager = ReturnType; export const useUndoState = create(() => ({ canUndo: false, canRedo: false })); + +// A run of typing coalesces into one undo step until the user pauses for this +// long (the timer resets on every keystroke, so only an actual gap between +// keystroke ends a run). 200ms was short enough that any normal-paced gap +// (typing slower than ~5 cps) flushed between keystrokes, undoing char by char. +const COALESCE_MS = 500; + +type Op = { undo: () => Promise | void; redo: () => Promise | void }; + export const createUndoManager = () => { - let isGrouping = false; let undoManager = new RociUndoManager({ onChange: (state) => { useUndoState.setState(state); }, }); + + // @rocicorp/undo allows only one group open at a time. Three producers share + // this single instance — explicit command groups (Tab/Enter/Backspace via + // startGroup/withUndoGroup) and coalescing text-edit groups (addGrouped) — + // so all grouping state is owned here to keep them from closing each other's + // groups (which previously split a multi-mutation op across boundaries and + // orphaned blocks on undo). + let groupDepth = 0; // open explicit command groups (counted so nesting is safe) + let coalesceOpen = false; // a text-edit coalescing group is open + let currentCoalesceKey: string | undefined; // span key of the open group + let coalesceTimer: ReturnType | null = null; + // Serialize undo/redo so rapid keypresses don't re-enter @rocicorp/undo's + // recursive group walk (it mutates a shared index) and corrupt the stack. + let opChain: Promise = Promise.resolve(); + + let clearCoalesceTimer = () => { + if (coalesceTimer !== null) { + clearTimeout(coalesceTimer); + coalesceTimer = null; + } + }; + // Finalize the open coalescing text group, if any. + let flushCoalesce = () => { + clearCoalesceTimer(); + if (coalesceOpen) { + coalesceOpen = false; + currentCoalesceKey = undefined; + undoManager.endGroup(); + } + }; + let um = { - add: (args: { - undo: () => Promise | void; - redo: () => Promise | void; - }) => { + add: (args: Op) => { undoManager.add(args); }, - startGroup: (groupName?: string) => { - if (isGrouping) { - undoManager.endGroup(); - } - isGrouping = true; - undoManager.startGroup(); + // Open an atomic command group. Finalizes any pending text-edit group first + // so a command never merges with adjacent typing, and opens only one raw + // group regardless of nesting depth. + startGroup: () => { + flushCoalesce(); + if (groupDepth === 0) undoManager.startGroup(); + groupDepth++; }, endGroup: () => { - isGrouping = false; - undoManager.endGroup(); + if (groupDepth === 0) return; + groupDepth--; + if (groupDepth === 0) undoManager.endGroup(); }, - undo: () => undoManager.undo(), - redo: () => undoManager.redo(), withUndoGroup: (cb: () => T): T => { - const wasGrouping = isGrouping; - if (!wasGrouping) um.startGroup(); - const endIfOwner = () => { - if (!wasGrouping) um.endGroup(); - }; + um.startGroup(); + let end = () => um.endGroup(); let r: T; try { r = cb(); } catch (e) { - endIfOwner(); + end(); throw e; } - // Replicache mutator bodies — and the undoManager.add calls they make — - // run asynchronously, after the callback's synchronous portion returns. - // If the callback is async, hold the group open until it settles so those - // mutations land in this group instead of as separate undo entries. - if (r instanceof Promise) return r.finally(endIfOwner) as T; - endIfOwner(); + // Hold the group open until an async command settles so all its mutations + // (whose undoManager.add calls run as the awaited mutators execute) land + // in this group. + if (r instanceof Promise) return r.finally(end) as T; + end(); return r; }, + // Add a text edit that coalesces with adjacent edits sharing the same span + // key (e.g. the block/footnote entityID) within COALESCE_MS. A different + // key — or a lapsed window — starts a fresh group; an open command group + // (groupDepth > 0) just absorbs the edit. + addGrouped: (args: Op, coalesceKey?: string) => { + if (groupDepth > 0) { + undoManager.add(args); + return; + } + // A different span finalizes the open group now, independent of the + // timer, so edits to two different spans never merge into one undo step. + if (coalesceOpen && coalesceKey !== currentCoalesceKey) flushCoalesce(); + if (!coalesceOpen) { + undoManager.startGroup(); + coalesceOpen = true; + currentCoalesceKey = coalesceKey; + } + undoManager.add(args); + clearCoalesceTimer(); + coalesceTimer = setTimeout(flushCoalesce, COALESCE_MS); + }, + undo: () => { + flushCoalesce(); + opChain = opChain.then(() => undoManager.undo()).then( + () => {}, + () => {}, + ); + return opChain; + }, + redo: () => { + flushCoalesce(); + opChain = opChain.then(() => undoManager.redo()).then( + () => {}, + () => {}, + ); + return opChain; + }, }; return um; }; diff --git a/src/utils/list-operations.ts b/src/utils/list-operations.ts index 8b46dd8a..b2ee5f01 100644 --- a/src/utils/list-operations.ts +++ b/src/utils/list-operations.ts @@ -1,7 +1,6 @@ import { Block } from "components/Blocks/Block"; import { Replicache } from "replicache"; import type { ReplicacheMutators } from "src/replicache"; -import { v7 } from "uuid"; export function orderListItems( block: Block, @@ -45,10 +44,13 @@ export async function indent( if (!newParent) return { success: false }; if (foldState && foldState.foldedBlocks.includes(newParent.entity)) foldState.toggleFold(newParent.entity); - rep?.mutate.retractFact({ factID: block.factID }); - rep?.mutate.addLastBlock({ + // Reparent the block's existing card/block fact in one mutation (reusing its + // factID) rather than retract-old + add-new-id. assertFact moves the fact and + // captures the old (parent, position) as the single undo entry, so a split or + // mis-grouped undo can't half-apply it and orphan the block. + await rep?.mutate.addLastBlock({ parent: newParent.entity, - factID: v7(), + factID: block.factID, entity: block.value, });