Skip to content

Commit

Permalink
feat(core): use DataView transactions with multiple item changes (#14)
Browse files Browse the repository at this point in the history
* feat(core): use DataView transactions with multiple item changes
- we should use DataView transactions as much as possible when dealing with multiple items (array) in 1 call, that is true for (addItems, deleteItems, updateItems)
  - DataView `beginUpdate` ...  `endUpdate`
  • Loading branch information
ghiscoding authored Jul 16, 2020
1 parent 959097c commit 8cbd03a
Show file tree
Hide file tree
Showing 9 changed files with 114 additions and 10 deletions.
5 changes: 4 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,10 @@ npm run test:watch
- [x] Should work even after initializing the dataset later (SF)
- [x] Preset Filters not working with Tree Data View
- [x] Dynamically Add Columns
- [ ] Translations Support
- [x] Tree Data
- [ ] Translations Support
- [ ] add missing `collectionAsync` for Editors (maybe Filter too?)
- [x] Grid Service should use SlickGrid transactions `beginUpdate`, `endUpdate` for performance reason whenever possible

#### Other Todos
- [x] VScode Chrome Debugger
Expand All @@ -147,3 +149,4 @@ npm run test:watch
- [x] Add interfaces to all SlickGrid core lib classes & plugins (basically add Types to everything)
- [x] Copy cell text (context menu) doesn't work in SF
- [x] Remove all Services init method 2nd argument (we can get DataView directly from the Grid object)
- [ ] Search for any left "todo" in the entire solution
2 changes: 1 addition & 1 deletion packages/common/src/extensions/extensionUtility.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export class ExtensionUtility {
* This will basically only load the extension when user enables the feature
* @param extensionName
*/
loadExtensionDynamically(extensionName: ExtensionName): any {
loadExtensionDynamically(extensionName: ExtensionName) {
try {
switch (extensionName) {
case ExtensionName.autoTooltip:
Expand Down
2 changes: 1 addition & 1 deletion packages/common/src/global-grid-options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export const GlobalGridOptions: GridOption = {
dateFormat: 'YYYY-MM-DD, hh:mm a',
hideTotalItemCount: false,
hideLastUpdateTimestamp: true,
footerHeight: 20,
footerHeight: 25,
leftContainerClass: 'col-xs-12 col-sm-5',
rightContainerClass: 'col-xs-6 col-sm-7',
metricSeparator: '|',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export interface CustomFooterOption {
/** Date format used when showing the "Last Update" timestamp in the metrics section. */
dateFormat?: string;

/** Defaults to 20, height of the Custom Footer in pixels (this is required and is used by the auto-resizer) */
/** Defaults to 25, height of the Custom Footer in pixels (this is required and is used by the auto-resizer) */
footerHeight?: number;

/** Defaults to false, do we want to hide the last update timestamp (endTime)? */
Expand Down
91 changes: 90 additions & 1 deletion packages/common/src/services/__tests__/grid.service.spec.ts

Large diffs are not rendered by default.

12 changes: 12 additions & 0 deletions packages/common/src/services/grid.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,9 @@ export class GridService {
if (!Array.isArray(items)) {
return [this.addItem<T>(items, options) || 0];
} else {
this._dataView.beginUpdate();
items.forEach((item: T) => this.addItem(item, { ...options, highlightRow: false, resortGrid: false, triggerEvent: false, selectRow: false }));
this._dataView.endUpdate();
}

// do we want the item to be sorted in the grid, when set to False it will insert on first row (defaults to false)
Expand Down Expand Up @@ -415,13 +417,16 @@ export class GridService {
this.deleteItem<T>(items, options);
return [items[idPropName]];
}

this._dataView.beginUpdate();
const itemIds: string[] = [];
items.forEach((item: T) => {
if (item && item[idPropName] !== undefined) {
itemIds.push(item[idPropName]);
}
this.deleteItem<T>(item, { ...options, triggerEvent: false });
});
this._dataView.endUpdate();

// do we want to trigger an event after deleting the item
if (options.triggerEvent) {
Expand Down Expand Up @@ -469,11 +474,13 @@ export class GridService {

// when it's not an array, we can call directly the single item delete
if (Array.isArray(itemIds)) {
this._dataView.beginUpdate();
for (let i = 0; i < itemIds.length; i++) {
if (itemIds[i] !== null) {
this.deleteItemById(itemIds[i], { triggerEvent: false });
}
}
this._dataView.endUpdate();

// do we want to trigger an event after deleting the item
if (options.triggerEvent) {
Expand Down Expand Up @@ -516,10 +523,12 @@ export class GridService {
return [this.updateItem<T>(items, options)];
}

this._dataView.beginUpdate();
const gridRowNumbers: number[] = [];
items.forEach((item: T) => {
gridRowNumbers.push(this.updateItem<T>(item, { ...options, highlightRow: false, selectRow: false, triggerEvent: false }));
});
this._dataView.endUpdate();

// only highlight at the end, all at once
// we have to do this because doing highlight 1 by 1 would only re-select the last highlighted row which is wrong behavior
Expand Down Expand Up @@ -615,10 +624,13 @@ export class GridService {
if (!Array.isArray(items)) {
return [this.upsertItem<T>(items, options)];
}

this._dataView.beginUpdate();
const upsertedRows: { added: number | undefined, updated: number | undefined }[] = [];
items.forEach((item: T) => {
upsertedRows.push(this.upsertItem<T>(item, { ...options, highlightRow: false, resortGrid: false, selectRow: false, triggerEvent: false }));
});
this._dataView.endUpdate();

const rowNumbers = upsertedRows.map((upsertRow) => upsertRow.added !== undefined ? upsertRow.added : upsertRow.updated) as number[];

Expand Down
Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -1343,7 +1343,7 @@ describe('Slick-Vanilla-Grid-Bundle Component instantiated via Constructor', ()
dateFormat: 'YYYY-MM-DD, hh:mm a',
hideLastUpdateTimestamp: true,
hideTotalItemCount: false,
footerHeight: 20,
footerHeight: 25,
leftContainerClass: 'col-xs-12 col-sm-5',
metricSeparator: '|',
metricTexts: {
Expand Down Expand Up @@ -1380,7 +1380,7 @@ describe('Slick-Vanilla-Grid-Bundle Component instantiated via Constructor', ()
dateFormat: 'YYYY-MM-DD, hh:mm a',
hideLastUpdateTimestamp: true,
hideTotalItemCount: false,
footerHeight: 20,
footerHeight: 25,
leftContainerClass: 'col-xs-12 col-sm-5',
metricSeparator: '|',
metricTexts: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ import { SalesforceGlobalGridOptions } from '../salesforce-global-grid-options';

// using external non-typed js libraries
declare const Slick: SlickNamespace;
const DATAGRID_FOOTER_HEIGHT = 20;
const DATAGRID_FOOTER_HEIGHT = 25;
const DATAGRID_PAGINATION_HEIGHT = 35;

export class SlickVanillaGridBundle {
Expand Down Expand Up @@ -840,7 +840,7 @@ export class SlickVanillaGridBundle {
this.grid.autosizeColumns();
}

// auto-resize grid on browser resize
// auto-resize grid on browser resize (optionally provide grid height or width)
if (options.gridHeight || options.gridWidth) {
this.resizerPlugin.resizeGrid(0, { height: options.gridHeight, width: options.gridWidth });
} else {
Expand Down Expand Up @@ -1154,7 +1154,7 @@ export class SlickVanillaGridBundle {

if (isResizeRequired && this.resizerPlugin?.resizeGrid) {
this.resizerPlugin.resizeGrid();
} else if (/* !isResizeRequired || */ (this._intervalExecutionCounter++ > (4 * 60 * INTERVAL_MAX_MIN_RETRIES))) { // interval is 250ms, so 4x is 1sec, so (4 * 60 * intervalMaxTimeInMin) shoud be 70min
} else if ((!isResizeRequired && !this.gridOptions.useSalesforceDefaultGridOptions) || (this._intervalExecutionCounter++ > (4 * 60 * INTERVAL_MAX_MIN_RETRIES))) { // interval is 250ms, so 4x is 1sec, so (4 * 60 * intervalMaxTimeInMin) shoud be 70min
clearInterval(this._intervalId); // stop the interval if we don't need resize or if we passed let say 70min
}
}, 250);
Expand Down

0 comments on commit 8cbd03a

Please sign in to comment.