-
Notifications
You must be signed in to change notification settings - Fork 77
[WC-3441] DG2: export column enhancements #2287
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
Open
yordan-st
wants to merge
2
commits into
main
Choose a base branch
from
feat/WC-3441_DG2-export-column-enhancements
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2 changes: 2 additions & 0 deletions
2
packages/pluggableWidgets/datagrid-web/openspec/.openspec.yaml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| schema: tdd-refactor | ||
| created: 2026-06-24 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| ## Test Cases | ||
|
|
||
| ### Reproduction Tests | ||
|
|
||
| - Attribute number with default export type uses formatter - (unit) | ||
| - **Given**: Attribute column with `exportType = "default"`, attribute has `formatter = { type: "number", config: { groupDigits: true, decimalPrecision: 2 } }` | ||
| - **When**: Export reads the cell | ||
| - **Then**: Cell is `{ t: "n", v: 1234.56, z: "#,##0.00" }` | ||
|
|
||
| - Attribute date with default export type uses custom pattern - (unit) | ||
| - **Given**: Attribute column with `exportType = "default"`, attribute has `formatter = { type: "datetime", config: { type: "custom", pattern: "dd/MM/yyyy" } }` | ||
| - **When**: Export reads the cell | ||
| - **Then**: Cell is `{ t: "d", v: <stripped-date>, z: "dd/mm/yyyy" }` (M→m converted for Excel) | ||
|
|
||
| ### Edge Cases | ||
|
|
||
| - Attribute date with non-custom datetime config falls back to locale default - (unit) | ||
| - **Given**: Attribute column with `exportType = "default"`, attribute has `formatter = { type: "datetime", config: { type: "date" } }` | ||
| - **When**: Export reads the cell | ||
| - **Then**: Cell has `z: "dd-mm-yyyy"` (locale fallback, not derived from formatter) | ||
|
|
||
| - Attribute number with no decimal precision - (unit) | ||
| - **Given**: Attribute column with `exportType = "default"`, attribute has `formatter = { type: "number", config: { groupDigits: false, decimalPrecision: 0 } }` | ||
| - **When**: Export reads the cell | ||
| - **Then**: Cell has `z: "0"` (no grouping, no decimals) | ||
|
|
||
| - Attribute with formatter lacking type field - (unit) | ||
| - **Given**: Attribute column with `exportType = "default"`, attribute has basic formatter (no `type` property, e.g., default mock) | ||
| - **When**: Export reads the cell | ||
| - **Then**: Cell has `z: undefined` for numbers, locale fallback for dates (graceful degradation) | ||
|
|
||
| ### Regression Tests | ||
|
|
||
| - Custom export type still uses explicit format - (unit) | ||
| - **Given**: Attribute column with `exportType = "number"` and `exportNumberFormat = "#,##0.00"` | ||
| - **When**: Export reads the cell | ||
| - **Then**: Cell uses the explicit format, NOT the attribute formatter | ||
|
|
||
| - Dynamic text always exports as string regardless of export settings - (unit) | ||
| - **Given**: Dynamic text column with any `exportType` set | ||
| - **When**: Export reads the cell | ||
| - **Then**: Cell is always `{ t: "s" }`, export type ignored | ||
|
|
||
| - Custom content export unchanged - (unit) | ||
| - **Given**: Custom content column with `exportType = "number"` and `exportValue = "1234.56"` | ||
| - **When**: Export reads the cell | ||
| - **Then**: Cell is `{ t: "n", v: 1234.56, z: <format> }` (same as before) | ||
|
|
||
| ## Notes | ||
|
|
||
| - The `M → m` conversion is needed because Mendix uses Java-style date patterns (M = month) while Excel uses `m` for month. | ||
| - `getAttributeDefaultFormat` returns `undefined` for string, boolean, and enum attributes — these fall through to existing logic (displayValue for strings, native boolean cells). | ||
| - The `editorConfig` change is Studio Pro-only (design time) — no runtime test needed. |
30 changes: 30 additions & 0 deletions
30
packages/pluggableWidgets/datagrid-web/openspec/proposal.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| ## Why | ||
|
|
||
| When exporting a Data Grid 2 to Excel with `exportType = "default"` on attribute columns, the exported cells had no Excel format applied. Numbers exported as raw values without thousand separators or decimal precision, and dates used only the browser locale fallback. Users expected "default" to mean "use the attribute's own configured format" (e.g., 2 decimal places for a Decimal, `dd/MM/yyyy` for a DateTime). | ||
|
|
||
| Additionally, the export type and format properties were visible in Studio Pro for dynamic text columns even though the export logic ignores them — confusing for configurators. | ||
|
|
||
| ## Root Cause | ||
|
|
||
| `getCellFormat()` returned `undefined` when `exportType === "default"`, meaning no Excel format code (`z` field) was written to the cell. The attribute's `formatter` property (available on `ListAttributeValue` since Mendix 10) was never consulted. | ||
|
|
||
| The `editorConfig.ts` visibility logic only conditionally hid `exportNumberFormat`/`exportDateFormat` based on the export type value, but never hid all three export properties when `showContentAs === "dynamicText"`. | ||
|
|
||
| ## What Changes | ||
|
|
||
| Package: `packages/pluggableWidgets/datagrid-web` | ||
|
|
||
| - `src/features/data-export/cell-readers.ts` — New `getAttributeDefaultFormat(props)` function reads `props.attribute.formatter` and derives an Excel format string: | ||
| - `formatter.type === "number"` → builds format from `groupDigits` and `decimalPrecision` (e.g., `#,##0.00`) | ||
| - `formatter.type === "datetime"` with `config.type === "custom"` → converts Mendix pattern to Excel format (M→m replacement) | ||
| - Otherwise returns `undefined` (falls through to existing locale default for dates) | ||
| - Attribute reader now branches: `exportType === "default"` → `getAttributeDefaultFormat()`, else → existing `getCellFormat()`. | ||
|
|
||
| - `src/Datagrid.editorConfig.ts` — When `showContentAs === "dynamicText"`, hide `exportType`, `exportNumberFormat`, and `exportDateFormat` properties in Studio Pro. | ||
|
|
||
| ## Impact | ||
|
|
||
| - Attribute columns with `exportType = "default"` now export with their configured format. This is an enhancement, not a breaking change — previously these cells had no format, now they have one. | ||
| - No XML property changes. No migration needed. | ||
| - No behavioral change for `exportType = "number"` / `"date"` / `"boolean"` (custom path unchanged). | ||
| - Dynamic text columns: cosmetic-only change in Studio Pro property panel (properties hidden). Runtime behavior identical. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| ## 1. Test Setup | ||
|
|
||
| - [x] 1.1 Write test: attribute number with default exportType uses formatter config to derive Excel format | ||
| - [x] 1.2 Write test: attribute date with default exportType and custom pattern uses converted pattern | ||
| - [x] 1.3 Write test: attribute date with non-custom config falls back to locale default | ||
| - [x] 1.4 Write test: attribute number with groupDigits=false and decimalPrecision=0 produces format "0" | ||
|
|
||
| ## 2. Implementation | ||
|
|
||
| - [x] 2.1 Add `getAttributeDefaultFormat(props)` function in `cell-readers.ts` that reads `props.attribute.formatter` and derives Excel format string | ||
| - [x] 2.2 Branch attribute reader: `exportType === "default"` calls `getAttributeDefaultFormat()`, else calls existing `getCellFormat()` | ||
| - [x] 2.3 Hide `exportType`, `exportNumberFormat`, `exportDateFormat` in editorConfig when `showContentAs === "dynamicText"` | ||
|
|
||
| ## 3. Refactoring | ||
|
|
||
| - [x] 3.1 No refactoring needed — implementation is minimal and isolated | ||
|
|
||
| ## 4. Verification | ||
|
|
||
| - [x] 4.1 All 221 tests passing (4 new + 217 existing) | ||
| - [x] 4.2 Full test suite passes (no regressions) | ||
| - [x] 4.3 Build compiles without TypeScript errors | ||
| - [x] 4.4 PR submitted and reviewed | ||
|
|
||
| ## Notes | ||
|
|
||
| - No XML property changes were needed — existing `exportType` enum with "default" value covers the use case. | ||
| - The `formatter` property is available on `ListAttributeValue` since Mendix 10 runtime. | ||
| - iobuhov review feedback: use consistent assertion syntax (assert `cell.v` in all date tests). Fixed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.