From 1e900fd76365914bf0a95819a1d5fe3a6cef827d Mon Sep 17 00:00:00 2001 From: Andrew Seguin Date: Tue, 6 Jun 2017 12:00:27 -0700 Subject: [PATCH 1/2] feat(data-table): allow custom roles --- src/lib/core/data-table/data-table.spec.ts | 28 +++++++++++++++++++++- src/lib/core/data-table/data-table.ts | 15 +++++++++--- src/lib/core/testing/jasmine-matchers.ts | 6 +++-- 3 files changed, 43 insertions(+), 6 deletions(-) diff --git a/src/lib/core/data-table/data-table.spec.ts b/src/lib/core/data-table/data-table.spec.ts index b463faa77a05..361773b8fb8e 100644 --- a/src/lib/core/data-table/data-table.spec.ts +++ b/src/lib/core/data-table/data-table.spec.ts @@ -21,7 +21,7 @@ describe('CdkTable', () => { TestBed.configureTestingModule({ imports: [CdkDataTableModule], - declarations: [SimpleCdkTableApp], + declarations: [SimpleCdkTableApp, CustomRoleCdkTableApp], }).compileComponents(); fixture = TestBed.createComponent(SimpleCdkTableApp); @@ -81,6 +81,12 @@ describe('CdkTable', () => { expect(row).toBeRole('row'); getCells(row).forEach(cell => expect(cell).toBeRole('gridcell')); }); + + // Check that if the component was created with a custom role, that it does not override it. + fixture = TestBed.createComponent(CustomRoleCdkTableApp); + fixture.detectChanges(); + + expect(fixture.nativeElement.querySelector('cdk-table')).toBeRole('custom-role'); }); }); @@ -255,6 +261,26 @@ class SimpleCdkTableApp { @ViewChild(CdkTable) table: CdkTable; } +@Component({ + template: ` + + + Column A + {{row.a}} + + + + + + ` +}) +class CustomRoleCdkTableApp { + dataSource: FakeDataSource = new FakeDataSource(); + columnsToRender = ['column_a']; + + @ViewChild(CdkTable) table: CdkTable; +} + function getElements(element: Element, query: string): Element[] { return [].slice.call(element.querySelectorAll(query)); } diff --git a/src/lib/core/data-table/data-table.ts b/src/lib/core/data-table/data-table.ts index fbe26f7b0b2c..8d8e76537d9a 100644 --- a/src/lib/core/data-table/data-table.ts +++ b/src/lib/core/data-table/data-table.ts @@ -1,16 +1,19 @@ import { + Attribute, ChangeDetectionStrategy, ChangeDetectorRef, Component, ContentChild, ContentChildren, Directive, + ElementRef, Input, IterableChangeRecord, IterableDiffer, IterableDiffers, NgIterable, QueryList, + Renderer2, ViewChild, ViewContainerRef, ViewEncapsulation @@ -54,7 +57,6 @@ export class HeaderRowPlaceholder { `, host: { 'class': 'cdk-table', - 'role': 'grid' // TODO(andrewseguin): Allow the user to choose either grid or treegrid }, encapsulation: ViewEncapsulation.None, changeDetection: ChangeDetectionStrategy.OnPush, @@ -104,12 +106,19 @@ export class CdkTable implements CollectionViewer { @ContentChildren(CdkRowDef) _rowDefinitions: QueryList; constructor(private readonly _differs: IterableDiffers, - private readonly _changeDetectorRef: ChangeDetectorRef) { + private readonly _changeDetectorRef: ChangeDetectorRef, + elementRef: ElementRef, + renderer: Renderer2, + @Attribute('role') role: string) { // Show the stability warning of the data-table only if it doesn't run inside of jasmine. // This is just temporary and should reduce warnings when running the tests. if (!(typeof window !== 'undefined' && window['jasmine'])) { console.warn('The data table is still in active development ' + - 'and should be considered unstable.'); + 'and should be considered unstable.'); + } + + if (!role) { + renderer.setAttribute(elementRef.nativeElement, 'role', 'grid'); } // TODO(andrewseguin): Add trackby function input. diff --git a/src/lib/core/testing/jasmine-matchers.ts b/src/lib/core/testing/jasmine-matchers.ts index 037cfcf7a5db..953a1e09bdea 100644 --- a/src/lib/core/testing/jasmine-matchers.ts +++ b/src/lib/core/testing/jasmine-matchers.ts @@ -7,11 +7,13 @@ export const customMatchers: jasmine.CustomMatcherFactories = { return { compare: function (element: Element, expectedRole: string) { const result: jasmine.CustomMatcherResult = {pass: false}; - result.pass = element.getAttribute('role') === expectedRole; + const actualRole = element.getAttribute('role'); + + result.pass = actualRole === expectedRole; result.message = `Expected role for ${element.tagName} to be ${expectedRole}`; if (!result.pass) { - result.message += ` but was ${expectedRole}`; + result.message += ` but was ${actualRole}`; } return result; From 98f4932c890fce9e93ce712282bc3c66536bba74 Mon Sep 17 00:00:00 2001 From: Andrew Seguin Date: Tue, 6 Jun 2017 12:33:18 -0700 Subject: [PATCH 2/2] change to treegrid --- src/lib/core/data-table/data-table.spec.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/lib/core/data-table/data-table.spec.ts b/src/lib/core/data-table/data-table.spec.ts index 361773b8fb8e..c229db3deda7 100644 --- a/src/lib/core/data-table/data-table.spec.ts +++ b/src/lib/core/data-table/data-table.spec.ts @@ -81,13 +81,14 @@ describe('CdkTable', () => { expect(row).toBeRole('row'); getCells(row).forEach(cell => expect(cell).toBeRole('gridcell')); }); + }); + }); - // Check that if the component was created with a custom role, that it does not override it. - fixture = TestBed.createComponent(CustomRoleCdkTableApp); - fixture.detectChanges(); + it('should not clobber an existing table role', () => { + fixture = TestBed.createComponent(CustomRoleCdkTableApp); + fixture.detectChanges(); - expect(fixture.nativeElement.querySelector('cdk-table')).toBeRole('custom-role'); - }); + expect(fixture.nativeElement.querySelector('cdk-table')).toBeRole('treegrid'); }); it('should re-render the rows when the data changes', () => { @@ -263,7 +264,7 @@ class SimpleCdkTableApp { @Component({ template: ` - + Column A {{row.a}}