-
Notifications
You must be signed in to change notification settings - Fork 6.8k
feat(schematics): table schematic #10012
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
Changes from 3 commits
de5177d
4fe9892
13fbd00
4801ed9
42b2923
dbaf5a9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
# Table | ||
Creates a table component with sorting and paginator. | ||
|
||
Command: `ng generate material-table --collection=material-schematics` | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
<div class="mat-elevation-z8"> | ||
<mat-table #table [dataSource]="dataSource" matSort aria-label="Elements"> | ||
|
||
<!-- Id Column --> | ||
<ng-container matColumnDef="id"> | ||
<mat-header-cell *matHeaderCellDef mat-sort-header>Id</mat-header-cell> | ||
<mat-cell *matCellDef="let row">{{row.id}}</mat-cell> | ||
</ng-container> | ||
|
||
<!-- Name Column --> | ||
<ng-container matColumnDef="name"> | ||
<mat-header-cell *matHeaderCellDef mat-sort-header>Name</mat-header-cell> | ||
<mat-cell *matCellDef="let row">{{row.name}}</mat-cell> | ||
</ng-container> | ||
|
||
<mat-header-row *matHeaderRowDef="displayedColumns"></mat-header-row> | ||
<mat-row *matRowDef="let row; columns: displayedColumns;"></mat-row> | ||
</mat-table> | ||
|
||
<mat-paginator #paginator | ||
[length]="dataSource.data.length" | ||
[pageIndex]="0" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The length should come from the data, no? |
||
[pageSize]="10" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Compromise on 50? 10 is just too small to ever be realistic. |
||
[pageSizeOptions]="[25, 50, 100, 250]"> | ||
</mat-paginator> | ||
</div> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
|
||
import { fakeAsync, ComponentFixture, TestBed } from '@angular/core/testing'; | ||
|
||
import { <%= classify(name) %>Component } from './<%= dasherize(name) %>.component'; | ||
|
||
describe('<%= classify(name) %>Component', () => { | ||
let component: <%= classify(name) %>Component; | ||
let fixture: ComponentFixture<<%= classify(name) %>Component>; | ||
|
||
beforeEach(fakeAsync(() => { | ||
TestBed.configureTestingModule({ | ||
declarations: [ <%= classify(name) %>Component ] | ||
}) | ||
.compileComponents(); | ||
|
||
fixture = TestBed.createComponent(<%= classify(name) %>Component); | ||
component = fixture.componentInstance; | ||
fixture.detectChanges(); | ||
}); | ||
|
||
it('should compile', () => { | ||
expect(component).toBeTruthy(); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,131 @@ | ||
import { Component, OnInit, ViewChild, <% if(!!viewEncapsulation) { %>, ViewEncapsulation<% }%><% if(changeDetection !== 'Default') { %>, ChangeDetectionStrategy<% }%> } from '@angular/core'; | ||
import { Observable } from 'rxjs/Observable'; | ||
import { map } from 'rxjs/operators/map'; | ||
import { MatPaginator, MatSort } from '@angular/material'; | ||
import { DataSource } from '@angular/cdk/collections'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we have the spaces around the curly braces here? We don't have them in our source. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The default schematics have them so I'd tend to stay the same for consistency. @jelbourn - what do u think? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, we should be consistent with the rest of the CLI even if spaces in braces is nonsense. |
||
import { of } from 'rxjs/observable/of'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you expand on this comment? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. import {of as observableOf} from 'rxjs/observable/of'; Since |
||
import { merge } from 'rxjs/observable/merge'; | ||
|
||
/** Todo: Replace this with your own data model type */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
export interface DataItem { | ||
name: string; | ||
id: number; | ||
} | ||
|
||
function compare(a, b, isAsc) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can this name be more specific? /** Simple sort comparator for example ID/Name columns. */ I would also move this to the bottom of the file. |
||
return (a < b ? -1 : 1) * (isAsc ? 1 : -1); | ||
} | ||
|
||
export class ExampleDataSource extends DataSource<any> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The data source should go in its own file. I'd like this data source to play a big role in guiding users in structuring their data access and manipulation. It would be great if we could add an option for a client data-source vs. a server data source. The client one would perform everything locally and synchronously, while the server one would call out to (fake) server methods and do thing asynchronously. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of "Example", could we use the name of the page that's being generated? |
||
data: DataItem[] = [ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would move this array to be a constant at the top like
|
||
{id: 1, name: 'Hydrogen'}, | ||
{id: 2, name: 'Helium'}, | ||
{id: 3, name: 'Lithium'}, | ||
{id: 4, name: 'Beryllium'}, | ||
{id: 5, name: 'Boron'}, | ||
{id: 6, name: 'Carbon'}, | ||
{id: 7, name: 'Nitrogen'}, | ||
{id: 8, name: 'Oxygen'}, | ||
{id: 9, name: 'Fluorine'}, | ||
{id: 10, name: 'Neon'}, | ||
{id: 11, name: 'Sodium'}, | ||
{id: 12, name: 'Magnesium'}, | ||
{id: 13, name: 'Aluminum'}, | ||
{id: 14, name: 'Silicon'}, | ||
{id: 15, name: 'Phosphorus'}, | ||
{id: 16, name: 'Sulfur'}, | ||
{id: 17, name: 'Chlorine'}, | ||
{id: 18, name: 'Argon'}, | ||
{id: 19, name: 'Potassium'}, | ||
{id: 20, name: 'Calcium'}, | ||
]; | ||
|
||
constructor(private _paginator: MatPaginator, private _sort: MatSort) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've been thinking about this, and I'm thinking it would be good to decouple the data source and the sorter/paginator. This would look something like the data source having its own sort and pagination state, and then using the outputs on the directives to update the data source state. My main reason for wanting to do this is to not make users think they must use those components. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ya, I can see that but it wouldn't be near as clean then. What about a comment instead? Best way I could think of would be to:
|
||
super(); | ||
} | ||
|
||
connect(): Observable<Element[]> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a method description like /**
* Connect this data source to the table. The table will only update when
* the returned stream emits new items.
* @returns A stream of the items to be rendered.
*/ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Return value should be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did one better and did: |
||
// Combine everything that affects the rendered data into one update | ||
// stream for the data-table to consume. | ||
const dataMutations = [ | ||
of(this.data), | ||
this._paginator.page, | ||
this._sort.sortChange | ||
]; | ||
|
||
return merge(...dataMutations).pipe(map(() => { | ||
return this.getPagedData(this.getSortedData(this.data)); | ||
})); | ||
} | ||
|
||
disconnect() {} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe there should be an example of what cleaning up here would look like, or a comment that says something like "Don't forget to clean up!". There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a comment inside the empty // TODO: clean up any open connections, free any held resources, etc. |
||
|
||
private getPagedData(data) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add method description |
||
const startIndex = this._paginator.pageIndex * this._paginator.pageSize; | ||
return data.splice(startIndex, this._paginator.pageSize); | ||
} | ||
|
||
private getSortedData(data) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add method description |
||
if (!this._sort.active || this._sort.direction === '') { | ||
return data; | ||
} | ||
|
||
return data.sort((a, b) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: missing an extra space above the |
||
const isAsc = this._sort.direction == 'asc'; | ||
switch (this._sort.active) { | ||
case 'name': return compare(a.name, b.name, isAsc); | ||
case 'id': return compare(+a.id, +b.id, isAsc); | ||
default: return 0; | ||
} | ||
}); | ||
} | ||
} | ||
|
||
@Component({ | ||
selector: '<%= selector %>',<% if(inlineTemplate) { %> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @hansl are there any plans to make it easier to do templateUrl vs inline template without duplicating? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It could be done manually, ie. put the template content in a variable that’s used both here and in the separate file. Read that value from a file on disk. There’s no silver bullet for this one. |
||
template: ` | ||
<div class="mat-elevation-z8"> | ||
<mat-table #table [dataSource]="dataSource" matSort aria-label="Elements"> | ||
|
||
<!-- Id Column --> | ||
<ng-container matColumnDef="id"> | ||
<mat-header-cell *matHeaderCellDef mat-sort-header>Id</mat-header-cell> | ||
<mat-cell *matCellDef="let row">{{row.id}}</mat-cell> | ||
</ng-container> | ||
|
||
<!-- Name Column --> | ||
<ng-container matColumnDef="name"> | ||
<mat-header-cell *matHeaderCellDef mat-sort-header>Name</mat-header-cell> | ||
<mat-cell *matCellDef="let row">{{row.name}}</mat-cell> | ||
</ng-container> | ||
|
||
<mat-header-row *matHeaderRowDef="displayedColumns"></mat-header-row> | ||
<mat-row *matRowDef="let row; columns: displayedColumns;"></mat-row> | ||
</mat-table> | ||
|
||
<mat-paginator #paginator | ||
[length]="dataSource.data.length" | ||
[pageIndex]="0" | ||
[pageSize]="10" | ||
[pageSizeOptions]="[25, 50, 100, 250]"> | ||
</mat-paginator> | ||
</div> | ||
`,<% } else { %> | ||
templateUrl: './<%= dasherize(name) %>.component.html',<% } if(inlineStyle) { %> | ||
styles: []<% } else { %> | ||
styleUrls: ['./<%= dasherize(name) %>.component.<%= styleext %>']<% } %><% if(!!viewEncapsulation) { %>, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't write this, I copied the code from the default component template for this section. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doesn't mean it shouldn't be corrected. |
||
encapsulation: ViewEncapsulation.<%= viewEncapsulation %><% } if (changeDetection !== 'Default') { %>, | ||
changeDetection: ChangeDetectionStrategy.<%= changeDetection %><% } %> | ||
}) | ||
export class <%= classify(name) %>Component implements OnInit { | ||
@ViewChild(MatPaginator) paginator: MatPaginator; | ||
@ViewChild(MatSort) sort: MatSort; | ||
dataSource: ExampleDataSource; | ||
|
||
/** Columns displayed in the table. Columns IDs can be added, removed, or reordered. */ | ||
displayedColumns = ['id', 'name']; | ||
|
||
ngOnInit() { | ||
this.dataSource = new ExampleDataSource(this.paginator, this.sort); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
import {chain, Rule, noop, Tree, SchematicContext} from '@angular-devkit/schematics'; | ||
import {Schema} from './schema'; | ||
import {addModuleImportToModule} from '../utils/ast'; | ||
import {findModuleFromOptions} from '../utils/devkit-utils/find-module'; | ||
import {buildComponent} from '../utils/devkit-utils/component'; | ||
|
||
/** | ||
* Scaffolds a new table component. | ||
* Internally it bootstraps the base component schematic | ||
*/ | ||
export default function(options: Schema): Rule { | ||
return chain([ | ||
buildComponent({...options}), | ||
options.skipImport ? noop() : addTableModulesToModule(options) | ||
]); | ||
} | ||
|
||
/** | ||
* Adds the required modules to the relative module. | ||
*/ | ||
function addTableModulesToModule(options: Schema) { | ||
return (host: Tree) => { | ||
const modulePath = findModuleFromOptions(host, options); | ||
addModuleImportToModule(host, modulePath, 'MatTableModule', '@angular/material'); | ||
addModuleImportToModule(host, modulePath, 'MatPaginatorModule', '@angular/material'); | ||
addModuleImportToModule(host, modulePath, 'MatSortModule', '@angular/material'); | ||
return host; | ||
}; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
import {SchematicTestRunner} from '@angular-devkit/schematics/testing'; | ||
import {join} from 'path'; | ||
import {Tree} from '@angular-devkit/schematics'; | ||
import {createTestApp} from '../utils/testing'; | ||
import {getFileContent} from '@schematics/angular/utility/test'; | ||
|
||
const collectionPath = join(__dirname, '../collection.json'); | ||
|
||
describe('material-table-schematic', () => { | ||
let runner: SchematicTestRunner; | ||
const options = { | ||
name: 'foo', | ||
path: 'app', | ||
sourceDir: 'src', | ||
inlineStyle: false, | ||
inlineTemplate: false, | ||
changeDetection: 'Default', | ||
styleext: 'css', | ||
spec: true, | ||
module: undefined, | ||
export: false, | ||
prefix: undefined, | ||
viewEncapsulation: undefined, | ||
}; | ||
|
||
beforeEach(() => { | ||
runner = new SchematicTestRunner('schematics', collectionPath); | ||
}); | ||
|
||
it('should create table files and add them to module', () => { | ||
const tree = runner.runSchematic('materialTable', { ...options }, createTestApp()); | ||
const files = tree.files; | ||
|
||
expect(files).toContain('/src/app/foo/foo.component.css'); | ||
expect(files).toContain('/src/app/foo/foo.component.html'); | ||
expect(files).toContain('/src/app/foo/foo.component.spec.ts'); | ||
expect(files).toContain('/src/app/foo/foo.component.ts'); | ||
|
||
const moduleContent = getFileContent(tree, '/src/app/app.module.ts'); | ||
expect(moduleContent).toMatch(/import.*Foo.*from '.\/foo\/foo.component'/); | ||
expect(moduleContent).toMatch(/declarations:\s*\[[^\]]+?,\r?\n\s+FooComponent\r?\n/m); | ||
}); | ||
|
||
it('should add table imports to module', () => { | ||
const tree = runner.runSchematic('materialTable', { ...options }, createTestApp()); | ||
const moduleContent = getFileContent(tree, '/src/app/app.module.ts'); | ||
|
||
expect(moduleContent).toContain('MatTableModule'); | ||
expect(moduleContent).toContain('MatPaginatorModule'); | ||
expect(moduleContent).toContain('MatSortModule'); | ||
|
||
expect(moduleContent).toContain( | ||
`import { MatTableModule, MatPaginatorModule, MatSortModule } from '@angular/material';`); | ||
}); | ||
|
||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
import {Schema as ComponentSchema} from '@schematics/angular/component/schema'; | ||
|
||
export interface Schema extends ComponentSchema {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,98 @@ | ||
{ | ||
"$schema": "http://json-schema.org/schema", | ||
"id": "SchematicsMaterialTable", | ||
"title": "Material Table Options Schema", | ||
"type": "object", | ||
"properties": { | ||
"path": { | ||
"type": "string", | ||
"description": "The path to create the component.", | ||
"default": "app", | ||
"visible": false | ||
}, | ||
"sourceDir": { | ||
"type": "string", | ||
"description": "The path of the source directory.", | ||
"default": "src", | ||
"alias": "sd", | ||
"visible": false | ||
}, | ||
"appRoot": { | ||
"type": "string", | ||
"description": "The root of the application.", | ||
"visible": false | ||
}, | ||
"name": { | ||
"type": "string", | ||
"description": "The name of the component." | ||
}, | ||
"inlineStyle": { | ||
"description": "Specifies if the style will be in the ts file.", | ||
"type": "boolean", | ||
"default": false, | ||
"alias": "is" | ||
}, | ||
"inlineTemplate": { | ||
"description": "Specifies if the template will be in the ts file.", | ||
"type": "boolean", | ||
"default": false, | ||
"alias": "it" | ||
}, | ||
"viewEncapsulation": { | ||
"description": "Specifies the view encapsulation strategy.", | ||
"enum": ["Emulated", "Native", "None"], | ||
"type": "string", | ||
"alias": "ve" | ||
}, | ||
"changeDetection": { | ||
"description": "Specifies the change detection strategy.", | ||
"enum": ["Default", "OnPush"], | ||
"type": "string", | ||
"default": "Default", | ||
"alias": "cd" | ||
}, | ||
"prefix": { | ||
"type": "string", | ||
"description": "The prefix to apply to generated selectors.", | ||
"default": "app", | ||
"alias": "p" | ||
}, | ||
"styleext": { | ||
"description": "The file extension to be used for style files.", | ||
"type": "string", | ||
"default": "css" | ||
}, | ||
"spec": { | ||
"type": "boolean", | ||
"description": "Specifies if a spec file is generated.", | ||
"default": true | ||
}, | ||
"flat": { | ||
"type": "boolean", | ||
"description": "Flag to indicate if a dir is created.", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think it's clear what a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was copied from the base component schematic. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doesn't mean we can't make it better for ourselves There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree but I wanted to keep this as close to the original as possible so when they support JSON schema inheritance, we can just do away with this file completely. |
||
"default": false | ||
}, | ||
"skipImport": { | ||
"type": "boolean", | ||
"description": "Flag to skip the module import.", | ||
"default": false | ||
}, | ||
"selector": { | ||
"type": "string", | ||
"description": "The selector to use for the component." | ||
}, | ||
"module": { | ||
"type": "string", | ||
"description": "Allows specification of the declaring module.", | ||
"alias": "m" | ||
}, | ||
"export": { | ||
"type": "boolean", | ||
"default": false, | ||
"description": "Specifies if declaring module exports the component." | ||
} | ||
}, | ||
"required": [ | ||
"name" | ||
] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make it easier for users to know what can be renamed (e.g.
row
)? Perhaps calling the dataelement
instead ofrow
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like
row
better because its a bit more generic. First thing I'm gonna do when I download this is rename that, if its row I'm less likely to want to do this.