Skip to content

Commit

Permalink
fix(resizer): remove delay to call resize by content to avoid flickering
Browse files Browse the repository at this point in the history 
- loading of Columns Grid Presets should only be done once (before resizing) and loading Filters Grid Presets could be done one or more times, so to fix this I have separated `loadPresetsWhenDatasetInitialized` into 2 new methods and made sure to call the columns grid preset only once.
- this issue was identified while porting the previous PR #341 into Angular-Slickgrid
  • Loading branch information
@ghiscoding
ghiscoding committed May 17, 2021
1 parent 3967f02 commit ea4a196
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 41 deletions.
14 changes: 4 additions & 10 deletions src/app/modules/angular-slickgrid/components/__tests__/angular-slickgrid.component.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ describe('App Component', () => {
});

describe('resizeColumnsByCellContent method', () => {
it('should call "resizeColumnsByCellContent" when the DataView "onSetItemsCalled" event is triggered and "enableAutoResizeColumnsByCellContent" is set', (done) => {
it('should call "resizeColumnsByCellContent" when the DataView "onSetItemsCalled" event is triggered and "enableAutoResizeColumnsByCellContent" is set', () => {
const resizeContentSpy = jest.spyOn(resizerServiceStub, 'resizeColumnsByCellContent');
const mockDataset = [{ id: 1, firstName: 'John' }, { id: 2, firstName: 'Jane' }];

Expand All @@ -318,13 +318,10 @@ describe('App Component', () => {
fixture.detectChanges();
component.dataView.onSetItemsCalled.notify({ idProperty: 'id', itemCount: 1 });

setTimeout(() => {
expect(resizeContentSpy).toHaveBeenCalledWith(true);
done();
}, 10);
expect(resizeContentSpy).toHaveBeenCalledWith(true);
});

it('should call "resizeColumnsByCellContent" when the DataView "onSetItemsCalled" event is triggered and "enableAutoResizeColumnsByCellContent" is set', (done) => {
it('should call "resizeColumnsByCellContent" when the DataView "onSetItemsCalled" event is triggered and "enableAutoResizeColumnsByCellContent" is set', () => {
const resizeContentSpy = jest.spyOn(resizerServiceStub, 'resizeColumnsByCellContent');
const mockDataset = [{ id: 1, firstName: 'John' }, { id: 2, firstName: 'Jane' }];

Expand All @@ -334,10 +331,7 @@ describe('App Component', () => {
fixture.detectChanges();
component.dataView.onSetItemsCalled.notify({ idProperty: 'id', itemCount: 1 });

setTimeout(() => {
expect(resizeContentSpy).toHaveBeenCalledWith(false);
done();
}, 10);
expect(resizeContentSpy).toHaveBeenCalledWith(false);
});

describe('Custom Footer', () => {
Expand Down
62 changes: 34 additions & 28 deletions src/app/modules/angular-slickgrid/components/angular-slickgrid.component.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn

if (dataset.length > 0) {
if (!this._isDatasetInitialized) {
this.loadPresetsWhenDatasetInitialized();
this.loadFilterPresetsWhenDatasetInitialized();

if (this.gridOptions.enableCheckboxSelector) {
this.loadRowSelectionPresetWhenExists();
Expand Down Expand Up @@ -602,7 +602,8 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn
}

// load any presets if any (after dataset is initialized)
this.loadPresetsWhenDatasetInitialized();
this.loadColumnPresetsWhenDatasetInitialized();
this.loadFilterPresetsWhenDatasetInitialized();
}


Expand Down Expand Up @@ -657,9 +658,7 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn

// when user has resize by content enabled, we'll force a full width calculation since we change our entire dataset
if (args.itemCount > 0 && (this.gridOptions.autosizeColumnsByCellContentOnFirstLoad || this.gridOptions.enableAutoResizeColumnsByCellContent)) {
// add a delay so that if column positions changes by changeColumnsArrangement() when using custom Grid Views
// or presets.columns won't have any impact on the list of visible columns and their positions
setTimeout(() => this.resizer.resizeColumnsByCellContent(!this.gridOptions?.resizeByContentOnlyOnFirstLoad), 10);
this.resizer.resizeColumnsByCellContent(!this.gridOptions?.resizeByContentOnlyOnFirstLoad);
}
});

Expand Down Expand Up @@ -883,6 +882,9 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn
this.sharedService.dataView = this.dataView;
this.sharedService.grid = this.grid;

// load the resizer service
this.resizer.init(this.grid);

this.extensionService.bindDifferentExtensions();
this.bindDifferentHooks(this.grid, this.gridOptions, this.dataView);

Expand Down Expand Up @@ -939,7 +941,7 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn
if (!this._isDatasetInitialized && (this.gridOptions.enableCheckboxSelector || this.gridOptions.enableRowSelection)) {
this.loadRowSelectionPresetWhenExists();
}
this.loadPresetsWhenDatasetInitialized();
this.loadFilterPresetsWhenDatasetInitialized();
this._isDatasetInitialized = true;
}
}
Expand Down Expand Up @@ -1044,33 +1046,37 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn
}
}

private loadPresetsWhenDatasetInitialized() {
/** Load any possible Columns Grid Presets */
private loadColumnPresetsWhenDatasetInitialized() {
// if user entered some Columns "presets", we need to reflect them all in the grid
if (this.gridOptions.presets && Array.isArray(this.gridOptions.presets.columns) && this.gridOptions.presets.columns.length > 0) {
const gridColumns: Column[] = this.gridStateService.getAssociatedGridColumns(this.grid, this.gridOptions.presets.columns);
if (gridColumns && Array.isArray(gridColumns) && gridColumns.length > 0) {
// make sure that the checkbox selector is also visible if it is enabled
if (this.gridOptions.enableCheckboxSelector) {
const checkboxColumn = (Array.isArray(this._columnDefinitions) && this._columnDefinitions.length > 0) ? this._columnDefinitions[0] : null;
if (checkboxColumn && checkboxColumn.id === '_checkbox_selector' && gridColumns[0].id !== '_checkbox_selector') {
gridColumns.unshift(checkboxColumn);
}
}

// keep copy the original optional `width` properties optionally provided by the user.
// We will use this when doing a resize by cell content, if user provided a `width` it won't override it.
gridColumns.forEach(col => col.originalWidth = col.width);

// finally set the new presets columns (including checkbox selector if need be)
this.grid.setColumns(gridColumns);
}
}
}

/** Load any possible Filters Grid Presets */
private loadFilterPresetsWhenDatasetInitialized() {
if (this.gridOptions && !this.customDataView) {
// if user entered some Filter "presets", we need to reflect them all in the DOM
if (this.gridOptions.presets && Array.isArray(this.gridOptions.presets.filters)) {
this.filterService.populateColumnFilterSearchTermPresets(this.gridOptions.presets.filters);
}

// if user entered some Columns "presets", we need to reflect them all in the grid
if (this.gridOptions.presets && Array.isArray(this.gridOptions.presets.columns) && this.gridOptions.presets.columns.length > 0) {
const gridColumns: Column[] = this.gridStateService.getAssociatedGridColumns(this.grid, this.gridOptions.presets.columns);
if (gridColumns && Array.isArray(gridColumns) && gridColumns.length > 0) {
// make sure that the checkbox selector is also visible if it is enabled
if (this.gridOptions.enableCheckboxSelector) {
const checkboxColumn = (Array.isArray(this._columnDefinitions) && this._columnDefinitions.length > 0) ? this._columnDefinitions[0] : null;
if (checkboxColumn && checkboxColumn.id === '_checkbox_selector' && gridColumns[0].id !== '_checkbox_selector') {
gridColumns.unshift(checkboxColumn);
}
}

// keep copy the original optional `width` properties optionally provided by the user.
// We will use this when doing a resize by cell content, if user provided a `width` it won't override it.
gridColumns.forEach(col => col.originalWidth = col.width);

// finally set the new presets columns (including checkbox selector if need be)
this.grid.setColumns(gridColumns);
}
}
}
}

Expand Down
41 changes: 40 additions & 1 deletion src/app/modules/angular-slickgrid/services/__tests__/gridState.service.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { Subject } from 'rxjs';
import { ExtensionService } from '../extension.service';
import { FilterService } from '../filter.service';
import { GridStateService } from '../gridState.service';
import { ResizerService } from '../resizer.service';
import { SharedService } from '../shared.service';
import { SortService } from '../sort.service';
import {
Expand Down Expand Up @@ -59,6 +60,15 @@ const gridStub = {
onSelectedRowsChanged: new Slick.Event(),
};

const resizerServiceStub = {
init: jest.fn(),
dispose: jest.fn(),
bindAutoResizeDataGrid: jest.fn(),
resizeGrid: jest.fn(),
resizeColumnsByCellContent: jest.fn(),
} as unknown as ResizerService;


const extensionServiceStub = {
getExtensionByName: (name: string) => { }
} as ExtensionService;
Expand All @@ -79,7 +89,7 @@ describe('GridStateService', () => {

beforeEach(() => {
sharedService = new SharedService();
service = new GridStateService(extensionServiceStub, filterServiceStub, sharedService, sortServiceStub);
service = new GridStateService(extensionServiceStub, filterServiceStub, resizerServiceStub, sharedService, sortServiceStub);
service.init(gridStub, dataViewStub);
jest.spyOn(gridStub, 'getSelectionModel').mockReturnValue(true);
});
Expand Down Expand Up @@ -174,11 +184,40 @@ describe('GridStateService', () => {
jest.spyOn(SharedService.prototype, 'allColumns', 'get').mockReturnValue(allColumnsMock);
const setColsSpy = jest.spyOn(gridStub, 'setColumns');
const autoSizeSpy = jest.spyOn(gridStub, 'autosizeColumns');
const resizeByContentSpy = jest.spyOn(resizerServiceStub, 'resizeColumnsByCellContent');

service.changeColumnsArrangement(presetColumnsMock);

expect(setColsSpy).toHaveBeenCalledWith([rowCheckboxColumnMock, ...columnsWithoutCheckboxMock]);
expect(autoSizeSpy).toHaveBeenCalled();
expect(resizeByContentSpy).not.toHaveBeenCalled();
});

it('should call the method and expect slickgrid "setColumns" and a pubsub event "onFullResizeByContentRequested" to be called with newest columns when "triggerAutoSizeColumns" is false and "enableAutoResizeColumnsByCellContent" is true', () => {
gridOptionMock.enableAutoResizeColumnsByCellContent = true;
jest.spyOn(SharedService.prototype, 'allColumns', 'get').mockReturnValue(allColumnsMock);
const setColsSpy = jest.spyOn(gridStub, 'setColumns');
const autoSizeSpy = jest.spyOn(gridStub, 'autosizeColumns');
const resizeByContentSpy = jest.spyOn(resizerServiceStub, 'resizeColumnsByCellContent');

service.changeColumnsArrangement(presetColumnsMock, false);

expect(setColsSpy).toHaveBeenCalledWith([rowCheckboxColumnMock, ...columnsWithoutCheckboxMock]);
expect(autoSizeSpy).not.toHaveBeenCalled();
expect(resizeByContentSpy).toHaveBeenCalledWith(true);
});

it('should call the method and expect slickgrid "setColumns" and a pubsub event "onFullResizeByContentRequested" to be called with newest columns when "triggerAutoSizeColumns" is false and "enableAutoResizeColumnsByCellContent" is true', () => {
jest.spyOn(SharedService.prototype, 'allColumns', 'get').mockReturnValue(allColumnsMock);
const setColsSpy = jest.spyOn(gridStub, 'setColumns');
const autoSizeSpy = jest.spyOn(gridStub, 'autosizeColumns');
const resizeByContentSpy = jest.spyOn(resizerServiceStub, 'resizeColumnsByCellContent');

service.changeColumnsArrangement(presetColumnsMock, false, true);

expect(setColsSpy).toHaveBeenCalledWith([rowCheckboxColumnMock, ...columnsWithoutCheckboxMock]);
expect(autoSizeSpy).not.toHaveBeenCalled();
expect(resizeByContentSpy).toHaveBeenCalledWith(true);
});

it('should call the method and expect only 1 method of slickgrid "setColumns" to be called when we define 2nd argument (triggerAutoSizeColumns) as False', () => {
Expand Down
11 changes: 9 additions & 2 deletions src/app/modules/angular-slickgrid/services/gridState.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { ExtensionService } from './extension.service';
import { FilterService } from './filter.service';
import { SortService } from './sort.service';
import { unsubscribeAllObservables } from './utilities';
import { ResizerService } from './resizer.service';
import { SharedService } from './shared.service';

// using external non-typed js libraries
Expand All @@ -41,6 +42,7 @@ export class GridStateService {
constructor(
private extensionService: ExtensionService,
private filterService: FilterService,
private resizerService: ResizerService,
private sharedService: SharedService,
private sortService: SortService
) {
Expand Down Expand Up @@ -153,18 +155,21 @@ export class GridStateService {
* Dynamically change the arrangement/distribution of the columns Positions/Visibilities and optionally Widths.
* For a column to have its visibly as hidden, it has to be part of the original list but excluded from the list provided as argument to be considered a hidden field.
* If you are passing columns Width, then you probably don't want to trigger the autosizeColumns (2nd argument to False).
* We could also resize the columns by their content but be aware that you can only trigger 1 type of resize at a time (either the 2nd argument or the 3rd last argument but not both at same time)
* The resize by content could be called by the 3rd argument OR simply by enabling `enableAutoResizeColumnsByCellContent` but again this will only get executed when the 2nd argument is set to false.
* @param {Array<Column>} definedColumns - defined columns
* @param {Boolean} triggerAutoSizeColumns - True by default, do we also want to call the "autosizeColumns()" method to make the columns fit in the grid?
* @param {Boolean} triggerColumnsFullResizeByContent - False by default, do we also want to call full columns resize by their content?
*/
changeColumnsArrangement(definedColumns: CurrentColumn[], triggerAutoSizeColumns = true) {
changeColumnsArrangement(definedColumns: CurrentColumn[], triggerAutoSizeColumns = true, triggerColumnsFullResizeByContent = false) {
if (Array.isArray(definedColumns) && definedColumns.length > 0) {
const gridColumns: Column[] = this.getAssociatedGridColumns(this._grid, definedColumns);

if (gridColumns && Array.isArray(gridColumns) && gridColumns.length > 0) {
// make sure that the checkbox selector is still visible in the list when it is enabled
if (this._gridOptions.enableCheckboxSelector) {
const checkboxColumn = (Array.isArray(this.sharedService.allColumns) && this.sharedService.allColumns.length > 0) ? this.sharedService.allColumns[0] : null;
if (checkboxColumn && checkboxColumn.id === '_checkbox_selector' && gridColumns[0].id !== '_checkbox_selector') {
if (checkboxColumn?.id === '_checkbox_selector' && gridColumns[0].id !== '_checkbox_selector') {
gridColumns.unshift(checkboxColumn);
}
}
Expand All @@ -180,6 +185,8 @@ export class GridStateService {
// resize the columns to fit the grid canvas
if (triggerAutoSizeColumns) {
this._grid.autosizeColumns();
} else if (triggerColumnsFullResizeByContent || this._gridOptions.enableAutoResizeColumnsByCellContent) {
this.resizerService.resizeColumnsByCellContent(true);
}
}
}
Expand Down

0 comments on commit ea4a196

Please sign in to comment.