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

feat(core): use DataView transactions with multiple item changes #14

Merged
merged 3 commits into from
Jul 16, 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
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