fix(pivot-grid): fix date format based on the localization#17256
fix(pivot-grid): fix date format based on the localization#17256
Conversation
Hristo313
added
❌ status: awaiting-test
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds locale-aware formatting for Pivot Grid date dimension headers by introducing a dimension-level formatter and applying it when extracting row dimension headers.
Changes:
- Add
formattertoIPivotDimensionto control display text without affecting grouping keys. - Apply dimension
formatterwhen building row header columns. - Update date dimension behavior/tests to use locale-aware short date formatting.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| projects/igniteui-angular/grids/pivot-grid/src/pivot-row-dimension-content.component.ts | Applies the new dimension formatter when extracting header text. |
| projects/igniteui-angular/grids/pivot-grid/src/pivot-grid.spec.ts | Updates expected row header date to a locale-derived short date string. |
| projects/igniteui-angular/grids/core/src/pivot-grid.interface.ts | Introduces IPivotDimension.formatter API with documentation. |
| projects/igniteui-angular/grids/core/src/pivot-grid-dimensions.ts | Adds locale-aware formatting for full-date leaf values when no custom memberFunction is provided. |
| if (options.fullDate) { | ||
| if (inBaseDimension.memberFunction) { | ||
| // User supplied a custom memberFunction — preserve it without adding a formatter. | ||
| baseDimension = inBaseDimension; | ||
| } else { | ||
| // No custom memberFunction: create a new dimension object with a locale-aware | ||
| // formatter that shows dates in short-date format. | ||
| baseDimension = { | ||
| ...inBaseDimension, | ||
| formatter: (value: any) => { | ||
| const dateValue = (value !== null && value !== undefined && value !== '') | ||
| ? getDateFormatter().createDateFromValue(value) | ||
| : null; | ||
| return dateValue | ||
| ? getDateFormatter().formatDateTime(dateValue, undefined, { dateStyle: 'short' }) | ||
| : value; | ||
| } | ||
| }; | ||
| } | ||
| } |
There was a problem hiding this comment.
New branching behavior is introduced for options.fullDate based on whether inBaseDimension.memberFunction is provided. The updated spec covers locale formatting, but it doesn’t appear to cover the memberFunction branch (ensuring no formatter is attached/used). Add a unit test that sets fullDate: true with a custom memberFunction and asserts header rendering behaves as intended for that path.
| const formattedDate = Intl.DateTimeFormat(undefined, { dateStyle: 'short' }).format(new Date(2021, 11, 8)); | ||
| const expectedHeaders = ['All Periods', '2021', 'Q4', 'December', formattedDate]; |
There was a problem hiding this comment.
The production code formats via getDateFormatter().formatDateTime(..., { dateStyle: 'short' }), while the test computes expectations using Intl.DateTimeFormat(...). These can differ across environments/locales/formatting backends even when both are 'short'. To avoid CI flakiness, compute formattedDate using the same date formatting utility used by the component (or explicitly set/override the test locale to a known value and assert that exact string).
skrustev
left a comment
skrustev
left a comment
There was a problem hiding this comment.
Please provide more context to the formatter, like dimension and row data.
| */ | ||
| /* csTreatAsEvent: PivotDimensionFormatterEventHandler */ | ||
| /* blazorOnlyScript */ | ||
| formatter?: (value: any) => string | null | undefined; |
There was a problem hiding this comment.
Also naming it just formatter is kinda confusing. This is not a formatter for the dimension cell. I think it should be named something like headerFormatter/columnFormatter.
There was a problem hiding this comment.
Done in bf31407: renamed the new dimension display hook from formatter to headerFormatter to make the scope explicit for dimension headers.
Agent-Logs-Url: https://github.com/IgniteUI/igniteui-angular/sessions/2b06b154-22a8-4853-bdc6-e286e0fda6f5 Co-authored-by: Hristo313 <57346540+Hristo313@users.noreply.github.com>
Agent-Logs-Url: https://github.com/IgniteUI/igniteui-angular/sessions/2b06b154-22a8-4853-bdc6-e286e0fda6f5 Co-authored-by: Hristo313 <57346540+Hristo313@users.noreply.github.com>
Implemented in bf31407 (and aligned in 039f036): dimension header formatting now receives additional context ( |
|
@copilot Fix the failing tests |
Agent-Logs-Url: https://github.com/IgniteUI/igniteui-angular/sessions/e6bad7cc-ebb9-4035-8e68-c65ced5691af Co-authored-by: Hristo313 <57346540+Hristo313@users.noreply.github.com>
Fixed in 7595db4. The failing CI test was caused by strict object identity checks in the new pivot formatter-context spec; I replaced them with stable structural assertions ( |
Closes #16442
Description
Adds a display-only formatter hook to IPivotDimension and uses it to localize the leaf-level fullDate values produced by IgxPivotDateDimension, so both row and column dimension headers show locale-aware short dates instead of raw ISO strings.
Motivation / Context
Type of Change (check all that apply):
Component(s) / Area(s) Affected:
How Has This Been Tested?
Test Configuration:
Screenshots / Recordings
Checklist:
feature/README.MDupdates for the feature docsREADME.MDCHANGELOG.MDupdates for newly added functionalityng updatemigrations for the breaking changes (migrations guidelines)