Skip to content

Fix(table): Support 'scope' attribute in HTML import.#8094

Merged
etrepum merged 3 commits intofacebook:mainfrom
Sa-Te:fix/table-header-scope
Feb 5, 2026
Merged

Fix(table): Support 'scope' attribute in HTML import.#8094
etrepum merged 3 commits intofacebook:mainfrom
Sa-Te:fix/table-header-scope

Conversation

@Sa-Te
Copy link
Copy Markdown
Contributor

@Sa-Te Sa-Te commented Jan 26, 2026

Currently, the table cell importer assumes all <th> elements are Row Headers. This ignores the scope attribute if present (e.g., <th scope="col">), leading to incorrect header states for semantic tables.

Fix: Updated $convertTableCellNodeElement to check for scope="col" and scope="row". It defaults to Row Header if scope is missing, preserving existing fallback behavior while enabling high-fidelity imports for semantic HTML.

Tests: Added unit tests in LexicalTableCellNode.test.ts to verify scope="col", scope="row", and missing scope scenarios.

@vercel
Copy link
Copy Markdown

vercel bot commented Jan 26, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
lexical Ready Ready Preview, Comment Feb 4, 2026 11:41pm
lexical-playground Ready Ready Preview, Comment Feb 4, 2026 11:41pm

Request Review

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jan 26, 2026
@etrepum etrepum added the extended-tests Run extended e2e tests on a PR label Jan 30, 2026
Comment on lines +19 to +23
import {
$convertTableCellNodeElement,
TableCellNode,
} from '../../LexicalTableCellNode';

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It would be better to test the public interface here rather than import these implementation details

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point. I've refactored the tests to use TableCellNode.importDOM() instead of importing the internal helper function directly. Checks passed!

Copy link
Copy Markdown
Collaborator

@etrepum etrepum left a comment

Choose a reason for hiding this comment

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

the tests seem a bit weird, were these types generated by an llm or something? __headerState is even technically part of the public interface, /** @internal */ just means it's not documented.

const node = result!.node as TableCellNode;

expect(
(node as TableCellNode & {__headerState: number}).__headerState,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't know why you're doing this cast?

Suggested change
(node as TableCellNode & {__headerState: number}).__headerState,
node.getHeaderState(),

const node = result!.node as TableCellNode;

expect(
(node as TableCellNode & {__headerState: number}).__headerState,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
(node as TableCellNode & {__headerState: number}).__headerState,
node.getHeaderState(),

const node = result!.node;

expect(
(node as TableCellNode & {__headerState: number}).__headerState,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
(node as TableCellNode & {__headerState: number}).__headerState,
node.getHeaderState(),


const result = convertHTMLTag(th);

const node = result!.node as TableCellNode;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const node = result!.node as TableCellNode;
const node = result!.node;
assert($isTableCellNode(node), 'Expected TableCellNode');


const result = convertHTMLTag(th);

const node = result!.node as TableCellNode;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const node = result!.node as TableCellNode;
const node = result!.node;
assert($isTableCellNode(node), 'Expected TableCellNode');

// No scope attribute set
const result = convertHTMLTag(th);

const node = result!.node;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const node = result!.node;
const node = result!.node;
assert($isTableCellNode(node), 'Expected TableCellNode');

@etrepum etrepum enabled auto-merge February 5, 2026 00:29
@etrepum etrepum added this pull request to the merge queue Feb 5, 2026
Merged via the queue into facebook:main with commit dcb4c95 Feb 5, 2026
42 checks passed
rayterion pushed a commit to rayterion/lexical that referenced this pull request Feb 8, 2026
@etrepum etrepum mentioned this pull request Feb 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. extended-tests Run extended e2e tests on a PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants