(feat) Add column info to section properties #9
Conversation
luiisgallego
left a comment
luiisgallego
left a comment
There was a problem hiding this comment.
In general it looks good, but the code could be improved a bit more. Also, I think there are cases not covered yet, but I am not entirely sure. Added Ines as reviewer, since she did the TD, so she can help with the next revisions.
src/properties/section-properties.ts
Outdated
| numberOfColumns: null | number; | ||
| equalWidth: null | boolean; | ||
| separator: null | boolean; | ||
| columns?: {columnWidth: Length | null, columnSpace?: Length | null}[] | null; |
There was a problem hiding this comment.
Maybe "definition" or something like that in order to avoid repeating "columns" twice.
There was a problem hiding this comment.
Apart from that, in which case does the "columns" property not exist?
There was a problem hiding this comment.
Columns are missing when all columns have equal size (equalWidth is true). What I don't understand is why some properties are optional (?), and some other aren't, but at the same time those can be null. Are those optional? They aren't...?
src/properties/section-properties.ts
Outdated
| if (!node) { | ||
| return {}; | ||
| } | ||
| }; |
There was a problem hiding this comment.
This seems to be necessary because the createXmlRoundRobinTest function used in the test suite expects null to be a possibility. Not a great reason, but it is the reason.
As for why it's that way, it seems like we were trying to cover the possibility that certain elements might not be there as we create a document. If this is something we need to dive into more, let's open a new issue for it.
src/properties/section-properties.ts
Outdated
| "equalWidth": docxml:st-on-off(./${QNS.w}cols/@${QNS.w}equalwidth), | ||
| "columns": array{ | ||
| ./${QNS.w}cols/${QNS.w}col/map{ | ||
| "columnWidth": if (@${QNS.w}w) then docxml:length(@${QNS.w}w, 'twip') else (), |
src/properties/section-properties.ts
Outdated
| export function sectionPropertiesToNode(data: SectionProperties = {}): Node { | ||
| return create( | ||
|
|
||
| // console.log(data.columns?.columns?.forEach((col) => { console.log(col.columnWidth)})) |
| } | ||
| ); | ||
| }); | ||
|
|
| equalWidth: false, | ||
| separator: true, | ||
| columns: [ | ||
| {columnWidth: twip(1440), columnSpace: twip(720)}, |
There was a problem hiding this comment.
Prettier does not work in this repo. Fix it in this PR, or create another one and then fix this indentation.
| attribute ${QNS.w}equalwidth { $columns('equalWidth') }, | ||
| attribute ${QNS.w}num { $columns('numberOfColumns') }, | ||
| if (docxml:st-on-off(string($columns('equalWidth')))) then () |
There was a problem hiding this comment.
When there is no "columns" information, does the "equalWidth" attribute always exist?
There was a problem hiding this comment.
Could you add more tests forcing incorrect scenarios? Maybe some missing properties? Extra/unknown properties?
src/properties/section-properties.ts
Outdated
| numberOfColumns: null | number; | ||
| equalWidth: null | boolean; | ||
| separator: null | boolean; | ||
| columns?: {columnWidth: Length | null, columnSpace?: Length | null}[] | null; |
There was a problem hiding this comment.
Columns are missing when all columns have equal size (equalWidth is true). What I don't understand is why some properties are optional (?), and some other aren't, but at the same time those can be null. Are those optional? They aren't...?
src/properties/section-properties.ts
Outdated
| equalWidth: null | boolean; | ||
| separator: null | boolean; | ||
| columns?: {columnWidth: Length | null, columnSpace?: Length | null}[] | null; | ||
| }; |
harvestcore
changed the title
| @@ -1,8 +1,7 @@ | |||
| import { describe, run } from 'https://deno.land/x/[email protected]/mod.ts'; | |||
harvestcore
force-pushed
the
export-column-info
branch
from
850073d to
44b4825
Compare
This PR adds basic support for the section properties necessary to create a document with columns. The
SectionPropertiestype should now handle Word documents with<w:cols>elements, and any subsequent<w:col>elements for individually specified columns.