Skip to content

Commit

Permalink
fix(material-experimental/mdc-table): error when flexbox-based table …
Browse files Browse the repository at this point in the history
…is initialized (#21428)

The MDC-based table tries to add a class to its internal `tbody` element and it assumes
that it's always going to find it. The assumption used to be correct since we only
supported native `table` elements for the MDC implementation, however as of #20994
we also support flexbox-based ones which throw an error, because they don't have a
`tbody`.

These changes add a check to prevent the error and include a couple of sanity tests to
catch issues like this in the future.

I've also reworked the tests so that they use the MDC-based `MatPaginator` and
`MatTableDataSource`, instead of the base ones.

(cherry picked from commit 2255b66)
  • Loading branch information
crisbeto authored and annieyw committed Jan 6, 2021
1 parent 4d60f90 commit aade5d1
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 10 deletions.
2 changes: 1 addition & 1 deletion src/cdk/table/table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ export class CdkTable<T> implements AfterContentChecked, CollectionViewer, OnDes
private _cachedRenderRowsMap = new Map<T, WeakMap<CdkRowDef<T>, RenderRow<T>[]>>();

/** Whether the table is applied to a native `<table>`. */
private _isNativeHtmlTable: boolean;
protected _isNativeHtmlTable: boolean;

/**
* Utility class that is responsible for applying the appropriate sticky positioning styles to
Expand Down
12 changes: 9 additions & 3 deletions src/material-experimental/mdc-table/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -62,17 +62,23 @@ ng_test_library(
deps = [
":mdc-table",
"//src/cdk/table",
"//src/material/paginator",
"//src/material-experimental/mdc-paginator",
"//src/material/sort",
"//src/material/table",
"@npm//@angular/platform-browser",
"@npm//rxjs",
],
)

ng_web_test_suite(
name = "unit_tests",
static_files = ["@npm//:node_modules/@material/data-table/dist/mdc.dataTable.js"],
static_files = [
"@npm//:node_modules/@material/textfield/dist/mdc.textfield.js",
"@npm//:node_modules/@material/line-ripple/dist/mdc.lineRipple.js",
"@npm//:node_modules/@material/notched-outline/dist/mdc.notchedOutline.js",
"@npm//:node_modules/@material/ripple/dist/mdc.ripple.js",
"@npm//:node_modules/@material/dom/dist/mdc.dom.js",
"@npm//:node_modules/@material/data-table/dist/mdc.dataTable.js",
],
deps = [
":table_tests_lib",
"//src/material-experimental:mdc_require_config.js",
Expand Down
60 changes: 57 additions & 3 deletions src/material-experimental/mdc-table/table.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,11 @@ import {
TestBed,
tick
} from '@angular/core/testing';
import {MatTable, MatTableModule} from './index';
import {MatTable, MatTableDataSource, MatTableModule} from './index';
import {DataSource} from '@angular/cdk/table';
import {BehaviorSubject, Observable} from 'rxjs';
import {MatSort, MatSortHeader, MatSortModule} from '@angular/material/sort';
import {MatPaginator, MatPaginatorModule} from '@angular/material/paginator';
import {MatTableDataSource} from '@angular/material/table';
import {MatPaginator, MatPaginatorModule} from '@angular/material-experimental/mdc-paginator';
import {NoopAnimationsModule} from '@angular/platform-browser/animations';

describe('MDC-based MatTable', () => {
Expand All @@ -29,6 +28,7 @@ describe('MDC-based MatTable', () => {
StickyTableApp,
TableWithNgContainerRow,
NestedTableApp,
MatFlexTableApp,
],
}).compileComponents();
}));
Expand Down Expand Up @@ -164,6 +164,14 @@ describe('MDC-based MatTable', () => {
expect(tbody.textContent.trim()).toContain('No data');
});

it('should set the content styling class on the tbody', () => {
let fixture = TestBed.createComponent(NativeHtmlTableApp);
fixture.detectChanges();

const tbodyElement = fixture.nativeElement.querySelector('tbody');
expect(tbodyElement.classList).toContain('mdc-data-table__content');
});

});

it('should render with MatTableDataSource and sort', () => {
Expand Down Expand Up @@ -213,6 +221,13 @@ describe('MDC-based MatTable', () => {
}).not.toThrow();
}));

it('should be able to render a flexbox-based table', () => {
expect(() => {
const fixture = TestBed.createComponent(MatFlexTableApp);
fixture.detectChanges();
}).not.toThrow();
});

describe('with MatTableDataSource and sort/pagination/filter', () => {
let tableElement: HTMLElement;
let fixture: ComponentFixture<ArrayDataSourceMatTableApp>;
Expand Down Expand Up @@ -961,6 +976,45 @@ class TableWithNgContainerRow {
}


@Component({
template: `
<mat-table [dataSource]="dataSource">
<ng-container matColumnDef="column_a">
<mat-header-cell *matHeaderCellDef> Column A</mat-header-cell>
<mat-cell *matCellDef="let row"> {{row.a}}</mat-cell>
<mat-footer-cell *matFooterCellDef> Footer A</mat-footer-cell>
</ng-container>
<ng-container matColumnDef="column_b">
<mat-header-cell *matHeaderCellDef> Column B</mat-header-cell>
<mat-cell *matCellDef="let row"> {{row.b}}</mat-cell>
<mat-footer-cell *matFooterCellDef> Footer B</mat-footer-cell>
</ng-container>
<ng-container matColumnDef="column_c">
<mat-header-cell *matHeaderCellDef> Column C</mat-header-cell>
<mat-cell *matCellDef="let row"> {{row.c}}</mat-cell>
<mat-footer-cell *matFooterCellDef> Footer C</mat-footer-cell>
</ng-container>
<ng-container matColumnDef="special_column">
<mat-cell *matCellDef="let row"> fourth_row </mat-cell>
</ng-container>
<mat-header-row *matHeaderRowDef="columnsToRender"></mat-header-row>
<mat-row *matRowDef="let row; columns: columnsToRender"></mat-row>
<div *matNoDataRow>No data</div>
<mat-footer-row *matFooterRowDef="columnsToRender"></mat-footer-row>
</mat-table>
`
})
class MatFlexTableApp {
dataSource: FakeDataSource | null = new FakeDataSource();
columnsToRender = ['column_a', 'column_b', 'column_c'];
@ViewChild(MatTable) table: MatTable<TestData>;
}


function getElements(element: Element, query: string): Element[] {
return [].slice.call(element.querySelectorAll(query));
}
Expand Down
11 changes: 8 additions & 3 deletions src/material-experimental/mdc-table/table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,15 @@ export class MatTable<T> extends CdkTable<T> implements OnInit {
/** Overrides the need to add position: sticky on every sticky cell element in `CdkTable`. */
protected needsPositionStickyOnElement = false;

// After ngOnInit, the `CdkTable` has created and inserted the table sections (thead, tbody,
// tfoot). MDC requires the `mdc-data-table__content` class to be added to the body.
ngOnInit() {
super.ngOnInit();
this._elementRef.nativeElement.querySelector('tbody').classList.add('mdc-data-table__content');

// After ngOnInit, the `CdkTable` has created and inserted the table sections (thead, tbody,
// tfoot). MDC requires the `mdc-data-table__content` class to be added to the body. Note that
// this only applies to native tables, because we don't wrap the content of flexbox-based ones.
if (this._isNativeHtmlTable) {
const tbody = this._elementRef.nativeElement.querySelector('tbody');
tbody.classList.add('mdc-data-table__content');
}
}
}
1 change: 1 addition & 0 deletions tools/public_api_guard/cdk/table.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ export declare class CdkTable<T> implements AfterContentChecked, CollectionViewe
protected readonly _elementRef: ElementRef;
_footerRowOutlet: FooterRowOutlet;
_headerRowOutlet: HeaderRowOutlet;
protected _isNativeHtmlTable: boolean;
_multiTemplateDataRows: boolean;
_noDataRow: CdkNoDataRow;
_noDataRowOutlet: NoDataRowOutlet;
Expand Down

0 comments on commit aade5d1

Please sign in to comment.