Skip to content

Commit

Permalink
fix(ngrid): wrong ds index reference in context when using multirow s…
Browse files Browse the repository at this point in the history 
…etup
  • Loading branch information
@shlomiassaf
shlomiassaf committed Dec 3, 2020
1 parent a408f06 commit 58ab268
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 55 deletions.
6 changes: 3 additions & 3 deletions libs/ngrid/clipboard/src/lib/clipboard.plugin.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ export class PblNgridClipboardPlugin implements OnDestroy {
if (col) {
const colIndex = columnApi.renderIndexOf(col);
if (colIndex > -1) {
const rowIndex = contextApi.findRowInCache(point.rowIdent).dataIndex;
const rowIndex = contextApi.findRowInCache(point.rowIdent).dsIndex;
const dataItem = col.getValue(grid.ds.source[rowIndex]);
const row = data.get(point.rowIdent) || [];
row[colIndex] = dataItem;
Expand All @@ -129,8 +129,8 @@ export class PblNgridClipboardPlugin implements OnDestroy {

const entries = Array.from(data.entries());
entries.sort((a, b) => {
const aIndex = contextApi.findRowInCache(a[0]).dataIndex;
const bIndex = contextApi.findRowInCache(b[0]).dataIndex;
const aIndex = contextApi.findRowInCache(a[0]).dsIndex;
const bIndex = contextApi.findRowInCache(b[0]).dsIndex;
if (aIndex < bIndex) {
return -1;
} else {
Expand Down
26 changes: 13 additions & 13 deletions libs/ngrid/src/lib/grid/context/api.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ export class ContextApi<T = any> {
if (ref instanceof PblCellContext) {
return ref.col.getValue(ref.rowContext.$implicit);
} else if (ref) {
const row = this.extApi.grid.ds.source[ref[0].dataIndex];
const row = this.extApi.grid.ds.source[ref[0].dsIndex];
const column = this.extApi.grid.columnApi.findColumnAt(ref[1]);
return column.getValue(row);
}
Expand Down Expand Up @@ -344,7 +344,7 @@ export class ContextApi<T = any> {
findRowInView(rowIdentity: any): PblRowContext<T> | undefined {
const rowState = this.cache.get(rowIdentity);
if (rowState) {
const renderRowIndex = rowState.dataIndex - this.extApi.grid.ds.renderStart;
const renderRowIndex = rowState.dsIndex - this.extApi.grid.ds.renderStart;
const rowContext = this.viewCache.get(renderRowIndex);
if (rowContext && rowContext.identity === rowIdentity) {
return rowContext;
Expand Down Expand Up @@ -374,28 +374,28 @@ export class ContextApi<T = any> {
if (!offset) {
return rowState;
} else {
const dataIndex = rowState.dataIndex + offset;
const identity = this.getRowIdentity(dataIndex);
const dsIndex = rowState.dsIndex + offset;
const identity = this.getRowIdentity(dsIndex);
if (identity !== null) {
let result = this.findRowInCache(identity);
if (!result && create && dataIndex < this.extApi.grid.ds.length) {
result = PblRowContext.defaultState(identity, dataIndex, this.columnApi.columns.length);
if (!result && create && dsIndex < this.extApi.grid.ds.length) {
result = PblRowContext.defaultState(identity, dsIndex, this.columnApi.columns.length);
this.cache.set(identity, result);
}
return result;
}
}
}

getRowIdentity(dataIndex: number, rowData?: T): string | number | null {
getRowIdentity(dsIndex: number, rowData?: T): string | number | null {
const { ds } = this.extApi.grid;
const { primary } = this.extApi.columnStore;

const row = rowData || ds.source[dataIndex];
const row = rowData || ds.source[dsIndex];
if (!row) {
return null;
} else {
return primary ? primary.getValue(row) : dataIndex;
return primary ? primary.getValue(row) : dsIndex;
}
}

Expand All @@ -408,11 +408,11 @@ export class ContextApi<T = any> {
}

_updateRowContext(rowContext: PblRowContext<T>, renderRowIndex: number) {
const dataIndex = this.extApi.grid.ds.renderStart + renderRowIndex;
const identity = this.getRowIdentity(dataIndex, rowContext.$implicit);
const dsIndex = this.extApi.grid.ds.renderStart + renderRowIndex;
const identity = this.getRowIdentity(dsIndex, rowContext.$implicit);
if (rowContext.identity !== identity) {
rowContext.saveState();
rowContext.dataIndex = dataIndex;
rowContext.dsIndex = dsIndex;
rowContext.identity = identity;
rowContext.fromState(this.getCreateState(rowContext));
rowContext.updateOutOfViewState();
Expand All @@ -428,7 +428,7 @@ export class ContextApi<T = any> {
private getCreateState(context: PblRowContext<T>) {
let state = this.cache.get(context.identity);
if (!state) {
state = PblRowContext.defaultState(context.identity, context.dataIndex, this.columnApi.columns.length);
state = PblRowContext.defaultState(context.identity, context.dsIndex, this.columnApi.columns.length);
this.cache.set(context.identity, state);
}
return state;
Expand Down
63 changes: 28 additions & 35 deletions libs/ngrid/src/lib/grid/context/row.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,17 @@ export class PblRowContext<T> implements PblNgridRowContext<T> {
get $implicit(): T | undefined { return this._$implicit; }
set $implicit(value: T) {
if (this._$implicit !== value) {
this.updateRowData(value);
this._$implicit = value;
this.updateRowData();
}
};

/** Index of the data object in the provided data array. */
index?: number;
/** Index of the data object in the provided data array. */
get dataIndex() { return this.index; }
set dataIndex(value: number) { this.index = value; }

/** Index location of the rendered row that this cell is located within. */
renderIndex?: number;
/** Length of the number of total rows. */
Expand All @@ -31,6 +36,8 @@ export class PblRowContext<T> implements PblNgridRowContext<T> {
/** True if this cell is contained in a row with an odd-numbered index. */
odd?: boolean;

/** The index at the datasource */
dsIndex: number;
identity: any;
firstRender: boolean;
outOfView: boolean;
Expand All @@ -47,29 +54,12 @@ export class PblRowContext<T> implements PblNgridRowContext<T> {
private cells: PblCellContext<T>[];

private _$implicit?: T;
private _updatePending: boolean;

constructor(_data: T, public dataIndex: number, private extApi: PblNgridExtensionApi<T>) {
/* TODO: material2#14198
The row context come from the `cdk` and it can be of 2 types, depending if multiple row templates are used or not.
`index` is used for single row template mode and `renderIndex` for multi row template mode.
There library and/or plugins require access to the rendered index and having 2 locations is a problem...
It's a bug trap, adding more complexity and some time access issue because the `CdkTable` instance is not always available.
This is a workaround for have a single location for the rendered index.
I chose to `index` as the single location although `renderIndex` will probably be chosen by the material team.
This is because it's less likely to occur as most tables does not have multi row templates (detail row)
A refactor will have to be done in the future.
There is a pending issue to do so in https://github.com/angular/material2/issues/14198
Also related: https://github.com/angular/material2/issues/14199
*/
const applyWorkaround = extApi.cdkTable.multiTemplateDataRows;
if (applyWorkaround) {
Object.defineProperty(this, 'index', { get: function() { return this.renderIndex; } });
}

constructor(_data: T, dsIndex: number, private extApi: PblNgridExtensionApi<T>) {
this.dsIndex = dsIndex;
this._$implicit = _data;
this.identity = this.extApi.contextApi.getRowIdentity(dataIndex, _data);
this.identity = this.extApi.contextApi.getRowIdentity(dsIndex, _data);

this.grid = extApi.grid;
const cells = this.cells = [];
Expand All @@ -82,12 +72,12 @@ export class PblRowContext<T> implements PblNgridRowContext<T> {
}
}

static defaultState<T = any>(identity: any, dataIndex: number, cellsCount: number): RowContextState<T> {
static defaultState<T = any>(identity: any, dsIndex: number, cellsCount: number): RowContextState<T> {
const cells: CellContextState<T>[] = [];
for (let i = 0; i < cellsCount; i++) {
cells.push(PblCellContext.defaultState());
}
return { identity, dataIndex, cells, firstRender: true, external: {} };
return { identity, dsIndex, cells, firstRender: true, external: {} };
}

getExternal<P extends keyof ExternalRowContextState>(key: P): ExternalRowContextState[P] {
Expand All @@ -104,7 +94,7 @@ export class PblRowContext<T> implements PblNgridRowContext<T> {
getState(): RowContextState<T> {
return {
identity: this.identity,
dataIndex: this.dataIndex,
dsIndex: this.dsIndex,
firstRender: this.firstRender,
cells: this.cells.map( c => c.getState() ),
external: this.external,
Expand All @@ -113,7 +103,7 @@ export class PblRowContext<T> implements PblNgridRowContext<T> {

fromState(state: RowContextState<T>): void {
this.firstRender = state.firstRender;
this.dataIndex = state.dataIndex;
this.dsIndex = state.dsIndex;
this.external = state.external;
for (let i = 0, len = this.cells.length; i < len; i++) {
this.cells[i].fromState(state.cells[i], this);
Expand All @@ -124,11 +114,6 @@ export class PblRowContext<T> implements PblNgridRowContext<T> {
this.extApi.contextApi.saveState(this);
}

updateContext(context: RowContext<T>): void {
context.dataIndex = this.dataIndex;
Object.assign(this, context);
}

/**
* Returns the cell context for the column at the specified position.
* > The position is relative to ALL columns (NOT RENDERED COLUMNS)
Expand All @@ -155,7 +140,11 @@ export class PblRowContext<T> implements PblNgridRowContext<T> {
attachRow(row: PblNgridRowComponent<T>) {
this.detachRow(this._attachedRow);
this._attachedRow = row;
this.updateOutOfViewState();
if (this._updatePending) {
this.updateRowData();
} else {
this.updateOutOfViewState();
}
}

detachRow(row: PblNgridRowComponent<T>) {
Expand All @@ -165,8 +154,12 @@ export class PblRowContext<T> implements PblNgridRowContext<T> {
}
}

private updateRowData(data: T) {
this._$implicit = data;
this.extApi.contextApi._updateRowContext(this, this._attachedRow.rowIndex);
private updateRowData() {
if (this._attachedRow) {
this._updatePending = false;
this.extApi.contextApi._updateRowContext(this, this._attachedRow.rowIndex);
} else {
this._updatePending = !!this._$implicit;
}
}
}
5 changes: 4 additions & 1 deletion libs/ngrid/src/lib/grid/context/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { PblRowContext } from './row';

export interface RowContextState<T = any> {
identity: any;
dataIndex: number;
dsIndex: number;
cells: CellContextState<T>[];
firstRender: boolean;
external: any;
Expand Down Expand Up @@ -102,6 +102,9 @@ export interface PblNgridRowContext<T = any> extends RowContext<T> {
*/
outOfView: boolean;

/** The index at the datasource */
dsIndex: number;

readonly grid: PblNgridComponent<T>;

/**
Expand Down
2 changes: 1 addition & 1 deletion libs/ngrid/src/lib/grid/row/row.component.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ export class PblNgridRowComponent<T = any> extends PblNgridBaseRowComponent<'dat
}

private identityUpdated() {
this.element.setAttribute('row-id', this.context.dataIndex as any);
this.element.setAttribute('row-id', this.context.dsIndex as any);
this.element.setAttribute('row-key', this.context.identity);
}
}
4 changes: 2 additions & 2 deletions libs/ngrid/target-events/src/lib/target-events/focus-and-selection.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,8 +207,8 @@ function createHandlers(targetEvents: PblNgridTargetEventsPlugin) {
}
hCells.push({ rowIdent: activeFocus.rowIdent, colIndex: cellContext.index });

const rowHeight = Math.abs(cellContext.rowContext.dataIndex - focusedRowState.dataIndex);
const dir = focusedRowState.dataIndex > cellContext.rowContext.dataIndex ? -1 : 1;
const rowHeight = Math.abs(cellContext.rowContext.dsIndex - focusedRowState.dsIndex);
const dir = focusedRowState.dsIndex > cellContext.rowContext.dsIndex ? -1 : 1;
for (let i = 1; i <= rowHeight; i++) {
const state = contextApi.findRowInCache(activeFocus.rowIdent, dir * i, true);
vCells.push({ rowIdent: state.identity, colIndex: activeFocus.colIndex });
Expand Down

0 comments on commit 58ab268

Please sign in to comment.