Model workbook-defined (custom) table styles#54
Conversation
Add Workbook.tableStyles: custom table/pivot style definitions keyed by style name, following the namedStyles precedent. Each definition carries pivot/table applicability flags and a list of per-region elements with differential Style formatting (the inline-style approach of PivotFormat.style), so no JSF-level dxf table is needed. New types: - TableStyleDefinition: one named custom style - TableStyleElement: formatting of one table region (+ stripe size) - TableStyleElementType: the 28 region kinds (ST_TableStyleType)
Widen the name fields to PivotTableStyleName | (string & {}) (and the
table equivalent) so workbook-defined style names are representable.
The (string & {}) intersection keeps editor autocomplete for the
built-in names while admitting any string. The closed unions remain
exported unchanged, still meaning "a built-in style name".
Names that are neither built-in nor defined in Workbook.tableStyles
are documented to render unstyled, matching an omitted name.
Replace the inline `Name | (string & {})` idiom on the table and pivot
table style `name` properties with a named, documented utility type:
LooseAutocomplete<T extends string> = T | (string & {})
It accepts any string while preserving editor autocomplete for the known
literals in T; the `& {}` on the string arm prevents TypeScript from
collapsing the union to plain `string` (microsoft/TypeScript#29729).
Applied to:
- TableStyle.name?: LooseAutocomplete<TableStyleName> | null
- PivotTableStyle.name?: LooseAutocomplete<PivotTableStyleName> | null
Behaviourally identical to the inline form; this is a clarity rename.
| * without a style has no visual effect (a stripe element may still carry a meaningful | ||
| * {@link size}). | ||
| */ | ||
| style?: Style; |
There was a problem hiding this comment.
We should not be converting differential styles to regular styles. We should be using indexes into a list of differential styles. Dxf's will be shared between at least pivot tables, dynamic styles, and this and we should model them correctly instead of creating legacy (which I fear I have already done by merging and shipping #53).
There was a problem hiding this comment.
OK! Does it make sense for me to try to draft something towards this, or is there no point because it will call on a bunch of latent domain-knowledge that currently exists only in your head and those of a couple of ex-Microsoft retirees? :)
There was a problem hiding this comment.
This is a good question. It appears to me that dxf's are pretty much what we think they are, ie. what is captured in this PR + what we know about the general indexing of styles (named- or cell-). So I think this is pretty much a "do the same as with cell styles but make sure docs & types reflect how dxf's work". I don't have much time right now to think about this because I'm doing something that's on a semi-deadline, but if you don't mind a bit of a delay before I can reasonably catch up with you, then just go for it.
There was a problem hiding this comment.
And ... we already have PivotFormat.style released ... will that need to change to dxfId?: integer?
There was a problem hiding this comment.
Yep. We've never shipped a reader so it might be simplest just to flip to the int and bump a major version.
Of course we are free to pick a simpler prop name: We use s for style, so we can be justified in just d, ds, dxf or whatever we like. Kinda depends on whether we're able to think of anything catcher than .differentialStyles[] for the list. 🤷
There was a problem hiding this comment.
A draft is up in #55, some things still left to hash out there. Went with diffStyles for the list name.
There was a problem hiding this comment.
And stacked on that, a draft of a dxf-based replacement superseding this, up at #56.
Per review: drop the LooseAutocomplete helper (a code-editor-autocomplete convenience to be discussed in its own PR, if at all) and widen the style name fields with plain `string`: - TableStyle.name?: TableStyleName | string | null - PivotTableStyle.name?: PivotTableStyleName | string | null Delete src/types/LooseAutocomplete.ts, its index export, and the now unused imports.
Per review: replace the `pivot?`/`table?` boolean pair (which allows the nonsensical "applies to neither" combination) with a single enum: table?: 'table' | 'pivot' | 'all' // @default "all" ... and drop the TableStyleDefinition docstring's OOXML sentence.
Per review: per-member comments in a union type don't render in the generated docs, so move them into a markdown table in the type's JSDoc (matching the CellValueType / TextVerticalType pattern). Add a Kind column indicating whether each region applies to tables, pivot tables, or both. Also apply the review's reworded "Some regions only occur in tables…" intro.
|
Superseded by #56 to be based on differential styles. |
2 participants
What
Model custom, workbook-defined table styles in JSF, so a table or pivot table declaring a non-built-in style name round-trips.
Workbook.tableStyles?: Record<string, TableStyleDefinition>to model the workbook's<tableStyles>TableStyleElementTypemirrors OOXMLST_TableStyleType; each element has an inline differentialStyle(the dxf overlay).TableStyle.name/PivotTableStyle.nameto accept a custom name, while keeping autocomplete for the built-ins, viaLooseAutocomplete<T> = T | (string & {})(string & {})stays assignable tostring, but stops the union collapsing tostring, so the literals survive; see Literal String Union Autocomplete microsoft/TypeScript#29729 and Open-Ended Unions and Autocomplete With TypeScript.Why
The name unions were closed over the built-in names, so a custom style name was not representable. And
<tableStyles>definitions had no model. This adds the model needed to round-trip and resolve custom table and pivot styles.