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> ``` BREAKING CHANGE: `<md-toolbar-row>` elements will be no longer auto-generated for the first row. Meaning that custom styling for the first `<md-toolbar-row>` is no longer working as before. Since no toolbar-row is auto-generated anymore, there are two different modes for the toolbar now. Either place content directly inside of the `<md-toolbar>` or place multiple `<md-toolbar-row>` elements. Fixes #6004. Fixes #1718.
angular · Sep 29, 2017 · ccc2608 · ccc2608
1 parent 53c42a4
commit ccc2608
Show file tree

Hide file tree

Showing 8 changed files with 197 additions and 82 deletions.
diff --git a/src/demo-app/toolbar/toolbar-demo.html b/src/demo-app/toolbar/toolbar-demo.html
@@ -35,27 +35,27 @@
 
   <p>
     <mat-toolbar color="accent">
-      <span>Custom Toolbar</span>
-      <mat-toolbar-row>
-        <span>Second Line</span>
-      </mat-toolbar-row>
+      <mat-toolbar-row>First Row</mat-toolbar-row>
+      <mat-toolbar-row>Second Row</mat-toolbar-row>
     </mat-toolbar>
   </p>
 
   <p>
     <mat-toolbar color="primary">
-      <span>Custom Toolbar</span>
+      <mat-toolbar-row>
+        <span>First Row</span>
+      </mat-toolbar-row>
 
       <mat-toolbar-row>
-        <span>Second Line</span>
+        <span>Second Row</span>
 
         <span class="demo-fill-remaining"></span>
 
         <mat-icon class="demo-toolbar-icon">verified_user</mat-icon>
       </mat-toolbar-row>
 
       <mat-toolbar-row>
-        <span>Third Line</span>
+        <span>Third Row</span>
 
         <span class="demo-fill-remaining"></span>
 
@@ -64,5 +64,4 @@
       </mat-toolbar-row>
     </mat-toolbar>
   </p>
-
-</div>
+</div>
diff --git a/src/lib/toolbar/toolbar-module.ts b/src/lib/toolbar/toolbar-module.ts
@@ -8,11 +8,11 @@
 
 import {NgModule} from '@angular/core';
 import {MatCommonModule} from '@angular/material/core';
+import {PlatformModule} from '@angular/cdk/platform';
 import {MatToolbar, MatToolbarRow} from './toolbar';
 
-
 @NgModule({
-  imports: [MatCommonModule],
+  imports: [MatCommonModule, PlatformModule],
   exports: [MatToolbar, MatToolbarRow, MatCommonModule],
   declarations: [MatToolbar, MatToolbarRow],
 })

diff --git a/src/lib/toolbar/toolbar.html b/src/lib/toolbar/toolbar.html
@@ -1,6 +1,2 @@
-<div class="mat-toolbar-layout">
-  <mat-toolbar-row>
-    <ng-content></ng-content>
-  </mat-toolbar-row>
-  <ng-content select="mat-toolbar-row"></ng-content>
-</div>
+<ng-content></ng-content>
+<ng-content select="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 `<mat-toolbar-row>` elements. Any content outside of an 
-`<mat-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
 <mat-toolbar>
-  <span>First Row</span>
-
+  <span>My Application</span>
+</mat-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 `<mat-toolbar-row>` elements
+inside of a `<mat-toolbar>`.
+
+```html
+<mat-toolbar>  
   <mat-toolbar-row>
-    <span>Second Row</span>
+    <span>First Row</span>
   </mat-toolbar-row>
 
   <mat-toolbar-row>
-    <span>Third Row</span>
+    <span>Second Row</span>
   </mat-toolbar-row>
 </mat-toolbar>
 ```
 
+**Note**: Placing content outside of a `<mat-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,115 @@ import {TestBed, async, ComponentFixture} from '@angular/core/testing';
 import {By} from '@angular/platform-browser';
 import {MatToolbarModule} from './index';
 
-
 describe('MatToolbar', () => {
 
-  let fixture: ComponentFixture<TestApp>;
-  let testComponent: TestApp;
-  let toolbarElement: HTMLElement;
-
   beforeEach(async(() => {
     TestBed.configureTestingModule({
       imports: [MatToolbarModule],
-      declarations: [TestApp],
+      declarations: [ToolbarSingleRow, ToolbarMultipleRows, ToolbarMixedRowModes],
     });
 
     TestBed.compileComponents();
   }));
 
-  beforeEach(() => {
-    fixture = TestBed.createComponent(TestApp);
-    testComponent = fixture.debugElement.componentInstance;
-    toolbarElement = fixture.debugElement.query(By.css('mat-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();
+      testComponent.toolbarColor = 'warn';
+      fixture.detectChanges();
 
-    expect(toolbarElement.classList.contains('mat-accent')).toBe(false);
-    expect(toolbarElement.classList.contains('mat-warn')).toBe(true);
+      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);
+    });
+
+    it('should throw an error if a toolbar-row is added later', () => {
+      const fixture = TestBed.createComponent(ToolbarMixedRowModes);
+
+      fixture.componentInstance.showToolbarRow = false;
+      fixture.detectChanges();
+
+      expect(() => {
+        fixture.componentInstance.showToolbarRow = true;
+        fixture.detectChanges();
+      }).toThrowError(/attempting to combine different/i);
+    });
   });
 
 });
 
 
-@Component({template: `<mat-toolbar [color]="toolbarColor">Test Toolbar</mat-toolbar>`})
-class TestApp {
+@Component({
+  template: `
+    <mat-toolbar [color]="toolbarColor">
+      <span>First Row</span>
+    </mat-toolbar>
+  `
+})
+class ToolbarSingleRow {
   toolbarColor: string;
 }
+
+@Component({
+  template: `
+    <mat-toolbar>
+      <mat-toolbar-row>First Row</mat-toolbar-row>
+      <mat-toolbar-row>Second Row</mat-toolbar-row>
+    </mat-toolbar>
+  `
+})
+class ToolbarMultipleRows {}
+
+@Component({
+  template: `
+    <mat-toolbar>
+      First Row
+      <mat-toolbar-row *ngIf="showToolbarRow">Second Row</mat-toolbar-row>
+    </mat-toolbar>
+  `
+})
+class ToolbarMixedRowModes {
+  showToolbarRow: boolean = true;
+}