-
Notifications
You must be signed in to change notification settings - Fork 160
fix(pivot-grid): Handle currency pivot values with count aggregator - 19.2.x #15428
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
Changes from 3 commits
4e946d9
6cd017b
4c5c837
038a431
a6a744a
8b2455d
4742670
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,6 +47,7 @@ import { IgxIconComponent } from "../../icon/icon.component"; | |
| import { IgxInputGroupComponent } from "../../input-group/input-group.component"; | ||
| import { fadeIn, fadeOut } from 'igniteui-angular/animations'; | ||
| import { Size } from '../common/enums'; | ||
| import { GridColumnDataType } from '../../data-operations/data-util'; | ||
|
|
||
| interface IDataSelectorPanel { | ||
| name: string; | ||
|
|
@@ -517,13 +518,51 @@ export class IgxPivotDataSelectorComponent { | |
| * @internal | ||
| */ | ||
| public onAggregationChange(event: ISelectionEventArgs) { | ||
|
|
||
| if (!this.isSelected(event.newSelection.value)) { | ||
| this.value.aggregate = event.newSelection.value; | ||
|
|
||
| this.handleCountAggregator(); | ||
|
|
||
| this.grid.pipeTrigger++; | ||
| this.grid.cdr.markForCheck(); | ||
| } | ||
| } | ||
|
|
||
| private handleCountAggregator() { | ||
| const valueMember = this.value.member; | ||
| const columns = this.grid.columns; | ||
| const isCountAggregator = this.value.aggregate.key.toLowerCase() === 'count'; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
| const isSingleValue = this.grid.values.length === 1; | ||
| let shouldRemoveFromSet: boolean = false; | ||
|
|
||
| columns.forEach(column => { | ||
| const isRelevantColumn = column.field?.includes(valueMember); | ||
| const isCurrencyColumn = column.dataType === GridColumnDataType.Currency; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't care about the individual columns dataType. Just if the
No need to store and manage column state. |
||
|
|
||
| if (isSingleValue) { | ||
| if (isCountAggregator && isCurrencyColumn) { | ||
| column.dataType = GridColumnDataType.Number; | ||
| this.grid.currencyColumnSet.add(valueMember); | ||
| } else if (this.grid.currencyColumnSet.has(valueMember)) { | ||
| column.dataType = GridColumnDataType.Currency; | ||
| } | ||
| } else if (isRelevantColumn) { | ||
| if (isCountAggregator && isCurrencyColumn) { | ||
| column.dataType = GridColumnDataType.Number; | ||
| this.grid.currencyColumnSet.add(valueMember); | ||
| } else if (this.grid.currencyColumnSet.has(valueMember)) { | ||
| column.dataType = GridColumnDataType.Currency; | ||
| shouldRemoveFromSet = true; | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| if (shouldRemoveFromSet) { | ||
| this.grid.currencyColumnSet.delete(valueMember); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * @hidden | ||
| * @internal | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,6 +38,7 @@ import { IgxDropDirective } from '../../directives/drag-drop/drag-drop.directive | |
| import { NgTemplateOutlet, NgClass, NgStyle } from '@angular/common'; | ||
| import { IgxPivotRowHeaderGroupComponent } from './pivot-row-header-group.component'; | ||
| import { IgxPivotRowDimensionHeaderGroupComponent } from './pivot-row-dimension-header-group.component'; | ||
| import { GridColumnDataType } from '../../data-operations/data-util'; | ||
|
|
||
| /** | ||
| * | ||
|
|
@@ -138,7 +139,7 @@ export class IgxPivotHeaderRowComponent extends IgxGridHeaderRowComponent implem | |
| @Inject(IGX_GRID_BASE) public override grid: PivotGridType, | ||
| ref: ElementRef<HTMLElement>, | ||
| cdr: ChangeDetectorRef, | ||
| protected renderer: Renderer2, | ||
| protected renderer: Renderer2 | ||
| ) { | ||
| super(ref, cdr); | ||
| } | ||
|
|
@@ -408,12 +409,50 @@ export class IgxPivotHeaderRowComponent extends IgxGridHeaderRowComponent implem | |
| * @internal | ||
| */ | ||
| public onAggregationChange(event: ISelectionEventArgs) { | ||
|
|
||
| if (!this.isSelected(event.newSelection.value)) { | ||
| this.value.aggregate = event.newSelection.value; | ||
|
|
||
| this.handleCountAggregator(); | ||
|
|
||
| this.grid.pipeTrigger++; | ||
| } | ||
| } | ||
|
|
||
| private handleCountAggregator() { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's a method with the same name and same code in the pivot selector. Perhaps set it somewhere where it can be re-used. Maybe in the pivot grid component since both should have access to it or in as a common util function? |
||
| const valueMember = this.value.member; | ||
| const columns = this.grid.columns; | ||
| const isCountAggregator = this.value.aggregate.key.toLowerCase() === 'count'; | ||
| const isSingleValue = this.grid.values.length === 1; | ||
| let shouldRemoveFromSet: boolean = false; | ||
|
|
||
| columns.forEach(column => { | ||
| const isRelevantColumn = column.field?.includes(valueMember); | ||
| const isCurrencyColumn = column.dataType === GridColumnDataType.Currency; | ||
|
|
||
| if (isSingleValue) { | ||
| if (isCountAggregator && isCurrencyColumn) { | ||
| column.dataType = GridColumnDataType.Number; | ||
| this.grid.currencyColumnSet.add(valueMember); | ||
| } else if (this.grid.currencyColumnSet.has(valueMember)) { | ||
| column.dataType = GridColumnDataType.Currency; | ||
| } | ||
| } else if (isRelevantColumn) { | ||
| if (isCountAggregator && isCurrencyColumn) { | ||
| column.dataType = GridColumnDataType.Number; | ||
| this.grid.currencyColumnSet.add(valueMember); | ||
| } else if (this.grid.currencyColumnSet.has(valueMember)) { | ||
| column.dataType = GridColumnDataType.Currency; | ||
| shouldRemoveFromSet = true; | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| if (shouldRemoveFromSet) { | ||
| this.grid.currencyColumnSet.delete(valueMember); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * @hidden | ||
| * @internal | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to store the current currency columns? It doesn't seem to matter what their individual
dataTypeis as we need to change them all anyway based on theIPivotValue'sdataTypeand current selected aggregator.pivot value with
dataType= "currency" + selected aggregator with aggregatorName: "COUNT" => All columns associated with it need to setdataType= "number" (doesn't matter what their state was before that).pivot value with
dataType= "currency" + selected aggregator with aggregatorName: "Not COUNT"=> All columns associated with it need to setdataType= "currency" (doesn't matter what their state was before that).Also there's a third numeric data type:
GridColumnDataType.Percentwhich we should also consider. It should also change the column types to number forCount.