Schematic improvements #11191

amcdnl · 2018-05-06T18:44:07Z

Addresses #10833

jelbourn

Could you squash your commits into one with a single description that describes the changes made?

jelbourn · 2018-05-07T16:40:06Z

src/lib/schematics/README.md

@@ -3,3 +3,6 @@ A collection of Schematics for Angular Material.

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


Can you change "Shell" to "Install"?

jelbourn · 2018-05-07T16:40:29Z

src/lib/schematics/collection.json

    // Create a dashboard component
-    "materialDashboard": {
+    "dashboard": {


We should keep the old name as an alias

jelbourn · 2018-05-07T16:40:56Z

src/lib/schematics/dashboard/README.md

@@ -0,0 +1,5 @@
+ # Dashboard


Why create a separate file for each one?

jelbourn · 2018-05-07T16:42:38Z

...hematics/dashboard/files/__path__/__name@dasherize@if-flat__/__name@dasherize__.component.ts

-    { 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(


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

jelbourn · 2018-05-07T16:44:34Z

src/lib/schematics/shell/index.ts

@@ -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`);


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?

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

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

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

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.

jelbourn · 2018-05-07T16:55:31Z

...schematics/table/files/__path__/__name@dasherize@if-flat__/__name@dasherize__.component.html

@@ -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">


Didn't @andrewseguin already change this?

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

crisbeto · 2018-05-07T21:44:15Z

src/lib/schematics/dashboard/README.md

+ # Dashboard
+ Creates a responive card grid list component .
+
+ Command: `ng g @angular/material:materialDashboard --name dashboard`


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

crisbeto · 2018-05-07T21:44:26Z

src/lib/schematics/dashboard/README.md

@@ -0,0 +1,5 @@
+ # Dashboard
+ Creates a responive card grid list component .


Extra space before the dot.

crisbeto · 2018-05-07T21:45:32Z

...hematics/dashboard/files/__path__/__name@dasherize@if-flat__/__name@dasherize__.component.ts

+          { title: 'Card 3', cols: 2, rows: 1 },
+          { title: 'Card 4', cols: 2, rows: 1 }
+        ];
+      } else {


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

crisbeto · 2018-05-07T21:48:01Z

src/lib/schematics/nav/README.md

+ # Nav
+ Creates a navigation component with responsive sidenav.
+
+ Command: `ng g @angular/material:materialNav --name nav`


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

crisbeto · 2018-05-07T21:48:19Z

src/lib/schematics/nav/README.md

@@ -0,0 +1,5 @@
+ # Nav
+ Creates a navigation component with responsive sidenav.


"with responsive" -> "with a responsive"

crisbeto · 2018-05-07T21:49:22Z

...tics/nav/files/__path__/__name@dasherize@if-flat__/__name@dasherize__.component.__styleext__

+}
+
+.mat-toolbar.mat-primary {
+  position: sticky;


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.

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

crisbeto · 2018-05-07T21:50:04Z

src/lib/schematics/table/README.md

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

crisbeto · 2018-05-07T21:52:53Z

Also needs a rebase.

ngbot · 2018-05-08T16:51:45Z

Hi @amcdnl! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

amcdnl

Addressed feedback

amcdnl · 2018-05-20T14:43:36Z

...schematics/table/files/__path__/__name@dasherize@if-flat__/__name@dasherize__.component.html

@@ -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">


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

amcdnl · 2018-05-20T14:57:00Z

...tics/nav/files/__path__/__name@dasherize@if-flat__/__name@dasherize__.component.__styleext__

+}
+
+.mat-toolbar.mat-primary {
+  position: sticky;


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

amcdnl · 2018-05-20T14:58:06Z

src/lib/schematics/shell/index.ts

@@ -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`);


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

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

# Conflicts: # src/lib/schematics/install/index.ts

ngbot · 2018-05-22T16:23:48Z

Hi @amcdnl! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

jelbourn

LGTM, just needs rebase

ngbot · 2018-05-22T23:54:14Z

Hi @amcdnl! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

jelbourn · 2018-06-04T16:38:36Z

CI is all red now

amcdnl · 2018-06-10T16:52:45Z

@jelbourn Fixed.

jelbourn

LGTM

angular-automatic-lock-bot · 2019-09-09T02:03:21Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

amcdnl added 7 commits May 6, 2018 12:08

fix: nav schematic fixes

111826c

feat(schematics): add responsive dashboard

e35554b

fix(schematics): move table to table elm

08174db

chore: add docs

cf0fc06

chore(schematics): add better naming

46bddb4

feat(schematics): update to use first arg as name

320c2ec

chore(schematics): remove old aliases

eace06d

amcdnl self-assigned this May 6, 2018

amcdnl requested a review from jelbourn May 6, 2018 18:44

googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label May 6, 2018

jelbourn reviewed May 7, 2018

View reviewed changes

crisbeto reviewed May 7, 2018

View reviewed changes

amcdnl commented May 20, 2018

View reviewed changes

amcdnl added 2 commits May 20, 2018 09:58

chore: pr feedback

1eca79b

Merge remote-tracking branch 'origin/master' into schematic-improvements

2acce79

# Conflicts: # src/lib/schematics/install/index.ts

jelbourn approved these changes May 22, 2018

View reviewed changes

jelbourn added pr: lgtm target: patch This PR is targeted for the next patch release labels May 22, 2018

amcdnl added 3 commits June 4, 2018 10:28

Merge branch 'master' into schematic-improvements

366f42d

chore: fix typo

2f7c1aa

fix(schematics): fix angular#11521

5237987

amcdnl mentioned this pull request Jun 4, 2018

Failed to create Navigation Schematic #11521

Closed

amcdnl added 2 commits June 10, 2018 11:11

Merge remote-tracking branch 'origin/master' into schematic-improvements

5fe8986

chore: fix build

c9296c7

jelbourn removed the pr: needs rebase label Jun 11, 2018

jelbourn approved these changes Jun 11, 2018

View reviewed changes

jelbourn added action: merge The PR is ready for merge by the caretaker pr: merge safe labels Jun 11, 2018

jelbourn merged commit 6bd2775 into angular:master Jun 11, 2018

amcdnl deleted the schematic-improvements branch June 12, 2018 13:45

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 9, 2019

		@@ -3,3 +3,6 @@ A collection of Schematics for Angular Material.

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

		@@ -0,0 +1,5 @@
		# Dashboard
		Creates a responive card grid list component .

		@@ -0,0 +1,5 @@
		# Nav
		Creates a navigation component with responsive sidenav.

Schematic improvements #11191

Schematic improvements #11191

Uh oh!

Conversation

amcdnl commented May 6, 2018

Uh oh!

jelbourn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

crisbeto commented May 7, 2018

Uh oh!

ngbot bot commented May 8, 2018

Uh oh!

amcdnl left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ngbot bot commented May 22, 2018

Uh oh!

jelbourn left a comment

Choose a reason for hiding this comment

Uh oh!

ngbot bot commented May 22, 2018

Uh oh!

jelbourn commented Jun 4, 2018

Uh oh!

amcdnl commented Jun 10, 2018

Uh oh!

jelbourn left a comment

Choose a reason for hiding this comment

Uh oh!

angular-automatic-lock-bot bot commented Sep 9, 2019

Uh oh!

Uh oh!