From 8d58cb42561bb569feb5c4c7d347e556c56b310f Mon Sep 17 00:00:00 2001 From: lastday97 Date: Fri, 21 Aug 2026 16:03:50 +0800 Subject: [PATCH] fix: keep caret in place when editing empty table cells --- src/index.js | 11 ++++-- src/utils/__tests__/inlineEditable.test.js | 43 ++++++++++++++++++++++ src/utils/inlineEditable.js | 28 ++++++++++++-- 3 files changed, 75 insertions(+), 7 deletions(-) create mode 100644 src/utils/__tests__/inlineEditable.test.js diff --git a/src/index.js b/src/index.js index 9c25e19..a0fb494 100644 --- a/src/index.js +++ b/src/index.js @@ -3,7 +3,7 @@ import '@logseq/libs' import parseMarkdownTable from './utils/parseRawInputByMarkdownIt' import { splitStrByTable } from './utils/splitStrByTable' import { looksLikeMarkdownTable, markdownTableToMatrix } from './utils/detectMarkdownTable' -import { attachInlineEditing, deleteInFocusedTableCell, insertInFocusedTableCell, moveCaretInFocusedTableCell, moveInFocusedTableCell, prepareInlineRenderer, resumePinnedToolbar } from './utils/inlineEditable' +import { attachInlineEditing, deleteInFocusedTableCell, insertInFocusedTableCell, moveCaretInFocusedTableCell, moveInFocusedTableCell, prepareInlineRenderer, resumePinnedToolbar, syncCellText } from './utils/inlineEditable' import i18n from './locales/i18n' import './index.css' @@ -359,16 +359,21 @@ if (isInBrowser) { // Inline styles beat Logseq's higher-specificity prose/table // rules (which were overriding our class-based stylesheet). const cellStyle = { textAlign: 'left', verticalAlign: 'top', whiteSpace: 'pre' } + // Cells carry NO `children` prop: text is applied by the + // syncCellText ref instead (uncontrolled contenteditable). If + // React owned the text child, the updateBlock echo re-render + // could rewrite it via setTextContent and yank the caret to the + // start of the cell (worst for empty cells). return React.createElement('div', { key: 's' + si, className: 'lsp-mdtable-scroll' }, React.createElement('table', { className: 'lsp-mdt' }, [ React.createElement('thead', { key: 'h' }, React.createElement('tr', null, - head.map((c, i) => React.createElement('th', { key: i, style: cellStyle }, c)))), + head.map((c, i) => React.createElement('th', { key: i, style: cellStyle, ref: (el) => syncCellText(el, c) })))), React.createElement('tbody', { key: 'b' }, body.map((row, ri) => React.createElement('tr', { key: ri }, head.map((_, ci) => - React.createElement('td', { key: ci, style: cellStyle }, row[ci] ?? ''))))) + React.createElement('td', { key: ci, style: cellStyle, ref: (el) => syncCellText(el, row[ci] ?? '') }))))) ])) }) const dbRaw = Number(logseq.settings?.inlineEditDebounceMs) diff --git a/src/utils/__tests__/inlineEditable.test.js b/src/utils/__tests__/inlineEditable.test.js new file mode 100644 index 0000000..b2f3e07 --- /dev/null +++ b/src/utils/__tests__/inlineEditable.test.js @@ -0,0 +1,43 @@ +import { syncCellText } from '../inlineEditable' + +const makeCell = (text) => { + const el = document.createElement('td') + el.textContent = text + return el +} + +describe('syncCellText', () => { + it('no-ops when the live text already matches (echo re-render)', () => { + const el = makeCell('hello') + syncCellText(el, 'hello') + expect(el.textContent).toBe('hello') + // The original text node must be reused, not rebuilt — replacing it is + // what resets the caret to the start of the cell. + const textNodes = Array.from(el.childNodes).filter(n => n.nodeType === Node.TEXT_NODE) + expect(textNodes).toHaveLength(1) + expect(textNodes[0].nodeValue).toBe('hello') + }) + + it('writes text into an empty cell', () => { + const el = makeCell('') + syncCellText(el, 'initial') + expect(el.textContent).toBe('initial') + }) + + it('clears a cell when the new text is empty', () => { + const el = makeCell('old') + syncCellText(el, '') + expect(el.textContent).toBe('') + }) + + it('overwrites genuinely different text (structural ops)', () => { + const el = makeCell('old') + syncCellText(el, 'new') + expect(el.textContent).toBe('new') + }) + + it('survives null/undefined elements', () => { + expect(() => syncCellText(null, 'x')).not.toThrow() + expect(() => syncCellText(undefined, 'x')).not.toThrow() + }) +}) diff --git a/src/utils/inlineEditable.js b/src/utils/inlineEditable.js index d1346ba..d1b111f 100644 --- a/src/utils/inlineEditable.js +++ b/src/utils/inlineEditable.js @@ -7,10 +7,16 @@ // // Two hazards are designed around here: // -// 1. Caret jumps. Every updateBlock re-runs the block renderer. We never -// re-sync cell text from props; React reconciles the identical serialized -// text as a no-op, so the live contentEditable DOM (and caret) is left -// untouched. We also ignore the echo of our own write. +// 1. Caret jumps. Every updateBlock re-runs the block renderer, so the React +// tree is reconciled against the freshly-written content. Cells are +// rendered WITHOUT a `children` prop (see syncCellText): React never owns +// their live text, so it can never rewrite it and displace the caret. The +// previous design relied on React treating identical serialized text as a +// no-op, which broke for empty cells — `setTextContent(node, '')` +// rebuilds the text child and the caret jumps to the start of the cell. +// Initial text is applied once via the cell ref, and only when it differs +// from the live DOM, so the normal echo re-render is a no-op. We also +// ignore the echo of our own write. // // 2. Structural corruption. A literal `|` or stray `\r` typed into a cell // would break re-parsing (the parser splits naively on `|`). Cells are @@ -34,6 +40,20 @@ const pendingToolbar = new Map() // ancestor, so we measure the real clipping ancestor and pin the scroll // box to a pixel width — the table then scrolls inside its own box. +// Apply initial cell text via a cell ref. Cells are rendered without a +// `children` prop (index.js) so React never manages their text: a cell has no +// React-owned text child, `shouldSetTextContent` stays false, and React can +// never call setTextContent on it during a re-render. That is what prevents +// the caret jump for empty cells (see the header note). We write the text once +// here, guarded by a comparison so the normal echo re-render (content === live +// DOM text) is a no-op and the caret is left exactly where the user put it. +// Structural ops (insert/delete/sort/move) produce genuinely different text, +// which this overwrites; resumePinnedToolbar re-anchors the caret afterwards. +export const syncCellText = (el, text) => { + if (!el) return + if (el.textContent !== text) el.textContent = text +} + const isScrollContainer = (cs) => /(auto|scroll|hidden)/.test(cs.overflowX) || /(auto|scroll)/.test(cs.overflowY)