Skip to content

Commit e61f315

Browse files
refactor: address PR review feedback
- Remove unused NodeSelection import from table.js - Remove dead trailingParagraphPos from insertTopLevelTableWithSeparators - Remove redundant .map() in findClosestLineInDirection - Pass pre-computed currentMetrics instead of measuring the element twice
1 parent c82e383 commit e61f315

2 files changed

Lines changed: 16 additions & 17 deletions

File tree

packages/super-editor/src/extensions/table/table.js

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ import {
188188
} from '../table-cell/helpers/legacyBorderMigration.js';
189189
import { isInTable } from '@helpers/isInTable.js';
190190
import { findParentNode } from '@helpers/findParentNode.js';
191-
import { NodeSelection, TextSelection, Plugin, PluginKey } from 'prosemirror-state';
191+
import { TextSelection, Plugin, PluginKey } from 'prosemirror-state';
192192
import { isCellSelection } from './tableHelpers/isCellSelection.js';
193193
import {
194194
addColumnBefore as originalAddColumnBefore,
@@ -274,37 +274,35 @@ function createTableSeparatorParagraph(schema) {
274274
* @param {number} pos
275275
* @param {import('prosemirror-model').Node} tableNode
276276
* @param {{ from?: number, to?: number }} [replaceRange]
277-
* @returns {{ inserted: boolean, trailingParagraphPos: number | null }}
277+
* @returns {{ inserted: boolean }}
278278
*/
279279
function insertTopLevelTableWithSeparators(tr, doc, pos, tableNode, replaceRange = {}) {
280280
const replaceFrom = replaceRange.from ?? pos;
281281
const replaceTo = replaceRange.to ?? pos;
282282
const sep = tableSeparatorNeeds(doc, pos, replaceRange);
283283
if (!sep.before && !sep.after) {
284284
tr.replaceWith(replaceFrom, replaceTo, tableNode);
285-
return { inserted: true, trailingParagraphPos: null };
285+
return { inserted: true };
286286
}
287287

288288
const nodes = [];
289-
let trailingParagraphPos = null;
290289

291290
if (sep.before) {
292291
const before = createTableSeparatorParagraph(doc.type.schema);
293-
if (!before) return { inserted: false, trailingParagraphPos: null };
292+
if (!before) return { inserted: false };
294293
nodes.push(before);
295294
}
296295

297296
nodes.push(tableNode);
298297

299298
if (sep.after) {
300299
const after = createTableSeparatorParagraph(doc.type.schema);
301-
if (!after) return { inserted: false, trailingParagraphPos: null };
302-
trailingParagraphPos = pos + nodes.reduce((size, node) => size + node.nodeSize, 0);
300+
if (!after) return { inserted: false };
303301
nodes.push(after);
304302
}
305303

306304
tr.replaceWith(replaceFrom, replaceTo, Fragment.from(nodes));
307-
return { inserted: true, trailingParagraphPos };
305+
return { inserted: true };
308306
}
309307

310308
/**

packages/super-editor/src/extensions/vertical-navigation/vertical-navigation.js

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,13 @@ function findAdjacentLineElement(currentLine, direction, caretX) {
342342
if (!currentLineMetrics) return null;
343343

344344
const currentPageLines = getPageLineElements(page);
345-
const adjacentOnCurrentPage = findClosestLineInDirection(currentPageLines, currentLine, direction, caretX);
345+
const adjacentOnCurrentPage = findClosestLineInDirection(
346+
currentPageLines,
347+
currentLine,
348+
currentLineMetrics,
349+
direction,
350+
caretX,
351+
);
346352
if (adjacentOnCurrentPage) return adjacentOnCurrentPage;
347353

348354
const pages = Array.from(page.parentElement?.querySelectorAll?.(`.${pageClass}`) ?? []);
@@ -429,14 +435,12 @@ function getPageLineElements(page) {
429435
* Chooses the closest visual line in the requested direction.
430436
* @param {Element[]} lineEls
431437
* @param {Element} currentLine
438+
* @param {NonNullable<ReturnType<typeof getLineMetrics>>} currentMetrics
432439
* @param {number} direction
433440
* @param {number} caretX
434441
* @returns {Element | null}
435442
*/
436-
function findClosestLineInDirection(lineEls, currentLine, direction, caretX) {
437-
const currentMetrics = getLineMetrics(currentLine);
438-
if (!currentMetrics) return null;
439-
443+
function findClosestLineInDirection(lineEls, currentLine, currentMetrics, direction, caretX) {
440444
const directionalCandidates = lineEls
441445
.filter((line) => line !== currentLine)
442446
.map((line) => ({ line, metrics: getLineMetrics(line) }))
@@ -464,10 +468,7 @@ function findClosestLineInDirection(lineEls, currentLine, direction, caretX) {
464468
isWithinTolerance(metrics.centerY, targetRowCenterY, getRowTolerance(currentMetrics, metrics)),
465469
);
466470

467-
return chooseLineClosestToX(
468-
rowCandidates.map(({ line, metrics }) => ({ line, metrics })),
469-
caretX,
470-
);
471+
return chooseLineClosestToX(rowCandidates, caretX);
471472
}
472473

473474
/**

0 commit comments

Comments
 (0)