Skip to content

Commit

Permalink
refactor(ngrid): remove multi box-model support
Browse files Browse the repository at this point in the history
BREAKING CHANGE:
`PblNgridComponent.boxSpaceModel` was used to set the box-model strategy of the cells, either margin or padding.
Padding was the deault, when switching to margin a lot of CSS overwrites were applied and different width logic was used for column calculations. This was the case at the very start but as more and more features were added it got very hard to maintain and only padding strategy was updated and margin was not working properly, especially when used with group headers. For this reason I've decided to deprecate it as it is probably not used by anyone and adds unwanted complaxity. If it will be requried in future versions we can apply it through a plugin.
  • Loading branch information
shlomiassaf committed Jun 22, 2019
1 parent 63e6013 commit 6bf2544
Show file tree
Hide file tree
Showing 10 changed files with 3 additions and 112 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
[transpose]="toggleTranspose"
[usePagination]="usePagination"
focusMode="row"
[boxSpaceModel]="boxSpaceModel"
[detailRow]="detailRowPredicate" [singleDetailRow]="singleDetailRow"
[showHeader]="showHeader"
[showFooter]="showFooter"
Expand Down Expand Up @@ -111,7 +110,6 @@ <h1>Detail Row</h1>
<div class="example-feature-action" fxFlex fxLayout="column">
<mat-slide-toggle (change)="usePagination = $event.checked ? 'pageNumber' : false" [checked]="usePagination">Show Paginator</mat-slide-toggle>
<mat-slide-toggle (change)="toggleTranspose = $event.checked" [checked]="toggleTranspose">Transpose</mat-slide-toggle>
<mat-slide-toggle (change)="boxSpaceModel = $event.checked ? 'margin' : 'padding'" [checked]="boxSpaceModel === 'margin'">Margin Box Space Model</mat-slide-toggle>
<mat-slide-toggle (change)="showHeader = $event.checked" [checked]="showHeader">Show Header</mat-slide-toggle>
<mat-slide-toggle (change)="showFooter = $event.checked" [checked]="showFooter">Show Footer</mat-slide-toggle>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,6 @@ export class AllInOneGridExampleComponent {
showHeader = true;
hideColumns: string[] = [];
toggleTranspose = false;
boxSpaceModel: 'padding' | 'margin' = 'padding';
enableRowSelection = true;
singleDetailRow = false;

Expand Down
14 changes: 0 additions & 14 deletions libs/ngrid/src/lib/table/col-width-logic/dynamic-column-width.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,20 +101,6 @@ export class DynamicColumnWidthLogic {

}

export const DYNAMIC_MARGIN_BOX_MODEL_SPACE_STRATEGY: BoxModelSpaceStrategy = {
cell(col: PblColumnSizeInfo): number {
const style = col.style;
return parseInt(style.marginLeft) + parseInt(style.marginRight)
},
groupCell(col: PblColumnSizeInfo): number {
return this.cell(col);
},
group(cols: PblColumnSizeInfo[]): number {
const len = cols.length;
return len > 0 ? parseInt(cols[0].style.marginLeft) + parseInt(cols[len - 1].style.marginRight) : 0;
}
};

export const DYNAMIC_PADDING_BOX_MODEL_SPACE_STRATEGY: BoxModelSpaceStrategy = {
cell(col: PblColumnSizeInfo): number {
const style = col.style;
Expand Down
12 changes: 1 addition & 11 deletions libs/ngrid/src/lib/table/column-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,6 @@ import { PblColumn } from './columns/column';
import { PblColumnStore } from './columns/column-store';

export interface AutoSizeToFitOptions {
/**
* When true will not take into account box model gaps (padding/margin) when calculating the widths.
*
* Enabling might yield unexpected results.
*/
ignoreBoxModel?: boolean;

/**
* When `px` will force all columns width to be in fixed pixels
* When `%` will force all column width to be in %
Expand Down Expand Up @@ -157,10 +150,7 @@ export class ColumnApi<T> {
const instructions = columnBehavior(column) || options;

overflowTotalWidth += widthBreakout.content;

if (!options.ignoreBoxModel) {
totalWidth -= widthBreakout.nonContent;
}
totalWidth -= widthBreakout.nonContent;

if (instructions.keepMinWidth && column.minWidth) {
totalMinWidth += column.minWidth;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,5 @@
cursor: text;
}

.pbl-cdk-table.pbl-ngrid-margin-cell-box-model .pbl-ngrid-header-cell.pbl-header-group-cell {
&:not(:last-child) {
border-right-width: 1px;
border-right-style: solid;
}
}
/* MULTI COLUMN AND COLUMN SPAN */

Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ import { PblVirtualScrollForOf } from '../features/virtual-scroll/virtual-scroll
styleUrls: ['./pbl-cdk-table.component.scss'],
host: { // tslint:disable-line:use-host-property-decorator
'class': 'pbl-cdk-table',
'[class.pbl-ngrid-margin-cell-box-model]': `isMarginSpace`
},
encapsulation: ViewEncapsulation.None,
changeDetection: ChangeDetectionStrategy.OnPush,
Expand All @@ -64,8 +63,6 @@ export class PblCdkTableComponent<T> extends CdkTable<T> implements OnDestroy {
this._element.style.minWidth = value ? value + 'px' : null;
}

get isMarginSpace(): boolean { return this.table.boxSpaceModel === 'margin'; }

private _minWidth: number | null = null;
private onRenderRows$: Subject<DataRowOutlet>;
private _lastSticky: PblNgridColumnDef;
Expand Down
2 changes: 0 additions & 2 deletions libs/ngrid/src/lib/table/services/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,13 @@ export interface PblNgridConfig {
showHeader?: boolean;
showFooter?: boolean;
noFiller?: boolean;
boxSpaceModel?: 'padding' | 'margin';
}
}

const DEFAULT_TABLE_CONFIG: PblNgridConfig['table'] = {
showHeader: true,
showFooter: false,
noFiller: false,
boxSpaceModel: 'padding',
};

export const PEB_NGRID_CONFIG = new InjectionToken<PblNgridConfig>('PEB_NGRID_CONFIG');
Expand Down
38 changes: 2 additions & 36 deletions libs/ngrid/src/lib/table/table.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,7 @@ import { PblColumn, PblColumnStore, PblMetaColumnStore, PblNgridColumnSet, PblNg
import { PblNgridCellContext, PblNgridMetaCellContext, ContextApi, PblNgridContextApi } from './context/index';
import { PblNgridRegistryService } from './services/table-registry.service';
import { PblNgridConfigService } from './services/config';
import {
DynamicColumnWidthLogic,
BoxModelSpaceStrategy,
DYNAMIC_PADDING_BOX_MODEL_SPACE_STRATEGY,
DYNAMIC_MARGIN_BOX_MODEL_SPACE_STRATEGY
} from './col-width-logic/dynamic-column-width';
import { DynamicColumnWidthLogic, DYNAMIC_PADDING_BOX_MODEL_SPACE_STRATEGY } from './col-width-logic/dynamic-column-width';
import { ColumnApi, AutoSizeToFitOptions } from './column-api';
import { PblCdkVirtualScrollViewportComponent } from './features/virtual-scroll/virtual-scroll-viewport.component';
import { PblNgridMetaRowService } from './meta-rows/index';
Expand Down Expand Up @@ -84,34 +79,6 @@ export function metaRowServiceFactory(table: { _extApi: PblNgridExtensionApi; })
})
@UnRx()
export class PblNgridComponent<T = any> implements AfterContentInit, AfterViewInit, DoCheck, OnChanges, OnDestroy {
readonly self = this;

/**
* Set's the margin cell indentation strategy.
* Margin cell indentation strategy apply margin to cells instead of paddings and defines all cell box-sizing to border-box.
*
* When not set (default) the table will use a padding cell indentation strategy.
* Padding cell indentation strategy apply padding to cells and defines all cell box-sizing to content-box.
*/
@Input() get boxSpaceModel(): 'padding' | 'margin' { return this._boxSpaceModel; };
set boxSpaceModel(value: 'padding' | 'margin') {
if (this._boxSpaceModel !== value) {
this._boxSpaceModel = value;
this._cellWidthLogic = value === 'margin'
? DYNAMIC_MARGIN_BOX_MODEL_SPACE_STRATEGY
: DYNAMIC_PADDING_BOX_MODEL_SPACE_STRATEGY
;
if (this.isInit) {
// The UI changes are applied by toggle the `pbl-ngrid-margin-cell-box-model` CSS class.
// This is managed through binding in `PblCdkTableComponent`.
// After this change we need to measure the cell's width again so we trigger a resizeRows call.
// We must run it deferred to allow binding to commit.
this.ngZone.onStable.pipe(take(1)).subscribe( () => this.resizeColumns() );
}
}
}
private _boxSpaceModel: 'padding' | 'margin';
private _cellWidthLogic: BoxModelSpaceStrategy;

/**
* Show/Hide the header row.
Expand Down Expand Up @@ -313,7 +280,6 @@ export class PblNgridComponent<T = any> implements AfterContentInit, AfterViewIn
public registry: PblNgridRegistryService,
@Attribute('id') public readonly id: string) {
const tableConfig = config.get('table');
this.boxSpaceModel = tableConfig.boxSpaceModel;
this.showHeader = tableConfig.showHeader;
this.showFooter = tableConfig.showFooter;
this.noFiller = tableConfig.noFiller;
Expand Down Expand Up @@ -942,7 +908,7 @@ export class PblNgridComponent<T = any> implements AfterContentInit, AfterViewIn
columnStore: this._store,
setViewport: (viewport) => this._viewport = viewport,
dynamicColumnWidthFactory: (): DynamicColumnWidthLogic => {
return new DynamicColumnWidthLogic(this._cellWidthLogic);
return new DynamicColumnWidthLogic(DYNAMIC_PADDING_BOX_MODEL_SPACE_STRATEGY);
}
};
this._extApi = extApi;
Expand Down
1 change: 0 additions & 1 deletion libs/ngrid/theme/_pbl-cdk-table.scss
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
border-color: $border-color;
}

// border of "padding" cell box model (not .pbl-ngrid-margin-cell-box-model)
.pbl-ngrid-header-cell.pbl-header-group-cell:before {
border-color: $border-color;
}
Expand Down
36 changes: 0 additions & 36 deletions libs/ngrid/theme/_pbl-ngrid.scss
Original file line number Diff line number Diff line change
Expand Up @@ -145,42 +145,6 @@
}
}

.pbl-ngrid-margin-cell-box-model {
.pbl-ngrid-cell, .pbl-ngrid-header-cell, .pbl-ngrid-footer-cell {
box-sizing: border-box;

margin-left: $cell-spacing;
padding-left: 0;
padding-right: 0;

&:first-of-type {
margin-left: $row-spacing;
padding-left: 0;
padding-right: 0;

[dir='rtl'] & {
margin-left: 0;
margin-right: $row-spacing;
padding-left: 0;
padding-right: 0;
}
}

&:last-of-type {
margin-right: $row-spacing;
padding-left: 0;
padding-right: 0;

[dir='rtl'] & {
margin-right: 0;
margin-left: $row-spacing;
padding-left: 0;
padding-right: 0;
}
}
}
}

// row-reorder (drag & drop)
.pbl-ngrid-row-prefix {
display: none;
Expand Down

0 comments on commit 6bf2544

Please sign in to comment.