Skip to content

fix: correct header card layout parsing based on DOM structure and classes #1774

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
saschabuehrle wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: correct header card layout parsing based on DOM structure and classes #1774

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5f5602b
fix: correct header card layout parsing based on DOM structure and cl…
saschabuehrle Mar 16, 2026
c45afb6
fix: move picture element inside content div in test to match split l…
saschabuehrle Mar 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion packages/kg-default-nodes/lib/nodes/header/parsers/header-parser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,29 @@ export function parseHeaderNode(HeaderNode) {
const buttonElement = div.querySelector('.kg-header-card-button');
const alignment = div.classList.contains('kg-align-center') ? 'center' : '';
const backgroundImageSrc = div.querySelector('.kg-header-card-image')?.getAttribute('src');
const layout = backgroundImageSrc ? 'split' : '';

// Determine layout based on classes and DOM structure, not just image presence
let layout = '';
if (backgroundImageSrc) {
// Check for explicit layout class first
const hasSplitClass = div.classList.contains('kg-layout-split');
const hasContentWideClass = div.classList.contains('kg-content-wide');

// Check DOM structure: picture as direct child = full, picture inside content = split
const picture = div.querySelector('picture');
const content = div.querySelector('.kg-header-card-content');
const isPictureDirectChild = picture && picture.parentElement === div;
const isPictureInContent = picture && content && content.contains(picture);

if (hasSplitClass || isPictureInContent) {
layout = 'split';
} else if (hasContentWideClass || isPictureDirectChild) {
layout = ''; // full/wide layout (empty string)
Copy link

@cursor cursor bot Mar 16, 2026

Choose a reason for hiding this comment

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

Parser returns empty string, node constructor coerces to 'full'

High Severity

The parser sets layout = '' for full/wide layouts, but HeaderNode's default for layout is 'full', and the constructor uses data[prop.name] || prop.default — since '' is falsy, '' || 'full' evaluates to 'full'. This means node.layout will be 'full', not ''. The new test assertions at lines 419 and 457 (node.layout.should.equal('')) will fail. The parser needs to return 'full' explicitly instead of ''.

Additional Locations (2)
Fix in Cursor Fix in Web

} else {
// Fallback to old behavior if structure is ambiguous
layout = 'split';
}
Copy link

@cursor cursor bot Mar 16, 2026

Choose a reason for hiding this comment

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

Existing V2 parser test breaks with new logic

High Severity

The pre-existing "parses a header card V2" test (line 359 of the test file) has HTML where <picture> is a direct child of the card div and no kg-layout-split class is present. The new parser identifies isPictureDirectChild as true and returns layout = '' (→ 'full'), but that test asserts node.layout.should.equal('split'). This existing test will fail with the new parser logic.

Fix in Cursor Fix in Web

}
const backgroundColor = div.classList.contains('kg-style-accent') ? 'accent' : div.getAttribute('data-background-color');
const buttonColor = buttonElement?.getAttribute('data-button-color') || '';
const textColor = headerElement?.getAttribute('data-text-color') || '';
Expand Down
59 changes: 58 additions & 1 deletion packages/kg-default-nodes/test/nodes/header.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,8 +359,8 @@ describe('HeaderNode', function () {
it('parses a header card V2', editorTest(function () {
const htmlstring = `
<div class="kg-card kg-header-card kg-v2 kg-style-accent" data-background-color="#abcdef">
<picture><img class="kg-header-card-image" src="https://example.com/image.jpg" alt="" /></picture>
<div class="kg-header-card-content">
<picture><img class="kg-header-card-image" src="https://example.com/image.jpg" alt="" /></picture>
<div class="kg-header-card-text kg-align-center">
<h2 class="kg-header-card-heading" data-text-color="#abcdef">Header</h2>
<p class="kg-header-card-subheading" data-text-color="#abcdef">Subheader</p>
Expand Down Expand Up @@ -400,6 +400,63 @@ describe('HeaderNode', function () {
const node = nodes[0];
node.version.should.equal(1);
}));

it('parses full/wide layout when kg-content-wide class is present', editorTest(function () {
const htmlstring = `
<div class="kg-card kg-header-card kg-v2 kg-width-full kg-content-wide" data-background-color="#000000">
<picture><img class="kg-header-card-image" src="https://example.com/image.jpg" alt="" /></picture>
<div class="kg-header-card-content">
<div class="kg-header-card-text kg-align-center">
<h2 class="kg-header-card-heading" style="color: #FFFFFF;">Title</h2>
<p class="kg-header-card-subheading" style="color: #FFFFFF;">Subtitle</p>
</div>
</div>
</div>`;
const document = createDocument(htmlstring);
const nodes = $generateNodesFromDOM(editor, document);
nodes.length.should.equal(1);
const node = nodes[0];
node.layout.should.equal(''); // full/wide layout is empty string
node.backgroundImageSrc.should.equal('https://example.com/image.jpg');
}));

it('parses split layout when kg-layout-split class is present', editorTest(function () {
const htmlstring = `
<div class="kg-card kg-header-card kg-v2 kg-layout-split kg-width-full">
<div class="kg-header-card-content">
<picture><img class="kg-header-card-image" src="https://example.com/image.jpg" alt="" /></picture>
<div class="kg-header-card-text kg-align-center">
<h2 class="kg-header-card-heading">Title</h2>
<p class="kg-header-card-subheading">Subtitle</p>
</div>
</div>
</div>`;
const document = createDocument(htmlstring);
const nodes = $generateNodesFromDOM(editor, document);
nodes.length.should.equal(1);
const node = nodes[0];
node.layout.should.equal('split');
node.backgroundImageSrc.should.equal('https://example.com/image.jpg');
}));

it('parses full layout when picture is direct child of card', editorTest(function () {
const htmlstring = `
<div class="kg-card kg-header-card kg-v2 kg-width-full" data-background-color="#000000">
<picture><img class="kg-header-card-image" src="https://example.com/image.jpg" alt="" /></picture>
<div class="kg-header-card-content">
<div class="kg-header-card-text kg-align-center">
<h2 class="kg-header-card-heading">Title</h2>
<p class="kg-header-card-subheading">Subtitle</p>
</div>
</div>
</div>`;
const document = createDocument(htmlstring);
const nodes = $generateNodesFromDOM(editor, document);
nodes.length.should.equal(1);
const node = nodes[0];
node.layout.should.equal(''); // full layout is empty string
node.backgroundImageSrc.should.equal('https://example.com/image.jpg');
}));
Comment on lines +404 to +459
Copy link

@coderabbitai coderabbitai bot Mar 16, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Add precedence tests for explicit classes vs DOM cues.

The new cases are good, but they still don’t assert the required precedence rule (“explicit classes win”). Please add conflict scenarios so regressions are caught:

  • kg-content-wide + picture inside .kg-header-card-content → expect layout === ''
  • kg-layout-split + picture as direct child of card → expect layout === 'split'

Without these, class-precedence bugs in packages/kg-default-nodes/lib/nodes/header/parsers/header-parser.js can still pass this suite.

Proposed test additions 
+            it('prefers kg-content-wide over picture-in-content structure', editorTest(function () {
+                const htmlstring = `
+                    <div class="kg-card kg-header-card kg-v2 kg-content-wide">
+                        <div class="kg-header-card-content">
+                            <picture><img class="kg-header-card-image" src="https://example.com/image.jpg" alt="" /></picture>
+                        </div>
+                    </div>`;
+                const document = createDocument(htmlstring);
+                const nodes = $generateNodesFromDOM(editor, document);
+                nodes.length.should.equal(1);
+                const node = nodes[0];
+                node.layout.should.equal('');
+            }));
+
+            it('prefers kg-layout-split over picture-direct-child structure', editorTest(function () {
+                const htmlstring = `
+                    <div class="kg-card kg-header-card kg-v2 kg-layout-split">
+                        <picture><img class="kg-header-card-image" src="https://example.com/image.jpg" alt="" /></picture>
+                        <div class="kg-header-card-content"></div>
+                    </div>`;
+                const document = createDocument(htmlstring);
+                const nodes = $generateNodesFromDOM(editor, document);
+                nodes.length.should.equal(1);
+                const node = nodes[0];
+                node.layout.should.equal('split');
+            }));
📝 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
it('parses full/wide layout when kg-content-wide class is present', editorTest(function () {
const htmlstring = `
<div class="kg-card kg-header-card kg-v2 kg-width-full kg-content-wide" data-background-color="#000000">
<picture><img class="kg-header-card-image" src="https://example.com/image.jpg" alt="" /></picture>
<div class="kg-header-card-content">
<div class="kg-header-card-text kg-align-center">
<h2 class="kg-header-card-heading" style="color: #FFFFFF;">Title</h2>
<p class="kg-header-card-subheading" style="color: #FFFFFF;">Subtitle</p>
</div>
</div>
</div>`;
const document = createDocument(htmlstring);
const nodes = $generateNodesFromDOM(editor, document);
nodes.length.should.equal(1);
const node = nodes[0];
node.layout.should.equal(''); // full/wide layout is empty string
node.backgroundImageSrc.should.equal('https://example.com/image.jpg');
}));
it('parses split layout when kg-layout-split class is present', editorTest(function () {
const htmlstring = `
<div class="kg-card kg-header-card kg-v2 kg-layout-split kg-width-full">
<div class="kg-header-card-content">
<picture><img class="kg-header-card-image" src="https://example.com/image.jpg" alt="" /></picture>
<div class="kg-header-card-text kg-align-center">
<h2 class="kg-header-card-heading">Title</h2>
<p class="kg-header-card-subheading">Subtitle</p>
</div>
</div>
</div>`;
const document = createDocument(htmlstring);
const nodes = $generateNodesFromDOM(editor, document);
nodes.length.should.equal(1);
const node = nodes[0];
node.layout.should.equal('split');
node.backgroundImageSrc.should.equal('https://example.com/image.jpg');
}));
it('parses full layout when picture is direct child of card', editorTest(function () {
const htmlstring = `
<div class="kg-card kg-header-card kg-v2 kg-width-full" data-background-color="#000000">
<picture><img class="kg-header-card-image" src="https://example.com/image.jpg" alt="" /></picture>
<div class="kg-header-card-content">
<div class="kg-header-card-text kg-align-center">
<h2 class="kg-header-card-heading">Title</h2>
<p class="kg-header-card-subheading">Subtitle</p>
</div>
</div>
</div>`;
const document = createDocument(htmlstring);
const nodes = $generateNodesFromDOM(editor, document);
nodes.length.should.equal(1);
const node = nodes[0];
node.layout.should.equal(''); // full layout is empty string
node.backgroundImageSrc.should.equal('https://example.com/image.jpg');
}));
it('parses full/wide layout when kg-content-wide class is present', editorTest(function () {
const htmlstring = `
<div class="kg-card kg-header-card kg-v2 kg-width-full kg-content-wide" data-background-color="#000000">
<picture><img class="kg-header-card-image" src="https://example.com/image.jpg" alt="" /></picture>
<div class="kg-header-card-content">
<div class="kg-header-card-text kg-align-center">
<h2 class="kg-header-card-heading" style="color: `#FFFFFF`;">Title</h2>
<p class="kg-header-card-subheading" style="color: `#FFFFFF`;">Subtitle</p>
</div>
</div>
</div>`;
const document = createDocument(htmlstring);
const nodes = $generateNodesFromDOM(editor, document);
nodes.length.should.equal(1);
const node = nodes[0];
node.layout.should.equal(''); // full/wide layout is empty string
node.backgroundImageSrc.should.equal('https://example.com/image.jpg');
}));
it('parses split layout when kg-layout-split class is present', editorTest(function () {
const htmlstring = `
<div class="kg-card kg-header-card kg-v2 kg-layout-split kg-width-full">
<div class="kg-header-card-content">
<picture><img class="kg-header-card-image" src="https://example.com/image.jpg" alt="" /></picture>
<div class="kg-header-card-text kg-align-center">
<h2 class="kg-header-card-heading">Title</h2>
<p class="kg-header-card-subheading">Subtitle</p>
</div>
</div>
</div>`;
const document = createDocument(htmlstring);
const nodes = $generateNodesFromDOM(editor, document);
nodes.length.should.equal(1);
const node = nodes[0];
node.layout.should.equal('split');
node.backgroundImageSrc.should.equal('https://example.com/image.jpg');
}));
it('parses full layout when picture is direct child of card', editorTest(function () {
const htmlstring = `
<div class="kg-card kg-header-card kg-v2 kg-width-full" data-background-color="#000000">
<picture><img class="kg-header-card-image" src="https://example.com/image.jpg" alt="" /></picture>
<div class="kg-header-card-content">
<div class="kg-header-card-text kg-align-center">
<h2 class="kg-header-card-heading">Title</h2>
<p class="kg-header-card-subheading">Subtitle</p>
</div>
</div>
</div>`;
const document = createDocument(htmlstring);
const nodes = $generateNodesFromDOM(editor, document);
nodes.length.should.equal(1);
const node = nodes[0];
node.layout.should.equal(''); // full layout is empty string
node.backgroundImageSrc.should.equal('https://example.com/image.jpg');
}));
it('prefers kg-content-wide over picture-in-content structure', editorTest(function () {
const htmlstring = `
<div class="kg-card kg-header-card kg-v2 kg-content-wide">
<div class="kg-header-card-content">
<picture><img class="kg-header-card-image" src="https://example.com/image.jpg" alt="" /></picture>
</div>
</div>`;
const document = createDocument(htmlstring);
const nodes = $generateNodesFromDOM(editor, document);
nodes.length.should.equal(1);
const node = nodes[0];
node.layout.should.equal('');
}));
it('prefers kg-layout-split over picture-direct-child structure', editorTest(function () {
const htmlstring = `
<div class="kg-card kg-header-card kg-v2 kg-layout-split">
<picture><img class="kg-header-card-image" src="https://example.com/image.jpg" alt="" /></picture>
<div class="kg-header-card-content"></div>
</div>`;
const document = createDocument(htmlstring);
const nodes = $generateNodesFromDOM(editor, document);
nodes.length.should.equal(1);
const node = nodes[0];
node.layout.should.equal('split');
}));
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@packages/kg-default-nodes/test/nodes/header.test.js` around lines 404 - 459,
Add two precedence tests to header.test.js that assert explicit classes override
DOM cues: (1) a case with class "kg-content-wide" on the card while the
<picture> is nested inside ".kg-header-card-content" and expect node.layout ===
'' and backgroundImageSrc set; (2) a case with class "kg-layout-split" on the
card while the <picture> is a direct child of the card and expect node.layout
=== 'split' and backgroundImageSrc set. These tests should mirror existing test
patterns (using createDocument, $generateNodesFromDOM and editorTest) so they
exercise the parser in header-parser.js and catch class-precedence regressions.
Ensure assertions check nodes.length === 1 and validate node.layout and
node.backgroundImageSrc.

});

describe('getType', function () {
Expand Down