Skip to content

[WC-3441] DG2: export column enhancements#2287

Open
yordan-st wants to merge 5 commits into
mainfrom
feat/WC-3441_DG2-export-column-enhancements
Open

[WC-3441] DG2: export column enhancements#2287
yordan-st wants to merge 5 commits into
mainfrom
feat/WC-3441_DG2-export-column-enhancements

Conversation

@yordan-st

@yordan-st yordan-st commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Pull request type

Bug fix (non-breaking change which fixes an issue)


Description

When export type is set to "Default" for attribute columns, the exported Excel cells now use the attribute's own formatting (number decimals/grouping, date pattern) instead of exporting raw values without formatting.

For numbers, the exported cell mirrors what the grid shows: Mendix Decimal attributes do not expose a fixed decimal precision at runtime (only whether digits are grouped), so the format uses up to 8 fractional digits with trailing zeros suppressed. This keeps 1234.56 as 1234.56, 0.5 as 0.5, and integers as integers — instead of collapsing to whole numbers.

Also hides the export type and format properties in Studio Pro for dynamic text columns, since they have no effect.

What should be covered while testing?

  1. Attribute column with exportType "Default" and a Decimal attribute → exported cell is a numeric cell that mirrors the grid (e.g. 1234.56 stays 1234.56, 0.5 stays 0.5, integers stay integers; thousands grouping applied when the attribute uses it). Excel format: #,##0.######## (grouped) or 0.######## (ungrouped).
  2. Attribute column with exportType "Default" and a DateTime attribute with custom pattern dd/MM/yyyy → exported cell should have Excel format dd/mm/yyyy
  3. Attribute column with exportType "Number" or "Date" (custom) → still uses the manually specified export format (unchanged behavior)
  4. Dynamic text column → Studio Pro should NOT show export type, export number format, or export date format properties
  5. Custom content columns → unchanged behavior, all export types still work
  6. Existing configurations with no changes → no regression, everything exports as before

@yordan-st yordan-st force-pushed the feat/WC-3441_DG2-export-column-enhancements branch from 6696f2e to 39c37d2 Compare June 22, 2026 16:01
@yordan-st yordan-st marked this pull request as ready for review June 22, 2026 16:11
@yordan-st yordan-st requested a review from a team as a code owner June 22, 2026 16:11
@github-actions

This comment has been minimized.

@yordan-st yordan-st force-pushed the feat/WC-3441_DG2-export-column-enhancements branch from 39c37d2 to 96a5c83 Compare June 23, 2026 12:10
@github-actions

This comment has been minimized.

iobuhov
iobuhov previously approved these changes Jun 23, 2026
@yordan-st yordan-st force-pushed the feat/WC-3441_DG2-export-column-enhancements branch from c94f30c to adc9e02 Compare June 24, 2026 12:51
@github-actions

This comment has been minimized.

@yordan-st yordan-st requested a review from iobuhov June 24, 2026 13:11
@yordan-st yordan-st force-pushed the feat/WC-3441_DG2-export-column-enhancements branch from adc9e02 to 1e6e1fd Compare July 1, 2026 13:43
yordan-st added 3 commits July 1, 2026 17:12
…trailing dot

A static `{base}.########` mask emits the literal decimal point even when
all fractional `#` digits collapse, so whole numbers exported as `1983.`
and broke the export e2e test. Count the fractional digits from the value
itself instead, so integers use `0` (no dot) and decimals mirror the grid.
@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

AI Code Review

⚠️ Approved with suggestions — low-severity items only, safe to merge


What was reviewed

File Change
packages/pluggableWidgets/datagrid-web/CHANGELOG.md Added entries for the new "Default" format behaviour and the Studio Pro visibility fix
packages/pluggableWidgets/datagrid-web/src/features/data-export/cell-readers.ts New countDecimalPlaces helper and getAttributeDefaultFormat function; attribute reader branched on exportType === "default"
packages/pluggableWidgets/datagrid-web/src/features/data-export/__tests__/cell-readers.spec.ts 7 new unit tests covering number/date default-format paths
packages/pluggableWidgets/datagrid-web/src/Datagrid.editorConfig.ts Hides exportType, exportNumberFormat, exportDateFormat for dynamicText columns
packages/pluggableWidgets/datagrid-web/openspec/*.{yaml,md} Design/proposal/tasks artefacts (design-time docs only)

Skipped (out of scope): dist/, pnpm-lock.yaml


Findings

⚠️ Low — countDecimalPlaces is unused outside getAttributeDefaultFormat; also partially duplicates countSignificantDigits

File: packages/pluggableWidgets/datagrid-web/src/features/data-export/cell-readers.ts lines 116–119
Note: Both countSignificantDigits and countDecimalPlaces call value.toFixed() and then do string surgery. The new helper is small and correct, but if Big is already in scope value.e (the exponent) and value.c (the coefficient array) could derive decimal places without a string round-trip. Not a bug — just worth noting if more helpers land here.


⚠️ Low — M → m replacement is global and could misfire on locale tags

File: packages/pluggableWidgets/datagrid-web/src/features/data-export/cell-readers.ts line 130
Note: cfg.pattern.replace(/M/g, "m") converts every M to m. The same replacement exists in getDefaultDateFormat() (line 78) so the approach is intentionally consistent with the rest of the file. However, if a custom pattern ever contains a locale tag like [$-en-US] the uppercase letters inside the tag are also lowercased, which could produce [$-en-us]. Low risk given Mendix's custom datetime patterns in practice, but a tighter regex scoped to outside […] blocks (like hasTimeComponent already does) would be future-proof.


⚠️ Low — Tests cast listAttribute(…) as any to attach .formatter

File: packages/pluggableWidgets/datagrid-web/src/features/data-export/__tests__/cell-readers.spec.ts lines 72, 87, 102, 116, 132, 146, 161
Note: All seven new tests monkey-patch attr.formatter after casting to any. The listAttribute builder from @mendix/widget-plugin-test-utils likely doesn't expose formatter yet. This is pragmatic and works, but if the builder is ever updated to support formatter as a first-class option (e.g. .withFormatter(…)) these tests should migrate. Consider leaving a comment or opening a follow-up ticket so the cast doesn't silently hide a builder-API gap in future.


Positives

  • The countDecimalPlaces-based approach to derive the format from the actual value (rather than a static 0.######## mask) is well-motivated and the edge-case reasoning (trailing dot on whole numbers) is clearly documented in both the comment and the design notes.
  • The exportType === "default" branch is inserted before the existing getCellFormat() call with zero change to the non-default paths — minimal blast radius.
  • The editorConfig change correctly uses hideNestedPropertiesIn (matching the existing pattern for customContent properties) rather than three separate hidePropertyIn calls.
  • CHANGELOG entries are user-facing and behaviour-focused — no implementation details leaked.
  • Test descriptions are specific and cover the key Mendix runtime subtlety (missing decimalPrecision) explicitly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants