Skip to content

Commit

Permalink
fix(presets): Grid State & Presets stopped working for columns
Browse files Browse the repository at this point in the history
- there was mainly 3 problems
1. ColumnPicker & GridMenu were re-registered but their instances were not overwritten and because of that, the onColumnsChanged in the GridState was never triggering anymore because the subscribe was on the instance that no longer existed
2. we were using the setColumns with allColumns, that was a regression introduced by previous commit, this in terms was cancelling any presets of columns from coming in
3. header menu was not triggering any changes, hiding a column and/or sorting a column had no effect on the Grid State.
- Example 15 is the reference for Grid State & Presets
  • Loading branch information
Ghislain Beaulac authored and Ghislain Beaulac committed Aug 5, 2019
1 parent d9a2325 commit 4e0b528
Show file tree
Hide file tree
Showing 23 changed files with 571 additions and 104 deletions.
16 changes: 8 additions & 8 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@ jobs:
steps:
- checkout
- restore_cache:
key: angular-slickgrid-build-{{ .Branch }}-{{ checksum "package.json" }}
- run: yarn install
key: angular-slickgrid-build-{{ .Branch }}-{{ checksum "yarn.lock" }}
- run: yarn install --frozen-lockfile
- run:
name: Install Jest JUnit coverage reporter
command: yarn add --dev jest-junit
- save_cache:
key: angular-slickgrid-build-{{ .Branch }}-{{ checksum "package.json" }}
key: angular-slickgrid-build-{{ .Branch }}-{{ checksum "yarn.lock" }}
paths:
- "node_modules"
- ~/.cache/yarn
- run:
name: Run Jest tests with JUnit as reporter
command: ./node_modules/.bin/jest --config test/jest.config.js --ci --runInBand --collectCoverage=true --reporters=default --reporters=jest-junit
Expand All @@ -31,17 +31,17 @@ jobs:
- restore_cache:
name: Restoring Cache for Cypress
keys:
- e2e-tests-{{ .Branch }}-{{ checksum "package.json" }}
- e2e-tests-{{ .Branch }}-{{ checksum "yarn.lock" }}
- run:
name: Installing Cypress dependencies with yarn
name: Installing Cypress dependencies
command: |
cd test/cypress
yarn install --frozen-lockfile
- save_cache:
name: Saving Cache for Cypress
key: e2e-tests-{{ .Branch }}-{{ checksum "package.json" }}
key: e2e-tests-{{ .Branch }}-{{ checksum "yarn.lock" }}
paths:
- "test/cypress/node_modules"
- ~/.cache/yarn
- run:
name: Running Cypress E2E tests with JUnit reporter
command: |
Expand Down
2 changes: 2 additions & 0 deletions src/app/examples/grid-menu.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ <h2>{{title}}</h2>
<i class="fa fa-language"></i>
Switch Language
</button>
<b>Locale:</b> <span style="font-style: italic"
data-test="selected-locale">{{selectedLanguage + '.json'}}</span>

<div class="col-sm-12">
<angular-slickgrid gridId="grid9"
Expand Down
9 changes: 6 additions & 3 deletions src/app/examples/grid-menu.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ export class GridMenuComponent implements OnInit {
visibleColumns: Column[];

constructor(private translate: TranslateService) {
this.selectedLanguage = this.translate.getDefaultLang();
// always start with English for Cypress E2E tests to be consistent
const defaultLang = 'en';
this.translate.use(defaultLang);
this.selectedLanguage = defaultLang;
}

ngOnInit(): void {
Expand Down Expand Up @@ -157,8 +160,8 @@ export class GridMenuComponent implements OnInit {
}

switchLanguage() {
this.selectedLanguage = (this.selectedLanguage === 'en') ? 'fr' : 'en';
this.translate.use(this.selectedLanguage);
const nextLocale = (this.selectedLanguage === 'en') ? 'fr' : 'en';
this.translate.use(nextLocale).subscribe(() => this.selectedLanguage = nextLocale);
}

toggleGridMenu(e) {
Expand Down
45 changes: 26 additions & 19 deletions src/app/examples/grid-state.component.html
Original file line number Diff line number Diff line change
@@ -1,22 +1,29 @@
<div class="container-fluid">
<h2>{{title}}</h2>
<div class="subtitle" [innerHTML]="subTitle"></div>
<h2>{{title}}</h2>
<div class="subtitle"
[innerHTML]="subTitle"></div>

<button class="btn btn-default btn-sm" (click)="clearGridStateFromLocalStorage()">
<i class="fa fa-times"></i>
Clear Grid State from Local Storage &amp; Reset Grid
</button>
<button class="btn btn-default btn-sm" (click)="switchLanguage()">
<i class="fa fa-language"></i>
Switch Language
</button>
<button class="btn btn-default btn-sm"
(click)="clearGridStateFromLocalStorage()"
data-test="reset-button">
<i class="fa fa-times"></i>
Clear Grid State from Local Storage &amp; Reset Grid
</button>
<button class="btn btn-default btn-sm"
(click)="switchLanguage()"
data-test="language-button">
<i class="fa fa-language"></i>
Switch Language
</button>
<b>Locale:</b> <span style="font-style: italic"
data-test="selected-locale">{{selectedLanguage + '.json'}}</span>

<angular-slickgrid gridId="grid16"
[columnDefinitions]="columnDefinitions"
[gridOptions]="gridOptions"
[dataset]="dataset"
(onAngularGridCreated)="angularGridReady($event)"
(onGridStateChanged)="gridStateChanged($event)"
(onBeforeGridDestroy)="saveCurrentGridState($event)">
</angular-slickgrid>
</div>
<angular-slickgrid gridId="grid16"
[columnDefinitions]="columnDefinitions"
[gridOptions]="gridOptions"
[dataset]="dataset"
(onAngularGridCreated)="angularGridReady($event)"
(onGridStateChanged)="gridStateChanged($event)"
(onBeforeGridDestroy)="saveCurrentGridState($event)">
</angular-slickgrid>
</div>
21 changes: 15 additions & 6 deletions src/app/examples/grid-state.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,14 @@ export class GridStateComponent implements OnInit {
ngOnInit(): void {
const presets = JSON.parse(localStorage[LOCAL_STORAGE_KEY] || null);

// use some Grid State preset defaults if you wish
// use some Grid State preset defaults if you wish or just restore from Locale Storage
// presets = presets || this.useDefaultPresets();

this.defineGrid(presets);

// always start with English for Cypress E2E tests to be consistent
const defaultLang = 'en';
this.translate.use(defaultLang);
this.selectedLanguage = defaultLang;
}

/** Clear the Grid State from Local Storage and reset the grid to it's original state */
Expand Down Expand Up @@ -100,7 +104,6 @@ export class GridStateComponent implements OnInit {
filter: {
collection: multiSelectFilterArray,
model: Filters.multipleSelect,
searchTerms: [1, 33, 44, 50, 66], // default selection
// we could add certain option(s) to the "multiple-select" plugin
filterOptions: {
maxHeight: 250,
Expand Down Expand Up @@ -141,7 +144,13 @@ export class GridStateComponent implements OnInit {
enableCheckboxSelector: true,
enableFiltering: true,
enableTranslate: true,
i18n: this.translate
i18n: this.translate,
columnPicker: {
hideForceFitButton: true
},
gridMenu: {
hideForceFitButton: true
},
};

// reload the Grid State with the grid options presets
Expand Down Expand Up @@ -196,8 +205,8 @@ export class GridStateComponent implements OnInit {
}

switchLanguage() {
this.selectedLanguage = (this.selectedLanguage === 'en') ? 'fr' : 'en';
this.translate.use(this.selectedLanguage);
const nextLocale = (this.selectedLanguage === 'en') ? 'fr' : 'en';
this.translate.use(nextLocale).subscribe(() => this.selectedLanguage = nextLocale);
}

useDefaultPresets() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn
this.bindBackendCallbackFunctions(this.gridOptions);
}

this.gridStateService.init(this.grid, this.extensionService, this.filterService, this.sortService);
this.gridStateService.init(this.grid);

this.onAngularGridCreated.emit({
// Slick Grid & DataView objects
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const filterServiceStub = {

const sortServiceStub = {
clearSorting: jest.fn(),
emitSortChanged: jest.fn(),
getCurrentColumnSorts: jest.fn(),
onBackendSortChanged: jest.fn(),
onLocalSortChanged: jest.fn(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export class ColumnPickerExtension implements Extension {
this.sharedService.gridOptions.columnPicker.columnTitle = this.sharedService.gridOptions.columnPicker.columnTitle || columnTitle;
this.sharedService.gridOptions.columnPicker.forceFitTitle = this.sharedService.gridOptions.columnPicker.forceFitTitle || forceFitTitle;
this.sharedService.gridOptions.columnPicker.syncResizeTitle = this.sharedService.gridOptions.columnPicker.syncResizeTitle || syncResizeTitle;
this._addon = new Slick.Controls.ColumnPicker(this.sharedService.columnDefinitions, this.sharedService.grid, this.sharedService.gridOptions);
this._addon = new Slick.Controls.ColumnPicker(this.sharedService.allColumns, this.sharedService.grid, this.sharedService.gridOptions);

if (this.sharedService.grid && this.sharedService.gridOptions.enableColumnPicker) {
if (this.sharedService.gridOptions.columnPicker.onExtensionRegistered) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ export class ExtensionUtility {
* @param array input
* @param index
*/
arrayRemoveItemByIndex(array: any[], index: number) {
return array.filter((el: any, i: number) => index !== i);
arrayRemoveItemByIndex<T>(array: T[], index: number): T[] {
return array.filter((el: T, i: number) => index !== i);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ export class GridMenuExtension implements Extension {
this.extensionUtility.translateItems(this.sharedService.gridOptions.gridMenu.customItems, 'titleKey', 'title');
this.extensionUtility.sortItems(this.sharedService.gridOptions.gridMenu.customItems, 'positionOrder');

this._addon = new Slick.Controls.GridMenu(this.sharedService.columnDefinitions, this.sharedService.grid, this.sharedService.gridOptions);
this._addon = new Slick.Controls.GridMenu(this.sharedService.allColumns, this.sharedService.grid, this.sharedService.gridOptions);

// hook all events
if (this.sharedService.grid && this.sharedService.gridOptions.gridMenu) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import { Constants } from '../constants';
import {
Column,
ColumnSort,
CurrentSorter,
EmitterType,
Extension,
ExtensionName,
GridOption,
Expand Down Expand Up @@ -192,10 +194,11 @@ export class HeaderMenuExtension implements Extension {
hideColumn(column: Column) {
if (this.sharedService.grid && this.sharedService.grid.getColumns && this.sharedService.grid.setColumns && this.sharedService.grid.getColumnIndex) {
const columnIndex = this.sharedService.grid.getColumnIndex(column.id);
const currentColumns = this.sharedService.grid.getColumns();
const currentColumns = this.sharedService.grid.getColumns() as Column[];
const visibleColumns = this.extensionUtility.arrayRemoveItemByIndex(currentColumns, columnIndex);
this.sharedService.visibleColumns = visibleColumns;
this.sharedService.grid.setColumns(visibleColumns);
this.sharedService.onColumnsChanged.next(visibleColumns);
}
}

Expand Down Expand Up @@ -333,12 +336,16 @@ export class HeaderMenuExtension implements Extension {
// get previously sorted columns
const sortedColsWithoutCurrent: ColumnSort[] = this.sortService.getCurrentColumnSorts(args.column.id + '');

let emitterType: EmitterType;

// add to the column array, the column sorted by the header menu
sortedColsWithoutCurrent.push({ sortCol: args.column, sortAsc: isSortingAsc });
if (this.sharedService.gridOptions.backendServiceApi) {
this.sortService.onBackendSortChanged(event, { multiColumnSort: true, sortCols: sortedColsWithoutCurrent, grid: this.sharedService.grid });
emitterType = EmitterType.remote;
} else if (this.sharedService.dataView) {
this.sortService.onLocalSortChanged(this.sharedService.grid, this.sharedService.dataView, sortedColsWithoutCurrent);
emitterType = EmitterType.local;
} else {
// when using customDataView, we will simply send it as a onSort event with notify
const isMultiSort = this.sharedService && this.sharedService.gridOptions && this.sharedService.gridOptions.multiColumnSort || false;
Expand All @@ -354,7 +361,23 @@ export class HeaderMenuExtension implements Extension {
sortCol: col && col.sortCol,
};
});
this.sharedService.grid.setSortColumns(newSortColumns); // add sort icon in UI

// add sort icon in UI
this.sharedService.grid.setSortColumns(newSortColumns);

// if we have an emitter type set, we will emit a sort changed
// for the Grid State Service to see the change.
// We also need to pass current sorters changed to the emitSortChanged method
if (emitterType) {
const currentLocalSorters: CurrentSorter[] = [];
newSortColumns.forEach((sortCol) => {
currentLocalSorters.push({
columnId: sortCol.columnId + '',
direction: sortCol.sortAsc ? 'ASC' : 'DESC'
});
});
this.sortService.emitSortChanged(emitterType, currentLocalSorters);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -588,13 +588,17 @@ describe('ExtensionService', () => {
});

it('should re-register the Column Picker when enable and method is called with new column definition collection provided as argument', () => {
const instanceMock = { onColumnsChanged: jest.fn() };
const extensionMock = { name: ExtensionName.columnPicker, addon: null, instance: null, class: null } as ExtensionModel;
const expectedExtension = { name: ExtensionName.columnPicker, addon: instanceMock, instance: instanceMock, class: null } as ExtensionModel;
const gridOptionsMock = { enableColumnPicker: true } as GridOption;
const columnsMock = [
{ id: 'field1', field: 'field1', headerKey: 'HELLO' },
{ id: 'field2', field: 'field2', headerKey: 'WORLD' }
] as Column[];
jest.spyOn(SharedService.prototype, 'gridOptions', 'get').mockReturnValue(gridOptionsMock);
jest.spyOn(SharedService.prototype, 'grid', 'get').mockReturnValue(gridStub);
const spyGetExt = jest.spyOn(service, 'getExtensionByName').mockReturnValue(extensionMock);
const spyCpDispose = jest.spyOn(extensionColumnPickerStub, 'dispose');
const spyCpRegister = jest.spyOn(extensionColumnPickerStub, 'register');
const spyAllCols = jest.spyOn(SharedService.prototype, 'allColumns', 'set');
Expand All @@ -609,20 +613,28 @@ describe('ExtensionService', () => {
});

it('should re-register the Grid Menu when enable and method is called with new column definition collection provided as argument', () => {
const instanceMock = { onColumnsChanged: jest.fn() };
const extensionMock = { name: ExtensionName.gridMenu, addon: null, instance: null, class: null } as ExtensionModel;
const expectedExtension = { name: ExtensionName.gridMenu, addon: instanceMock, instance: instanceMock, class: null } as ExtensionModel;
const gridOptionsMock = { enableGridMenu: true } as GridOption;
const columnsMock = [
{ id: 'field1', field: 'field1', headerKey: 'HELLO' },
{ id: 'field2', field: 'field2', headerKey: 'WORLD' }
] as Column[];
jest.spyOn(SharedService.prototype, 'gridOptions', 'get').mockReturnValue(gridOptionsMock);
jest.spyOn(SharedService.prototype, 'grid', 'get').mockReturnValue(gridStub);
const spyGetExt = jest.spyOn(service, 'getExtensionByName').mockReturnValue(extensionMock);
const spyGmDispose = jest.spyOn(extensionGridMenuStub, 'dispose');
const spyGmRegister = jest.spyOn(extensionGridMenuStub, 'register');
const spyGmRegister = jest.spyOn(extensionGridMenuStub, 'register').mockReturnValue(instanceMock);
const spyAllCols = jest.spyOn(SharedService.prototype, 'allColumns', 'set');
const setColumnsSpy = jest.spyOn(gridStub, 'setColumns');

service.renderColumnHeaders(columnsMock);

expect(expectedExtension).toEqual(expectedExtension);
expect(spyGetExt).toHaveBeenCalled();
expect(expectedExtension).toEqual(expectedExtension);
expect(spyGetExt).toHaveBeenCalled();
expect(spyGmDispose).toHaveBeenCalled();
expect(spyGmRegister).toHaveBeenCalled();
expect(spyAllCols).toHaveBeenCalledWith(columnsMock);
Expand Down
Loading

0 comments on commit 4e0b528

Please sign in to comment.