Skip to content
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

fix(core): clear dataset when destroying & fix few unsubscribed events #629

Merged
merged 2 commits into from
Nov 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 12 additions & 11 deletions src/app/examples/grid-frozen.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,9 @@ export class GridFrozenComponent implements OnInit, OnDestroy {

ngOnDestroy() {
// unsubscribe every SlickGrid subscribed event (or use the Slick.EventHandler)
this.gridObj.onMouseEnter.unsubscribe();
this.gridObj.onMouseLeave.unsubscribe();
this.gridObj.onMouseEnter.unsubscribe(this.highlightRow);
this.gridObj.onMouseLeave.unsubscribe(this.highlightRow);
this.highlightRow = null;
}

angularGridReady(angularGrid: AngularGridInstance) {
Expand All @@ -44,15 +45,15 @@ export class GridFrozenComponent implements OnInit, OnDestroy {
// with frozen (pinned) grid, in order to see the entire row being highlighted when hovering
// we need to do some extra tricks (that is because frozen grids use 2 separate div containers)
// the trick is to use row selection to highlight when hovering current row and remove selection once we're not
this.gridObj.onMouseEnter.subscribe(event => {
const cell = this.gridObj.getCellFromEvent(event);
this.gridObj.setSelectedRows([cell.row]); // highlight current row
event.preventDefault();
});
this.gridObj.onMouseLeave.subscribe(event => {
this.gridObj.setSelectedRows([]); // remove highlight
event.preventDefault();
});
this.gridObj.onMouseEnter.subscribe(event => this.highlightRow(event, true));
this.gridObj.onMouseLeave.subscribe(event => this.highlightRow(event, false));
}

highlightRow(event: Event, isMouseEnter: boolean) {
const cell = this.gridObj.getCellFromEvent(event);
const rows = isMouseEnter ? [cell.row] : [];
this.gridObj.setSelectedRows(rows); // highlight current row
event.preventDefault();
}

prepareDataGrid() {
Expand Down
17 changes: 11 additions & 6 deletions src/app/examples/grid-rowmove.component.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { Component, OnInit } from '@angular/core';
import { Component, OnDestroy, OnInit } from '@angular/core';
import { AngularGridInstance, Column, ExtensionName, Filters, Formatters, GridOption } from './../modules/angular-slickgrid';
import { TranslateService } from '@ngx-translate/core';

@Component({
templateUrl: './grid-rowmove.component.html'
})
export class GridRowMoveComponent implements OnInit {
export class GridRowMoveComponent implements OnInit, OnDestroy {
title = 'Example 17: Row Move & Checkbox Selector';
subTitle = `This example demonstrates using the <b>Slick.Plugins.RowMoveManager</b> plugin to easily move a row in the grid.<br/>
<ul>
Expand Down Expand Up @@ -41,6 +41,12 @@ export class GridRowMoveComponent implements OnInit {
return this.angularGrid && this.angularGrid.extensionService.getSlickgridAddonInstance(ExtensionName.rowMoveManager) || {};
}

ngOnDestroy() {
// nullify the callbacks to avoid mem leaks
this.onBeforeMoveRow = null;
this.onMoveRows = null;
}

ngOnInit(): void {
this.columnDefinitions = [
{ id: 'title', name: 'Title', field: 'title', filterable: true, },
Expand Down Expand Up @@ -95,8 +101,8 @@ export class GridRowMoveComponent implements OnInit {
disableRowSelection: true,
cancelEditOnDrag: true,
width: 30,
onBeforeMoveRows: (e, args) => this.onBeforeMoveRow(e, args),
onMoveRows: (e, args) => this.onMoveRows(e, args),
onBeforeMoveRows: this.onBeforeMoveRow,
onMoveRows: this.onMoveRows.bind(this),

// you can change the move icon position of any extension (RowMove, RowDetail or RowSelector icon)
// note that you might have to play with the position when using multiple extension
Expand Down Expand Up @@ -165,8 +171,7 @@ export class GridRowMoveComponent implements OnInit {
for (let i = 0; i < rows.length; i++) {
selectedRows.push(left.length + i);
}

this.angularGrid.slickGrid.resetActiveCell();
args.grid.resetActiveCell();
this.dataset = tmpDataset;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ const mockDataView = {
endUpdate: jest.fn(),
getItem: jest.fn(),
getItems: jest.fn(),
getLength: jest.fn(),
getItemMetadata: jest.fn(),
getPagingInfo: jest.fn(),
mapIdsToRows: jest.fn(),
Expand Down Expand Up @@ -335,7 +336,7 @@ describe('Angular-Slickgrid Custom Component instantiated via Constructor', () =

it('should load jQuery mousewheel when using a frozen grid', () => {
const loadSpy = jest.spyOn(component, 'loadJqueryMousewheelDynamically');
component.gridOptions.frozenRow = 3;
component.gridOptions.frozenColumn = 3;

component.ngOnInit();
component.ngAfterViewInit();
Expand Down Expand Up @@ -688,7 +689,7 @@ describe('Angular-Slickgrid Custom Component instantiated via Constructor', () =
component.ngAfterViewInit();
component.destroy(true);

expect(spy).toHaveBeenCalledWith();
expect(spy).toHaveBeenCalled();
});

it('should refresh a local grid and change pagination options pagination when a preset for it is defined in grid options', (done) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,6 @@ const treeDataServiceStub = {
describe('App Component', () => {
let fixture: ComponentFixture<AngularSlickgridComponent>;
let component: AngularSlickgridComponent;
let graphqlService: GraphqlService;
let translate: TranslateService;

beforeEach(async(() => {
// @ts-ignore
Expand Down Expand Up @@ -180,8 +178,6 @@ describe('App Component', () => {
// create the component
fixture = TestBed.createComponent(AngularSlickgridComponent);
component = fixture.debugElement.componentInstance;
graphqlService = TestBed.get(GraphqlService);
translate = TestBed.get(TranslateService);

// setup bindable properties
component.gridId = 'grid1';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ const slickgridEventPrefix = 'sg';
]
})
export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnInit {
private _dataset: any[];
private _dataset: any[] | null;
private _columnDefinitions: Column[];
private _eventHandler: SlickEventHandler = new Slick.EventHandler();
private _angularGridInstances: AngularGridInstance | undefined;
Expand All @@ -131,8 +131,8 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn
private _isDatasetInitialized = false;
private _isPaginationInitialized = false;
private _isLocalGrid = true;
dataView: any;
grid: any;
dataView: any | null;
grid: any | null;
gridHeightString: string;
gridWidthString: string;
groupingDefinition: any = {};
Expand Down Expand Up @@ -196,19 +196,19 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn
}

@Input()
get datasetHierarchical(): any[] {
get datasetHierarchical(): any[] | null {
return this.sharedService.hierarchicalDataset;
}
set datasetHierarchical(newHierarchicalDataset: any[]) {
set datasetHierarchical(newHierarchicalDataset: any[] | null) {
this.sharedService.hierarchicalDataset = newHierarchicalDataset;

if (this.filterService && this.filterService.clearFilters) {
if (newHierarchicalDataset && this.columnDefinitions && this.filterService && this.filterService.clearFilters) {
this.filterService.clearFilters();
}

// when a hierarchical dataset is set afterward, we can reset the flat dataset and call a tree data sort that will overwrite the flat dataset
setTimeout(() => {
if (this.dataView && this.sortService && this.sortService.processTreeDataInitialSort && this.gridOptions && this.gridOptions.enableTreeData) {
if (newHierarchicalDataset && this.dataView && this.sortService && this.sortService.processTreeDataInitialSort && this.gridOptions && this.gridOptions.enableTreeData) {
this.dataView.setItems([], this.gridOptions.datasetIdPropertyName);
this.sortService.processTreeDataInitialSort();
}
Expand Down Expand Up @@ -254,7 +254,7 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn
}

ngOnInit(): void {
if (this.gridOptions && this.gridOptions.frozenRow >= 0) {
if (this.gridOptions && (this.gridOptions.frozenColumn >= 0 || this.gridOptions.frozenRow >= 0)) {
this.loadJqueryMousewheelDynamically();
}

Expand Down Expand Up @@ -285,11 +285,29 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn
if (this._eventHandler && this._eventHandler.unsubscribeAll) {
this._eventHandler.unsubscribeAll();
}
if (this.dataView && this.dataView.setItems) {
this.dataView.setItems([]);
if (this.dataView) {
if (this.dataView && this.dataView.setItems) {
this.dataView.setItems([]);
}
if (this.dataView.destroy) {
this.dataView.destroy();
}
}
if (this.grid && this.grid.destroy) {
this.grid.destroy();
this.grid.destroy(shouldEmptyDomElementContainer);
}

if (this.backendServiceApi) {
for (const prop of Object.keys(this.backendServiceApi)) {
this.backendServiceApi[prop] = null;
}
this.backendServiceApi = null;
}
for (const prop of Object.keys(this.columnDefinitions)) {
this.columnDefinitions[prop] = null;
}
for (const prop of Object.keys(this.sharedService)) {
this.sharedService[prop] = null;
}

// we could optionally also empty the content of the grid container DOM element
Expand All @@ -299,6 +317,11 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn

// also unsubscribe all RxJS subscriptions
this.subscriptions = unsubscribeAllObservables(this.subscriptions);

this._dataset = null;
this.datasetHierarchical = null;
this._columnDefinitions = [];
this._angularGridInstances = null;
}

emptyGridContainerElm() {
Expand Down Expand Up @@ -326,11 +349,10 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn
if (backendApi.service instanceof GraphqlService || typeof backendApi.service.getDatasetName === 'function') {
backendApi.internalPostProcess = (processResult: GraphqlResult | GraphqlPaginatedResult) => {
const datasetName = (backendApi && backendApi.service && typeof backendApi.service.getDatasetName === 'function') ? backendApi.service.getDatasetName() : '';
this._dataset = [];
if (processResult && processResult.data && processResult.data[datasetName]) {
this._dataset = processResult.data[datasetName].hasOwnProperty('nodes') ? (processResult as GraphqlPaginatedResult).data[datasetName].nodes : (processResult as GraphqlResult).data[datasetName];
const data = processResult.data[datasetName].hasOwnProperty('nodes') ? (processResult as GraphqlPaginatedResult).data[datasetName].nodes : (processResult as GraphqlResult).data[datasetName];
const totalCount = processResult.data[datasetName].hasOwnProperty('totalCount') ? (processResult as GraphqlPaginatedResult).data[datasetName].totalCount : (processResult as GraphqlResult).data[datasetName].length;
this.refreshGridData(this._dataset, totalCount || 0);
this.refreshGridData(data, totalCount || 0);
}
};
}
Expand Down Expand Up @@ -786,7 +808,7 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn
if (!this.customDataView && (this.dataView && this.dataView.beginUpdate && this.dataView.setItems && this.dataView.endUpdate)) {
this.onDataviewCreated.emit(this.dataView);
this.dataView.beginUpdate();
this.dataView.setItems(this._dataset, this.gridOptions.datasetIdPropertyName);
this.dataView.setItems(this._dataset || [], this.gridOptions.datasetIdPropertyName);
this.dataView.endUpdate();

// if you don't want the items that are not visible (due to being filtered out or being on a different page)
Expand All @@ -813,7 +835,8 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn
}
}

if (this._dataset.length > 0) {
const datasetLn = this.dataView.getLength() || this._dataset && this._dataset.length || 0;
if (datasetLn > 0) {
if (!this._isDatasetInitialized && (this.gridOptions.enableCheckboxSelector || this.gridOptions.enableRowSelection)) {
this.loadRowSelectionPresetWhenExists();
}
Expand Down Expand Up @@ -1076,7 +1099,7 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn
private swapInternalEditorToSlickGridFactoryEditor(columnDefinitions: Column[]) {
return columnDefinitions.map((column: Column | any) => {
// on every Editor that have a "collectionAsync", resolve the data and assign it to the "collection" property
if (column.editor && column.editor.collectionAsync) {
if (column && column.editor && column.editor.collectionAsync) {
this.loadEditorCollectionAsync(column);
}
return { ...column, editor: column.editor && column.editor.model, internalColumnEditor: { ...column.editor } };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ describe('LongTextEditor', () => {
describe('isValueChanged method', () => {
it('should return True when previously dispatched keyboard event is a new char "a" and it should also update the text counter accordingly', () => {
const eventKeyDown = new (window.window as any).KeyboardEvent('keydown', { keyCode: KEY_CHAR_A, bubbles: true, cancelable: true });
const eventKeyUp = new (window.window as any).KeyboardEvent('keyup', { keyCode: KEY_CHAR_A, bubbles: true, cancelable: true });
const eventInput = new (window.window as any).Event('input', { keyCode: KEY_CHAR_A, bubbles: true, cancelable: true });
mockColumn.internalColumnEditor.maxLength = 255;

editor = new LongTextEditor(editorArguments);
Expand All @@ -230,7 +230,7 @@ describe('LongTextEditor', () => {
const maxTextLengthElm = document.body.querySelector<HTMLDivElement>('.editor-footer .max-length');
editor.focus();
editorElm.dispatchEvent(eventKeyDown);
editorElm.dispatchEvent(eventKeyUp);
editorElm.dispatchEvent(eventInput);

expect(currentTextLengthElm.textContent).toBe('1');
expect(maxTextLengthElm.textContent).toBe('255');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export class AutoCompleteEditor implements Editor {
private _autoCompleteOptions: AutocompleteOption;
private _currentValue: any;
private _defaultTextValue: string;
private _elementCollection: any[];
private _elementCollection: any[] | null;
private _lastInputEvent: JQuery.Event;

/** The JQuery DOM element */
Expand Down Expand Up @@ -73,7 +73,7 @@ export class AutoCompleteEditor implements Editor {
}

/** Get the Final Collection used in the AutoCompleted Source (this may vary from the "collection" especially when providing a customStructure) */
get elementCollection(): any[] {
get elementCollection(): any[] | null {
return this._elementCollection;
}

Expand Down Expand Up @@ -143,8 +143,9 @@ export class AutoCompleteEditor implements Editor {
if (this._$editorElm) {
this._$editorElm.autocomplete('destroy');
this._$editorElm.off('keydown.nav').remove();
this._$editorElm = null;
}
this._$editorElm = null;
this._elementCollection = null;
}

focus() {
Expand Down
8 changes: 4 additions & 4 deletions src/app/modules/angular-slickgrid/editors/longTextEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ export class LongTextEditor implements Editor {
this._$wrapper.find('.btn-save').on('click', () => this.save());
this._$wrapper.find('.btn-cancel').on('click', () => this.cancel());
this._$textarea.on('keydown', this.handleKeyDown.bind(this));
this._$textarea.on('keyup', this.handleKeyUp.bind(this));
this._$textarea.on('input', this.handleOnInputChange.bind(this));

this.position(this.args && this.args.position);
this._$textarea.focus().select();
Expand All @@ -146,7 +146,7 @@ export class LongTextEditor implements Editor {
destroy() {
if (this._$textarea) {
this._$textarea.off('keydown');
this._$textarea.off('keyup');
this._$textarea.off('input');
}
if (this._$wrapper) {
this._$wrapper.find('.btn-save').off('click');
Expand Down Expand Up @@ -268,8 +268,8 @@ export class LongTextEditor implements Editor {
}
}

/** On every keyup event, we'll update the current text length counter */
private handleKeyUp(event: KeyboardEvent & { target: HTMLTextAreaElement }) {
/** On every input change event, we'll update the current text length counter */
private handleOnInputChange(event: KeyboardEvent & { target: HTMLTextAreaElement }) {
const textLength = event.target.value.length;
this._$currentLengthElm.text(textLength);
}
Expand Down
Loading