fix(toolbar): no longer auto-generate toolbar rows

Currently the toolbar always generates the first `<md-toolbar-row>`. This means that developers have no opportunity to set directives/attributes/classes on the first toolbar row (e.g with flex-layout). With this change, the toolbar won't auto-generate any `<md-toolbar-row>` element. The toolbar will have two different row modes: _Single row toolbar_ ```html <md-toolbar> First Tow </md-toolbar> ``` _Multiple rows toolbar_ ```html <md-toolbar> <md-toolbar-row>First Row</md-toolbar-row> <md-toolbar-row>Second Row</md-toolbar-row> </md-toolbar> ``` This means that mixing those two row modes is no longer possible and allowed ```html <md-toolbar> <span>First Row</span> <md-toolbar-row>Second Row</md-toolbar-row> </md-toolbar> ``` Fixes angular#6004. Fixes angular#1718.
devversion · Sep 1, 2017 · 564d41b · 564d41b
1 parent 1b6b270
commit 564d41b
Show file tree

Hide file tree

Showing 8 changed files with 172 additions and 80 deletions.
diff --git a/src/demo-app/toolbar/toolbar-demo.html b/src/demo-app/toolbar/toolbar-demo.html
@@ -35,27 +35,27 @@
 
   <p>
     <md-toolbar color="accent">
-      <span>Custom Toolbar</span>
-      <md-toolbar-row>
-        <span>Second Line</span>
-      </md-toolbar-row>
+      <md-toolbar-row>First Row</md-toolbar-row>
+      <md-toolbar-row>Second Row</md-toolbar-row>
     </md-toolbar>
   </p>
 
   <p>
     <md-toolbar color="primary">
-      <span>Custom Toolbar</span>
+      <md-toolbar-row>
+        <span>First Row</span>
+      </md-toolbar-row>
 
       <md-toolbar-row>
-        <span>Second Line</span>
+        <span>Second Row</span>
 
         <span class="demo-fill-remaining"></span>
 
         <md-icon class="demo-toolbar-icon">verified_user</md-icon>
       </md-toolbar-row>
 
       <md-toolbar-row>
-        <span>Third Line</span>
+        <span>Third Row</span>
 
         <span class="demo-fill-remaining"></span>
 
@@ -64,5 +64,4 @@
       </md-toolbar-row>
     </md-toolbar>
   </p>
-
-</div>
+</div>
diff --git a/src/lib/toolbar/index.ts b/src/lib/toolbar/index.ts
@@ -7,12 +7,13 @@
  */
 
 import {NgModule} from '@angular/core';
+import {PlatformModule} from '@angular/cdk/platform';
 import {MdCommonModule} from '../core';
 import {MdToolbar, MdToolbarRow} from './toolbar';
 
 
 @NgModule({
-  imports: [MdCommonModule],
+  imports: [MdCommonModule, PlatformModule],
   exports: [MdToolbar, MdToolbarRow, MdCommonModule],
   declarations: [MdToolbar, MdToolbarRow],
 })

diff --git a/src/lib/toolbar/toolbar.html b/src/lib/toolbar/toolbar.html
@@ -1,6 +1,2 @@
-<div class="mat-toolbar-layout">
-  <md-toolbar-row>
-    <ng-content></ng-content>
-  </md-toolbar-row>
-  <ng-content select="md-toolbar-row, mat-toolbar-row"></ng-content>
-</div>
+<ng-content></ng-content>
+<ng-content select="md-toolbar-row, mat-toolbar-row"></ng-content>
diff --git a/src/lib/toolbar/toolbar.md b/src/lib/toolbar/toolbar.md
@@ -2,25 +2,38 @@
 
 <!-- example(toolbar-overview) -->
 
-### Multiple rows
-Toolbars can have multiple rows using `<md-toolbar-row>` elements. Any content outside of an 
-`<md-toolbar-row>` element are automatically placed inside of one at the beginning of the toolbar.
-Each toolbar row is a `display: flex` container.
+### Single row
+
+In the most situations, a toolbar will be placed at the top of your application and will only 
+have a single row that includes the title of your application.
 
 ```html
 <md-toolbar>
-  <span>First Row</span>
-
+  <span>My Application</span>
+</md-toolbar>
+```
+
+### Multiple rows
+
+The Material Design specifications describe that toolbars can also have multiple rows. Creating
+toolbars with multiple rows in Angular Material can be done by placing `<md-toolbar-row>` elements
+inside of a `<md-toolbar>`.
+
+```html
+<md-toolbar>  
   <md-toolbar-row>
-    <span>Second Row</span>
+    <span>First Row</span>
   </md-toolbar-row>
 
   <md-toolbar-row>
-    <span>Third Row</span>
+    <span>Second Row</span>
   </md-toolbar-row>
 </md-toolbar>
 ```
 
+**Note**: Placing content outside of a `<md-toolbar-row>` when multiple rows are specified, is not
+supported.
+
 ### Positioning toolbar content
 The toolbar does not perform any positioning of its content. This gives the user full power to 
 position the content as it suits their application.

diff --git a/src/lib/toolbar/toolbar.scss b/src/lib/toolbar/toolbar.scss
@@ -4,39 +4,39 @@ $mat-toolbar-height-desktop: 64px !default;
 $mat-toolbar-height-mobile-portrait: 56px !default;
 $mat-toolbar-height-mobile-landscape: 48px !default;
 
-$mat-toolbar-padding: 16px !default;
+$mat-toolbar-row-padding: 16px !default;
 
 
 @mixin mat-toolbar-height($height) {
-  .mat-toolbar {
+  .mat-toolbar-multiple-rows {
     min-height: $height;
   }
-  .mat-toolbar-row {
+  .mat-toolbar-row, .mat-toolbar-single-row {
     height: $height;
   }
 }
 
-.mat-toolbar {
+.mat-toolbar-row, .mat-toolbar-single-row {
   display: flex;
   box-sizing: border-box;
-  width: 100%;
-  padding: 0 $mat-toolbar-padding;
-  flex-direction: column;
 
-  .mat-toolbar-row {
-    display: flex;
-    box-sizing: border-box;
+  padding: 0 $mat-toolbar-row-padding;
+  width: 100%;
 
-    width: 100%;
+  // Flexbox Vertical Alignment
+  flex-direction: row;
+  align-items: center;
 
-    // Flexbox Vertical Alignment
-    flex-direction: row;
-    align-items: center;
+  // Per Material specs a toolbar cannot have multiple lines inside of a single row.
+  // Disable text wrapping inside of the toolbar. Developers are still able to overwrite it.
+  white-space: nowrap;
+}
 
-    // Per Material specs a toolbar cannot have multiple lines inside of a single row.
-    // Disable text wrapping inside of the toolbar. Developers are still able to overwrite it.
-    white-space: nowrap;
-  }
+.mat-toolbar-multiple-rows {
+  display: flex;
+  box-sizing: border-box;
+  flex-direction: column;
+  width: 100%;
 }
 
 // Set the default height for the toolbar.

diff --git a/src/lib/toolbar/toolbar.spec.ts b/src/lib/toolbar/toolbar.spec.ts
@@ -3,55 +3,105 @@ import {TestBed, async, ComponentFixture} from '@angular/core/testing';
 import {By} from '@angular/platform-browser';
 import {MdToolbarModule} from './index';
 
-
 describe('MdToolbar', () => {
 
-  let fixture: ComponentFixture<TestApp>;
-  let testComponent: TestApp;
-  let toolbarElement: HTMLElement;
-
   beforeEach(async(() => {
     TestBed.configureTestingModule({
       imports: [MdToolbarModule],
-      declarations: [TestApp],
+      declarations: [ToolbarSingleRow, ToolbarMultipleRows, ToolbarMixedRowModes],
     });
 
     TestBed.compileComponents();
   }));
 
-  beforeEach(() => {
-    fixture = TestBed.createComponent(TestApp);
-    testComponent = fixture.debugElement.componentInstance;
-    toolbarElement = fixture.debugElement.query(By.css('md-toolbar')).nativeElement;
-  });
+  describe('with single row', () => {
+    let fixture: ComponentFixture<ToolbarSingleRow>;
+    let testComponent: ToolbarSingleRow;
+    let toolbarElement: HTMLElement;
+
+    beforeEach(() => {
+      fixture = TestBed.createComponent(ToolbarSingleRow);
+      testComponent = fixture.debugElement.componentInstance;
+      toolbarElement = fixture.debugElement.query(By.css('.mat-toolbar')).nativeElement;
+    });
+
+    it('should apply class based on color attribute', () => {
+      testComponent.toolbarColor = 'primary';
+      fixture.detectChanges();
 
-  it('should apply class based on color attribute', () => {
-    testComponent.toolbarColor = 'primary';
-    fixture.detectChanges();
+      expect(toolbarElement.classList.contains('mat-primary')).toBe(true);
 
-    expect(toolbarElement.classList.contains('mat-primary')).toBe(true);
+      testComponent.toolbarColor = 'accent';
+      fixture.detectChanges();
 
-    testComponent.toolbarColor = 'accent';
-    fixture.detectChanges();
+      expect(toolbarElement.classList.contains('mat-primary')).toBe(false);
+      expect(toolbarElement.classList.contains('mat-accent')).toBe(true);
 
-    expect(toolbarElement.classList.contains('mat-primary')).toBe(false);
-    expect(toolbarElement.classList.contains('mat-accent')).toBe(true);
+      testComponent.toolbarColor = 'warn';
+      fixture.detectChanges();
+
+      expect(toolbarElement.classList.contains('mat-accent')).toBe(false);
+      expect(toolbarElement.classList.contains('mat-warn')).toBe(true);
+    });
 
-    testComponent.toolbarColor = 'warn';
-    fixture.detectChanges();
+    it('should set the toolbar role on the host', () => {
+      expect(toolbarElement.getAttribute('role')).toBe('toolbar');
+    });
 
-    expect(toolbarElement.classList.contains('mat-accent')).toBe(false);
-    expect(toolbarElement.classList.contains('mat-warn')).toBe(true);
+    it('should not wrap the first row contents inside of a generated element', () => {
+      expect(toolbarElement.firstElementChild!.tagName).toBe('SPAN',
+          'Expected the <span> element of the first row to be a direct child of the toolbar');
+    });
   });
 
-  it('should set the toolbar role on the host', () => {
-    expect(toolbarElement.getAttribute('role')).toBe('toolbar');
+  describe('with multiple rows', () => {
+
+    it('should project each toolbar-row element inside of the toolbar', () => {
+      const fixture = TestBed.createComponent(ToolbarMultipleRows);
+      fixture.detectChanges();
+
+      expect(fixture.debugElement.queryAll(By.css('.mat-toolbar > .mat-toolbar-row')).length)
+        .toBe(2, 'Expected one toolbar row to be present while no content is projected.');
+    });
+
+    it('should throw an error if different toolbar modes are mixed', () => {
+      expect(() => {
+        const fixture = TestBed.createComponent(ToolbarMixedRowModes);
+        fixture.detectChanges();
+      }).toThrowError(/attempting to combine different/i);
+    });
   });
 
 });
 
 
-@Component({template: `<md-toolbar [color]="toolbarColor">Test Toolbar</md-toolbar>`})
-class TestApp {
+@Component({
+  template: `
+    <md-toolbar [color]="toolbarColor">
+      <span>First Row</span>
+    </md-toolbar>
+  `
+})
+class ToolbarSingleRow {
   toolbarColor: string;
 }
+
+@Component({
+  template: `
+    <md-toolbar>
+      <md-toolbar-row>First Row</md-toolbar-row>
+      <md-toolbar-row>Second Row</md-toolbar-row>
+    </md-toolbar>
+  `
+})
+class ToolbarMultipleRows {}
+
+@Component({
+  template: `
+    <md-toolbar>
+      First Row
+      <md-toolbar-row>Second Row</md-toolbar-row>
+    </md-toolbar>
+  `
+})
+class ToolbarMixedRowModes {}
diff --git a/src/lib/toolbar/toolbar.ts b/src/lib/toolbar/toolbar.ts
@@ -13,15 +13,13 @@ import {
   Directive,
   ElementRef,
   Renderer2,
+  ContentChildren,
+  QueryList,
+  AfterViewInit,
+  isDevMode,
 } from '@angular/core';
 import {CanColor, mixinColor} from '../core/common-behaviors/color';
-
-
-@Directive({
-  selector: 'md-toolbar-row, mat-toolbar-row',
-  host: {'class': 'mat-toolbar-row'},
-})
-export class MdToolbarRow {}
+import {Platform} from '@angular/cdk/platform';
 
 // Boilerplate for applying mixins to MdToolbar.
 /** @docs-private */
@@ -30,6 +28,11 @@ export class MdToolbarBase {
 }
 export const _MdToolbarMixinBase = mixinColor(MdToolbarBase);
 
+@Directive({
+  selector: 'md-toolbar-row, mat-toolbar-row',
+  host: {'class': 'mat-toolbar-row'},
+})
+export class MdToolbarRow {}
 
 @Component({
   moduleId: module.id,
@@ -39,15 +42,43 @@ export const _MdToolbarMixinBase = mixinColor(MdToolbarBase);
   inputs: ['color'],
   host: {
     'class': 'mat-toolbar',
-    'role': 'toolbar'
+    'role': 'toolbar',
+    '[class.mat-toolbar-multiple-rows]': 'this._toolbarRows.length',
+    '[class.mat-toolbar-single-row]': '!this._toolbarRows.length'
   },
   changeDetection: ChangeDetectionStrategy.OnPush,
   encapsulation: ViewEncapsulation.None
 })
-export class MdToolbar extends _MdToolbarMixinBase implements CanColor {
+export class MdToolbar extends _MdToolbarMixinBase implements CanColor, AfterViewInit {
+
+  /** Reference to all toolbar row elements that have been projected. */
+  @ContentChildren(MdToolbarRow) _toolbarRows: QueryList<MdToolbarRow>;
 
-  constructor(renderer: Renderer2, elementRef: ElementRef) {
+  constructor(renderer: Renderer2, elementRef: ElementRef, private _platform: Platform) {
     super(renderer, elementRef);
   }
 
+  ngAfterViewInit() {
+    if (!isDevMode() || !this._platform.isBrowser || !this._toolbarRows.length) {
+      return;
+    }
+
+    const isCombinedUsage = [].slice.call(this._elementRef.nativeElement.childNodes)
+      .filter(node => !(node.classList && node.classList.contains('mat-toolbar-row')))
+      .some(node => node.textContent.trim());
+
+    if (isCombinedUsage) {
+      throwToolbarMixedModesError();
+    }
+  }
+}
+
+/**
+ * Throws an exception when attempting to combine the different toolbar row modes.
+ * @docs-private
+ */
+export function throwToolbarMixedModesError() {
+  throw Error('MdToolbar: Attempting to combine different toolbar modes. ' +
+    'Either specify multiple `<md-toolbar-row>` elements explicitly or just place content ' +
+    'inside of a `<md-toolbar>` for a single row.');
 }
diff --git a/src/material-examples/toolbar-multirow/toolbar-multirow-example.html b/src/material-examples/toolbar-multirow/toolbar-multirow-example.html
@@ -1,5 +1,7 @@
 <md-toolbar color="primary">
-  <span>Custom Toolbar</span>
+  <md-toolbar-row>
+    <span>Custom Toolbar</span>
+  </md-toolbar-row>
 
   <md-toolbar-row>
     <span>Second Line</span>