feat(schematics): dashboard schematic #10011

amcdnl · 2018-02-18T17:23:30Z

This PR adds a dashboard schematic.

Can be viewed here: https://github.com/amcdnl/material-example-schematics/tree/schematic-dashboard

jelbourn · 2018-02-18T20:32:53Z

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

+  margin: 20px;
+}
+
+mat-card {


Why absolute positioning?

I couldn't come up with a good way to make the card 100% height/width with 15px margin. Open to ideas if you have any.

calc(100% - 15px)?

Results in this - https://www.screencast.com/t/Q2MpKLRi :(

jelbourn · 2018-02-18T20:33:54Z

...matics/dashboard/files/__path__/__name@dasherize@if-flat__/__name@dasherize__.component.html

@@ -0,0 +1,24 @@
+<div class="grid-container">
+  <h1 class="mat-h1">Dashboard</h1>
+  <mat-grid-list cols="2" rowHeight="350px">


Grid-list is only useful when you want items with heterogenous rowspans. We could either mix that in, or just use cards laid out with flex styles.

I'll update it to have spans :)

Are the rowspans meaningful here? A screenshot would be helpful

Ya - https://www.screencast.com/t/op5NHbKC

crisbeto · 2018-02-19T19:13:47Z

...matics/dashboard/files/__path__/__name@dasherize@if-flat__/__name@dasherize__.component.html

+          <mat-card-title>
+            {{card.title}}
+            <button mat-icon-button [matMenuTriggerFor]="menu">
+              <mat-icon aria-label="Menu icon">more_vert</mat-icon>


It might be more appropriate to have the aria-label be on the button. See the examples in menu-a11y.html.

crisbeto · 2018-02-19T19:14:17Z

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

+    .compileComponents();
+  }));
+
+  beforeEach(() => {


The two beforeEach calls can be combined.

This code was copied from the base component schematic.

crisbeto · 2018-02-19T19:14:32Z

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

+  let component: <%= classify(name) %>Component;
+  let fixture: ComponentFixture<<%= classify(name) %>Component>;
+
+  beforeEach(async(() => {


Use fakeAsync instead of async.

This code was copied from the base component schematic.

crisbeto · 2018-02-19T19:14:45Z

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

+  it('should create', () => {
+    expect(component).toBeTruthy();
+  });
+});


Missing a newline at the end of the file.

crisbeto · 2018-02-19T19:14:56Z

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

+    fixture.detectChanges();
+  });
+
+  it('should create', () => {


should create -> should compile

This code was copied from the base component schematic.

crisbeto · 2018-02-19T19:15:36Z

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

@@ -0,0 +1,74 @@
+import { Component, <% if(!!viewEncapsulation) { %>, ViewEncapsulation<% }%><% if(changeDetection !== 'Default') { %>, ChangeDetectionStrategy<% }%> } from '@angular/core';
+
+declare var google: any;


It doesn't look like this is being used anywhere.

crisbeto · 2018-02-19T19:16:10Z

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

+              <mat-card-title>
+                {{card.title}}
+                <button mat-icon-button [matMenuTriggerFor]="menu">
+                  <mat-icon aria-label="Menu icon">more_vert</mat-icon>


Same note as the other template. The aria-label should be on the button and be more descriptive.

crisbeto · 2018-02-19T19:17:40Z

schematics/dashboard/index_spec.ts

+    const tree = runner.runSchematic('materialDashboard', { ...options }, createTestApp());
+    const files = tree.files;
+
+    expect(files.indexOf('/src/app/foo/foo.component.css')).toBeGreaterThanOrEqual(0);


These assertions can be rewritten using toContain, e.g.

expect(files).toContain('/src/app/foo/foo.component.css');

crisbeto · 2018-02-19T19:18:17Z

schematics/dashboard/schema.json

+    },
+    "flat": {
+      "type": "boolean",
+      "description": "Flag to indicate if a dir is created.",


Unclear what dir is referring to.

This code was copied from the base component schematic.

jelbourn · 2018-03-06T00:37:20Z

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

+  margin: 20px;
+}
+
+mat-card {


calc(100% - 15px)?

jelbourn · 2018-03-06T00:38:34Z

...matics/dashboard/files/__path__/__name@dasherize@if-flat__/__name@dasherize__.component.html

@@ -0,0 +1,24 @@
+<div class="grid-container">
+  <h1 class="mat-h1">Dashboard</h1>
+  <mat-grid-list cols="2" rowHeight="350px">


Are the rowspans meaningful here? A screenshot would be helpful

jelbourn · 2018-03-06T00:42:18Z

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

+  margin: 20px;
+}
+
+mat-card {


Kind of nit-picky, but since this could set the pattern for a project I'll get into it:

I think each of these styles that currently target a mat element or attribute should be a css class instead. While those selectors are fine in this skeleton, I don't want to give people the impression that this is the way they should always style custom components. Introducing a separate css class signals the intent that you want to style a specific instance of a component rather than all instances of that component.

jelbourn · 2018-03-06T00:44:45Z

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

+  changeDetection: ChangeDetectionStrategy.<%= changeDetection %><% } %>
+})
+export class <%= classify(name) %>Component {
+  cards = [


Condense this?

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} ];

amcdnl · 2018-03-08T00:45:35Z

@jelbourn - Ready for review.

jelbourn

LGTM

jelbourn · 2018-03-08T01:13:00Z

@amcdnl just needs rebase

angular-automatic-lock-bot · 2019-09-08T12:02:07Z

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.}

feat(schematics): dashboard schematic

bac514c

amcdnl self-assigned this Feb 18, 2018

amcdnl requested a review from jelbourn February 18, 2018 17:23

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

jelbourn reviewed Feb 18, 2018

View reviewed changes

jelbourn requested a review from crisbeto February 18, 2018 20:35

crisbeto reviewed Feb 19, 2018

View reviewed changes

chore: nit

b3b3af2

jelbourn reviewed Mar 6, 2018

View reviewed changes

jelbourn mentioned this pull request Mar 6, 2018

feat(schematics): navigation schematic #10009

Merged

amcdnl added 3 commits March 7, 2018 17:16

Merge branch 'master' into dashboard-schematic

b7da5c4

chore: pr feedback

54e067b

Merge branch 'master' into dashboard-schematic

a46d848

jelbourn approved these changes Mar 8, 2018

View reviewed changes

jelbourn added pr: lgtm action: merge The PR is ready for merge by the caretaker target: major This PR is targeted for the next major release labels Mar 8, 2018

Merge branch 'master' into dashboard-schematic

98c1fe5

jelbourn merged commit 6273d6a into angular:master Mar 10, 2018

amcdnl deleted the dashboard-schematic branch March 12, 2018 21:56

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

		@@ -0,0 +1,74 @@
		import { Component, <% if(!!viewEncapsulation) { %>, ViewEncapsulation<% }%><% if(changeDetection !== 'Default') { %>, ChangeDetectionStrategy<% }%> } from '@angular/core';

		declare var google: any;

feat(schematics): dashboard schematic #10011

feat(schematics): dashboard schematic #10011

Uh oh!

Conversation

amcdnl commented Feb 18, 2018

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!

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!

amcdnl commented Mar 8, 2018

Uh oh!

jelbourn left a comment

Choose a reason for hiding this comment

Uh oh!

jelbourn commented Mar 8, 2018

Uh oh!

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

Uh oh!

Uh oh!