-
-
Notifications
You must be signed in to change notification settings - Fork 462
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.
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
Fixes #877: table columns defaulting to small/unusable sizes in some word processors #943
Conversation
…n some word processors
What happens if no column width is provided? Does the document still look good? |
Hm, from what I can tell, in older versions of the standard it was a required element, but in the newer versions it's optional. So this may cause an issue with word 2007 or older -- worth a test there first. |
Looking at the ooxml wml.xsd, it looks like it's required; so probably shouldn't remove it. <xsd:complexType name="CT_Tbl">
<xsd:sequence>
<xsd:group ref="EG_RangeMarkupElements" minOccurs="0" maxOccurs="unbounded"/>
<xsd:element name="tblPr" type="CT_TblPr"/>
<xsd:element name="tblGrid" type="CT_TblGrid"/>
<xsd:group ref="EG_ContentRowContent" minOccurs="0" maxOccurs="unbounded"/>
</xsd:sequence>
</xsd:complexType> |
Also it says in the accompanying spec,
So the spec says one thing, but the schema says another. Fun. The schema is what's used by tooling though, so should probably stick to that. |
@@ -85,7 +85,9 @@ export class Table extends XmlComponent { | |||
}), | |||
); | |||
|
|||
this.root.push(new TableGrid(columnWidths)); | |||
if (columnWidths) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a test here?
And a test without columnWidths?
As of now, I don't think this is a good approach because in the xsd schema, it appears |
@devoidfury Ok, if so, can this be closed? |
Fixes #877 - don't add
w:tblGrid
to table whencolumnWidths
argument isn't provided. Fixed the Table tests to account for this, and added one additional test withcolumnWidths
set to make sure it's added in that case.