Fix(table): Support 'scope' attribute in HTML import.#8094
Fix(table): Support 'scope' attribute in HTML import.#8094etrepum merged 3 commits intofacebook:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| import { | ||
| $convertTableCellNodeElement, | ||
| TableCellNode, | ||
| } from '../../LexicalTableCellNode'; | ||
|
|
There was a problem hiding this comment.
It would be better to test the public interface here rather than import these implementation details
There was a problem hiding this comment.
Good point. I've refactored the tests to use TableCellNode.importDOM() instead of importing the internal helper function directly. Checks passed!
41ab693 to
ed91fb5
Compare
etrepum
left a comment
There was a problem hiding this comment.
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, |
There was a problem hiding this comment.
I don't know why you're doing this cast?
| (node as TableCellNode & {__headerState: number}).__headerState, | |
| node.getHeaderState(), |
| const node = result!.node as TableCellNode; | ||
|
|
||
| expect( | ||
| (node as TableCellNode & {__headerState: number}).__headerState, |
There was a problem hiding this comment.
| (node as TableCellNode & {__headerState: number}).__headerState, | |
| node.getHeaderState(), |
| const node = result!.node; | ||
|
|
||
| expect( | ||
| (node as TableCellNode & {__headerState: number}).__headerState, |
There was a problem hiding this comment.
| (node as TableCellNode & {__headerState: number}).__headerState, | |
| node.getHeaderState(), |
|
|
||
| const result = convertHTMLTag(th); | ||
|
|
||
| const node = result!.node as TableCellNode; |
There was a problem hiding this comment.
| 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; |
There was a problem hiding this comment.
| 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; |
There was a problem hiding this comment.
| const node = result!.node; | |
| const node = result!.node; | |
| assert($isTableCellNode(node), 'Expected TableCellNode'); |
…tyles() for strict assertions
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.