Skip to content

Schematic improvements #11191

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

Merged
merged 14 commits into from
Jun 11, 2018
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/lib/schematics/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,6 @@ A collection of Schematics for Angular Material.

## Collection
- [Shell](shell/README.md)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you change "Shell" to "Install"?

- [Dashboard](dashboard/README.md)
- [Table](table/README.md)
- [Nav](nav/README.md)
13 changes: 6 additions & 7 deletions src/lib/schematics/collection.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,27 +9,26 @@
"schema": "./shell/schema.json",
"aliases": ["material-shell"]
},

// Create a dashboard component
"materialDashboard": {
"dashboard": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should keep the old name as an alias

"description": "Create a card-based dashboard component",
"factory": "./dashboard/index",
"schema": "./dashboard/schema.json",
"aliases": [ "material-dashboard" ]
"aliases": []
},
// Creates a table component
"materialTable": {
"table": {
"description": "Create a component that displays data with a data-table",
"factory": "./table/index",
"schema": "./table/schema.json",
"aliases": [ "material-table" ]
"aliases": []
},
// Creates toolbar and navigation components
"materialNav": {
"nav": {
"description": "Create a component with a responsive sidenav for navigation",
"factory": "./nav/index",
"schema": "./nav/schema.json",
"aliases": [ "material-nav" ]
"aliases": []
}
}
}
5 changes: 5 additions & 0 deletions src/lib/schematics/dashboard/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Dashboard
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why create a separate file for each one?

Creates a responive card grid list component .
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra space before the dot.


Command: `ng g @angular/material:materialDashboard --name dashboard`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't it be ng g @angular/material:dashboard?


Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { Component<% if(!!viewEncapsulation) { %>, ViewEncapsulation<% }%><% if(changeDetection !== 'Default') { %>, ChangeDetectionStrategy<% }%> } from '@angular/core';
import { map } from 'rxjs/operators';
import { Breakpoints, BreakpointState, BreakpointObserver } from '@angular/cdk/layout';

@Component({
selector: '<%= selector %>',<% if(inlineTemplate) { %>
Expand Down Expand Up @@ -59,10 +61,26 @@ import { Component<% if(!!viewEncapsulation) { %>, ViewEncapsulation<% }%><% if(
changeDetection: ChangeDetectionStrategy.<%= changeDetection %><% } %>
})
export class <%= classify(name) %>Component {
cards = [
{ title: 'Card 1', cols: 2, rows: 1 },
{ title: 'Card 2', cols: 1, rows: 1 },
{ title: 'Card 3', cols: 1, rows: 2 },
{ title: 'Card 4', cols: 1, rows: 1 }
];
/** Based on the screen size, switch from standard to one column per row */
cards = this.breakpointObserver.observe(Breakpoints.Handset).pipe(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cols property of the grid-list itself should probably also change

map(({ matches }) => {
if (matches) {
return [
{ title: 'Card 1', cols: 2, rows: 1 },
{ title: 'Card 2', cols: 2, rows: 1 },
{ title: 'Card 3', cols: 2, rows: 1 },
{ title: 'Card 4', cols: 2, rows: 1 }
];
} else {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need the else here, you can just return at the bottom.

return [
{ title: 'Card 1', cols: 2, rows: 1 },
{ title: 'Card 2', cols: 1, rows: 1 },
{ title: 'Card 3', cols: 1, rows: 2 },
{ title: 'Card 4', cols: 1, rows: 1 }
];
}
})
);

constructor(private breakpointObserver: BreakpointObserver) {}
}
3 changes: 2 additions & 1 deletion src/lib/schematics/dashboard/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {addModuleImportToModule, findModuleFromOptions} from '../utils/ast';
import {buildComponent} from '../utils/devkit-utils/component';

/**
* Scaffolds a new navigation component.
* Scaffolds a new dashboard component.
* Internally it bootstraps the base component schematic
*/
export default function(options: Schema): Rule {
Expand All @@ -25,6 +25,7 @@ function addNavModulesToModule(options: Schema) {
addModuleImportToModule(host, modulePath, 'MatMenuModule', '@angular/material');
addModuleImportToModule(host, modulePath, 'MatIconModule', '@angular/material');
addModuleImportToModule(host, modulePath, 'MatButtonModule', '@angular/material');
addModuleImportToModule(host, modulePath, 'LayoutModule', '@angular/cdk/layout');
return host;
};
}
2 changes: 1 addition & 1 deletion src/lib/schematics/dashboard/index_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ describe('material-dashboard-schematic', () => {
});

it('should create dashboard files and add them to module', () => {
const tree = runner.runSchematic('materialDashboard', { ...options }, createTestApp());
const tree = runner.runSchematic('dashboard', { ...options }, createTestApp());
const files = tree.files;

expect(files).toContain('/src/app/foo/foo.component.css');
Expand Down
9 changes: 5 additions & 4 deletions src/lib/schematics/dashboard/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@
"project": {
"type": "string",
"description": "The name of the project.",
"visible": false
"visible": false,
"$default": {
"$source": "projectName"
}
},
"name": {
"type": "string",
Expand Down Expand Up @@ -90,7 +93,5 @@
"description": "Specifies if declaring module exports the component."
}
},
"required": [
"name"
]
"required": []
}
5 changes: 5 additions & 0 deletions src/lib/schematics/nav/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Nav
Creates a navigation component with responsive sidenav.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"with responsive" -> "with a responsive"


Command: `ng g @angular/material:materialNav --name nav`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be ng g @angular/material:nav?


Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,9 @@

.sidenav {
width: 200px;
box-shadow: 3px 0 6px rgba(0,0,0,.24);
}

.mat-toolbar.mat-primary {
position: sticky;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we really be using position: sticky? The browser support isn't great and fixed should be able to get the job done for an example.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We actually use position sticky on the material docs site. Supports seems decent just missing IE11.

top: 0;
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
*ngIf="(isHandset | async)!.matches">
<mat-icon aria-label="Side nav toggle icon">menu</mat-icon>
</button>
<span>Application Title</span>
<span><%= project %></span>
</mat-toolbar>
<!-- Add Content Here -->
</mat-sidenav-content>
</mat-sidenav-container>
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,9 @@ import { Observable } from 'rxjs';
*ngIf="(isHandset | async)!.matches">
<mat-icon aria-label="Side nav toggle icon">menu</mat-icon>
</button>
<span>Application Title</span>
<span><%= project %></span>
</mat-toolbar>
<!-- Add Content Here -->
</mat-sidenav-content>
</mat-sidenav-container>
`,<% } else { %>
Expand All @@ -44,7 +45,11 @@ import { Observable } from 'rxjs';

.sidenav {
width: 200px;
box-shadow: 3px 0 6px rgba(0,0,0,.24);
}

.mat-toolbar.mat-primary {
position: sticky;
top: 0;
}
`
]<% } else { %>
Expand Down
4 changes: 2 additions & 2 deletions src/lib/schematics/nav/index_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ describe('material-nav-schematic', () => {
});

it('should create nav files and add them to module', () => {
const tree = runner.runSchematic('materialNav', { ...options }, createTestApp());
const tree = runner.runSchematic('nav', { ...options }, createTestApp());
const files = tree.files;

expect(files).toContain('/src/app/foo/foo.component.css');
Expand All @@ -42,7 +42,7 @@ describe('material-nav-schematic', () => {
});

it('should add nav imports to module', () => {
const tree = runner.runSchematic('materialNav', { ...options }, createTestApp());
const tree = runner.runSchematic('nav', { ...options }, createTestApp());
const moduleContent = getFileContent(tree, '/src/app/app.module.ts');

expect(moduleContent).toContain('LayoutModule');
Expand Down
9 changes: 5 additions & 4 deletions src/lib/schematics/nav/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@
"project": {
"type": "string",
"description": "The name of the project.",
"visible": false
"visible": false,
"$default": {
"$source": "projectName"
}
},
"name": {
"type": "string",
Expand Down Expand Up @@ -90,7 +93,5 @@
"description": "Specifies if declaring module exports the component."
}
},
"required": [
"name"
]
"required": []
}
3 changes: 2 additions & 1 deletion src/lib/schematics/shell/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,5 @@ Adds Angular Material and its depedencies and pre-configures the application.
- Ensure `BrowserAnimationsModule` is installed and included in root module
- Adds pre-configured theme to `.angular-cli.json` file OR adds custom theme scaffolding to `styles.scss`

Command: `ng generate material-shell --collection=material-schematics`
Command: `ng add @angular/material`

6 changes: 3 additions & 3 deletions src/lib/schematics/shell/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {addModuleImportToRootModule, getStylesPath} from '../utils/ast';
import {InsertChange} from '../utils/devkit-utils/change';
import {getProjectFromWorkspace, getWorkspace} from '../utils/devkit-utils/config';
import {addHeadLink} from '../utils/html';
import {angularVersion, cdkVersion, materialVersion} from '../utils/lib-versions';
import {angularVersion, materialVersion} from '../utils/lib-versions';
import {addPackageToPackageJson} from '../utils/package';
import {Schema} from './schema';
import {addThemeToAppStyles} from './theming';
Expand All @@ -28,7 +28,7 @@ export default function(options: Schema): Rule {
/** Add material, cdk, annimations to package.json if not already present. */
function addMaterialToPackageJson() {
return (host: Tree) => {
addPackageToPackageJson(host, 'dependencies', '@angular/cdk', cdkVersion);
addPackageToPackageJson(host, 'dependencies', '@angular/cdk', materialVersion);
addPackageToPackageJson(host, 'dependencies', '@angular/material', materialVersion);
addPackageToPackageJson(host, 'dependencies', '@angular/animations', angularVersion);
return host;
Expand Down Expand Up @@ -78,7 +78,7 @@ function addBodyMarginToStyles(options: Schema) {
const buffer = host.read(stylesPath);
if (buffer) {
const src = buffer.toString();
const insertion = new InsertChange(stylesPath, src.length, `\nbody { margin: 0; }\n`);
const insertion = new InsertChange(stylesPath, src.length, `\nbody { margin: 0; font-family: 'Roboto', sans-serif; }\n`);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this should happen by default. The install schematic currently makes things available without changing the existing app, and I think it's best to preserve that. We can add something like a --typography option, though.

@crisbeto would setting .mat-body be the more appropriate thing to do here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have a mat-body, but setting mat-typography would propagate all the typography styles.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jelbourn - You told me to add this in the original issue.

We should add a style to make the font Roboto...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ha, I'm apparently fickle. Today I'm on board with it being there by default, but think we should do it with a css class rather than adding a new style on the body directly. I'm fine with that being in a follow-up PR, though.

const recorder = host.beginUpdate(stylesPath);
recorder.insertLeft(insertion.pos, insertion.toAdd);
host.commitUpdate(recorder);
Expand Down
8 changes: 4 additions & 4 deletions src/lib/schematics/shell/index_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@ describe('material-shell-schematic', () => {
});

it('should update package.json', () => {
const tree = runner.runSchematic('materialShell', {}, appTree);
const tree = runner.runSchematic('shell', {}, appTree);
const packageJson = JSON.parse(getFileContent(tree, '/package.json'));

expect(packageJson.dependencies['@angular/material']).toBeDefined();
expect(packageJson.dependencies['@angular/cdk']).toBeDefined();
});

it('should add default theme', () => {
const tree = runner.runSchematic('materialShell', {}, appTree);
const tree = runner.runSchematic('shell', {}, appTree);
const config: any = getConfig(tree);
config.apps.forEach(app => {
expect(app.styles).toContain(
Expand All @@ -37,7 +37,7 @@ describe('material-shell-schematic', () => {
});

it('should add custom theme', () => {
const tree = runner.runSchematic('materialShell', {
const tree = runner.runSchematic('shell', {
theme: 'custom'
}, appTree);

Expand All @@ -53,7 +53,7 @@ describe('material-shell-schematic', () => {
});

it('should add font links', () => {
const tree = runner.runSchematic('materialShell', {}, appTree);
const tree = runner.runSchematic('shell', {}, appTree);
const config: any = getConfig(tree);
const workspace = getWorkspace(tree);
const project = getProjectFromWorkspace(workspace, config.project.name);
Expand Down
7 changes: 3 additions & 4 deletions src/lib/schematics/table/README.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
# Table
Creates a table component with sorting and paginator.
# Table
Creates a table component with sorting and paginator.

Command: `ng generate material-table --collection=material-schematics`

Command: `ng g @angular/material:materialTable --name table`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't it be ng g @angular/material:table?

Original file line number Diff line number Diff line change
@@ -1,21 +1,20 @@
<div class="mat-elevation-z8">
<mat-table #table [dataSource]="dataSource" matSort aria-label="Elements">

<table mat-table #table [dataSource]="dataSource" matSort aria-label="Elements">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't @andrewseguin already change this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

He changed the one in the component, not the one in the html file.

<!-- 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>
<th mat-header-cell *matHeaderCellDef mat-sort-header>Id</th>
<td mat-cell *matCellDef="let row">{{row.id}}</td>
</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>
<th mat-header-cell *matHeaderCellDef mat-sort-header>Name</th>
<td mat-cell *matCellDef="let row">{{row.name}}</td>
</ng-container>

<mat-header-row *matHeaderRowDef="displayedColumns"></mat-header-row>
<mat-row *matRowDef="let row; columns: displayedColumns;"></mat-row>
</mat-table>
<tr mat-header-row *matHeaderRowDef="displayedColumns"></tr>
<tr mat-row *matRowDef="let row; columns: displayedColumns;"></tr>
</table>

<mat-paginator #paginator
[length]="dataSource.data.length"
Expand Down
4 changes: 2 additions & 2 deletions src/lib/schematics/table/index_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ describe('material-table-schematic', () => {
});

it('should create table files and add them to module', () => {
const tree = runner.runSchematic('materialTable', { ...options }, createTestApp());
const tree = runner.runSchematic('table', { ...options }, createTestApp());
const files = tree.files;

expect(files).toContain('/src/app/foo/foo.component.css');
Expand All @@ -50,7 +50,7 @@ describe('material-table-schematic', () => {
});

it('should add table imports to module', () => {
const tree = runner.runSchematic('materialTable', { ...options }, createTestApp());
const tree = runner.runSchematic('table', { ...options }, createTestApp());
const moduleContent = getFileContent(tree, '/src/app/app.module.ts');

expect(moduleContent).toContain('MatTableModule');
Expand Down
9 changes: 5 additions & 4 deletions src/lib/schematics/table/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@
"project": {
"type": "string",
"description": "The name of the project.",
"visible": false
"visible": false,
"$default": {
"$source": "projectName"
}
},
"name": {
"type": "string",
Expand Down Expand Up @@ -90,7 +93,5 @@
"description": "Specifies if declaring module exports the component."
}
},
"required": [
"name"
]
"required": []
}
1 change: 0 additions & 1 deletion src/lib/schematics/utils/lib-versions.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
export const materialVersion = '^6.0.0';
export const cdkVersion = '^6.0.0';
export const angularVersion = '^6.0.0';