Skip to content
69 changes: 60 additions & 9 deletions packages/koenig-lexical/src/plugins/TKPlugin.jsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import CardContext from '../context/CardContext';
import {$createTKNode, $isTKNode, ExtendedTextNode, TKNode} from '@tryghost/kg-default-nodes';
import {$getNodeByKey, $getSelection, $isDecoratorNode, $isRangeSelection, TextNode} from 'lexical';
import {$isListItemNode, $isListNode} from '@lexical/list';
import {SELECT_CARD_COMMAND} from './KoenigBehaviourPlugin';
import {createPortal} from 'react-dom';
import {useCallback, useContext, useEffect, useState} from 'react';
Expand All @@ -11,6 +12,41 @@ import {useTKContext} from '../context/TKContext';
const REGEX = new RegExp(/(^|.)([^\p{L}\p{N}\s]*(TK|Tk|tk)+[^\p{L}\p{N}\s]*)(.)?/u);
const WORD_CHAR_REGEX = new RegExp(/\p{L}|\p{N}/u);

// TK Indicator positioning constants
const INDICATOR_OFFSET_RIGHT = -56;
const INDICATOR_OFFSET_TOP = 4;

// Node type constants
const NODE_TYPES = {
LIST_ITEM: 'LI'
};

// Helper function to get effective top-level element, treating list items as containers
function getEffectiveTopLevelElement(node) {
const topLevel = node.getTopLevelElement();

if (!topLevel) {
return null;
}

// If the top-level element is not a list, return it directly
if (!$isListNode(topLevel)) {
return topLevel;
}

// Find the containing list item for list nodes
let currentNode = node;
while (currentNode?.getParent()) {
if ($isListItemNode(currentNode)) {
return currentNode;
}
currentNode = currentNode.getParent();
}

// Fallback to top-level element if no list item found
return topLevel;
}

function TKIndicator({editor, rootElement, parentKey, nodeKeys}) {
const tkClasses = editor._config.theme.tk?.split(' ') || [];
const tkHighlightClasses = editor._config.theme.tkHighlighted?.split(' ') || [];
Expand All @@ -20,14 +56,18 @@ function TKIndicator({editor, rootElement, parentKey, nodeKeys}) {
// position element relative to the TK Node containing element
const calculatePosition = useCallback(() => {
let top = 0;
let right = -56;
let right = INDICATOR_OFFSET_RIGHT;

const rootElementRect = rootElement.getBoundingClientRect();

const positioningElement = containingElement.querySelector('[data-kg-card]') || containingElement;
// Determine positioning element based on container type
const positioningElement = containingElement.nodeName === NODE_TYPES.LIST_ITEM
? containingElement
: containingElement.querySelector('[data-kg-card]') || containingElement;

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix crash when containingElement is null (initial render + observer)

calculatePosition dereferences containingElement before the post-hook early return; useEffect also observes a null element. Both can throw.

Apply:

-    const calculatePosition = useCallback(() => {
-        let top = 0;
-        let right = INDICATOR_OFFSET_RIGHT;
+    const calculatePosition = useCallback(() => {
+        // Guard: container may be missing during initial renders/mutations
+        if (!containingElement) {
+            return {top: 0, right: INDICATOR_OFFSET_RIGHT};
+        }
+        let top = 0;
+        let right = INDICATOR_OFFSET_RIGHT;
@@
-        const positioningElement = containingElement.nodeName === NODE_TYPES.LIST_ITEM
+        const positioningElement = (containingElement.tagName === 'LI')
             ? containingElement
             : containingElement.querySelector('[data-kg-card]') || containingElement;
@@
-    const [position, setPosition] = useState(calculatePosition());
+    const [position, setPosition] = useState(calculatePosition());
@@
-        observer.observe(rootElement);
-        observer.observe(containingElement);
+        observer.observe(rootElement);
+        if (containingElement) {
+            observer.observe(containingElement);
+        }

Also applies to: 79-79, 150-161

🤖 Prompt for AI Agents
In packages/koenig-lexical/src/plugins/TKPlugin.jsx around lines 56-67 (and
additionally at 79 and 150-161), calculatePosition and the useEffect observer
assume containingElement (and rootElement) are non-null and call methods on them
causing crashes on initial render or when the observer supplies null; add
null/undefined guards at the start of calculatePosition and in the useEffect
callbacks so they return early if containingElement or rootElement is falsy, and
when determining positioningElement only access containingElement.nodeName or
querySelector after confirming containingElement exists; ensure the callback
dependencies remain correct and that any observers only attach when the element
references are present.

const positioningElementRect = positioningElement.getBoundingClientRect();

top = positioningElementRect.top - rootElementRect.top + 4;
top = positioningElementRect.top - rootElementRect.top + INDICATOR_OFFSET_TOP;

if (positioningElementRect.right > rootElementRect.right) {
right = right - (positioningElementRect.right - rootElementRect.right);
Expand Down Expand Up @@ -83,12 +123,17 @@ function TKIndicator({editor, rootElement, parentKey, nodeKeys}) {
}

nodeKeys.forEach((key) => {
const element = editor.getElementByKey(key);
if (!element) {
return;
}

if (isHighlighted) {
editor.getElementByKey(key).classList.remove(...tkClasses);
editor.getElementByKey(key).classList.add(...tkHighlightClasses);
element.classList.remove(...tkClasses);
element.classList.add(...tkHighlightClasses);
} else {
editor.getElementByKey(key).classList.add(...tkClasses);
editor.getElementByKey(key).classList.remove(...tkHighlightClasses);
element.classList.add(...tkClasses);
element.classList.remove(...tkHighlightClasses);
}
});
};
Expand All @@ -115,6 +160,11 @@ function TKIndicator({editor, rootElement, parentKey, nodeKeys}) {
};
}, [rootElement, containingElement, calculatePosition]);

// Early return if containing element is not available (after all hooks)
if (!containingElement) {
return null;
}

const style = {
top: `${position.top}px`,
right: `${position.right}px`
Expand Down Expand Up @@ -157,7 +207,8 @@ export default function TKPlugin() {
if (mutation === 'destroyed') {
removeEditorTkNode(editor.getKey(), tkNodeKey);
} else {
const parentNodeKey = $getNodeByKey(tkNodeKey).getTopLevelElement()?.getKey();
const effectiveTopLevel = getEffectiveTopLevelElement($getNodeByKey(tkNodeKey));
const parentNodeKey = effectiveTopLevel?.getKey();
const topLevelNodeKey = parentEditorNodeKey || parentNodeKey;
Comment on lines +214 to 216
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

🧩 Analysis chain

Guard addEditorTkNode when no container key is resolved

Effective container can be null in rare cases; avoid inserting undefined keys into tkNodeMap.

Apply:

-const topLevelNodeKey = parentEditorNodeKey || parentNodeKey;
-addEditorTkNode(editor.getKey(), topLevelNodeKey, tkNodeKey);
+const topLevelNodeKey = parentEditorNodeKey || parentNodeKey;
+if (topLevelNodeKey) {
+    addEditorTkNode(editor.getKey(), topLevelNodeKey, tkNodeKey);
+}

If you suspect this never happens in practice, quick check:


🏁 Script executed:

#!/bin/bash
# Look for any safeguards already in addEditorTkNode implementation
rg -nC2 "function\s+addEditorTkNode|addEditorTkNode\s*\(" --type=js --type=jsx

Length of output: 27


🏁 Script executed:

#!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
echo "Git branch: $(git rev-parse --abbrev-ref HEAD || true)"
echo "--- Search for addEditorTkNode occurrences ---"
rg -nC3 "addEditorTkNode" || true
echo "--- Search for possible definitions ---"
rg -nC3 "function\s+addEditorTkNode" || true
rg -nC3 "const\s+addEditorTkNode\s*=" || true
rg -nC3 "export\s+function\s+addEditorTkNode" || true
echo "--- Search for tkNodeMap occurrences ---"
rg -nC3 "tkNodeMap" || true
echo "--- Show packages/koenig-lexical/src/plugins/TKPlugin.jsx (first 300 lines) ---"
if [ -f "packages/koenig-lexical/src/plugins/TKPlugin.jsx" ]; then
  sed -n '1,300p' packages/koenig-lexical/src/plugins/TKPlugin.jsx
else
  echo "TKPlugin.jsx not found at expected path; listing directory:"
  ls -la packages/koenig-lexical/src/plugins || true
fi

Length of output: 18029


🏁 Script executed:

#!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
echo "---- show addEditorTkNode occurrences ----"
rg -nC3 "addEditorTkNode" packages/koenig-lexical || true
echo "---- print TKContext.jsx ----"
if [ -f "packages/koenig-lexical/src/context/TKContext.jsx" ]; then
  sed -n '1,260p' packages/koenig-lexical/src/context/TKContext.jsx
  echo "---- remainder ----"
  sed -n '261,520p' packages/koenig-lexical/src/context/TKContext.jsx || true
else
  echo "packages/koenig-lexical/src/context/TKContext.jsx not found"
  ls -la packages/koenig-lexical/src || true
fi

Length of output: 6996


Guard addEditorTkNode when no container key is resolved

Effective container can be null; avoid inserting undefined keys into tkNodeMap.

Apply:

-const topLevelNodeKey = parentEditorNodeKey || parentNodeKey;
-addEditorTkNode(editor.getKey(), topLevelNodeKey, tkNodeKey);
+const topLevelNodeKey = parentEditorNodeKey || parentNodeKey;
+if (topLevelNodeKey) {
+    addEditorTkNode(editor.getKey(), topLevelNodeKey, tkNodeKey);
+}

File: packages/koenig-lexical/src/plugins/TKPlugin.jsx (around lines 208-211)

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const effectiveTopLevel = getEffectiveTopLevelElement($getNodeByKey(tkNodeKey));
const parentNodeKey = effectiveTopLevel?.getKey();
const topLevelNodeKey = parentEditorNodeKey || parentNodeKey;
const effectiveTopLevel = getEffectiveTopLevelElement($getNodeByKey(tkNodeKey));
const parentNodeKey = effectiveTopLevel?.getKey();
const topLevelNodeKey = parentEditorNodeKey || parentNodeKey;
if (topLevelNodeKey) {
addEditorTkNode(editor.getKey(), topLevelNodeKey, tkNodeKey);
}
🤖 Prompt for AI Agents
In packages/koenig-lexical/src/plugins/TKPlugin.jsx around lines 208-210, the
code computes topLevelNodeKey from an effective top-level element which can be
null and may lead to inserting undefined into tkNodeMap; before calling
addEditorTkNode (or otherwise adding entries keyed by topLevelNodeKey), guard by
checking that topLevelNodeKey is truthy (e.g., if (!topLevelNodeKey)
return/continue or skip the add) so no undefined keys are inserted into
tkNodeMap.

addEditorTkNode(editor.getKey(), topLevelNodeKey, tkNodeKey);
}
Expand Down Expand Up @@ -247,7 +298,7 @@ export default function TKPlugin() {
const parentContainer = editor.getElementByKey(parentKey);

if (!parentContainer) {
return false;
return null;
}

return (
Expand Down
36 changes: 36 additions & 0 deletions packages/koenig-lexical/test/e2e/plugins/TKPlugin.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,42 @@ test.describe('TK Plugin', async function () {
await expect(page.getByTestId('tk-indicator')).toHaveCount(3);
});

test('creates separate TK indicators for each list item containing TK', async function () {
await focusEditor(page);

await page.keyboard.type('- First item with TK');
await page.keyboard.press('Enter');
await page.keyboard.type('Second item with TK');
await page.keyboard.press('Enter');
await page.keyboard.type('Third item without placeholder');
await page.keyboard.press('Enter');
await page.keyboard.type('Fourth item with TK');

// Should have 3 separate TK indicators, one for each list item containing TK
await expect(page.getByTestId('tk-indicator')).toHaveCount(3);
});

test('positions TK indicators correctly for individual list items', async function () {
await focusEditor(page);

await page.keyboard.type('- First TK item');
await page.keyboard.press('Enter');
await page.keyboard.type('Second TK item');

const indicators = page.getByTestId('tk-indicator');
await expect(indicators).toHaveCount(2);

// Get positions to verify they're different (one per list item)
const firstIndicator = indicators.first();
const secondIndicator = indicators.last();

const firstBox = await firstIndicator.boundingBox();
const secondBox = await secondIndicator.boundingBox();

// The indicators should have different vertical positions
expect(Math.abs(firstBox.y - secondBox.y)).toBeGreaterThan(10);
});

test('clicking the indicator selects the first TK node in the parent', async function () {
await focusEditor(page);
await page.keyboard.type('TK and TK and TK');
Expand Down
Loading