From caee86ae58790a69ba62816f1c0bdff4f5d454bb Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Wed, 6 Sep 2017 12:42:34 +0200 Subject: [PATCH] fix(toolbar): no longer auto-generate toolbar rows Currently the toolbar always generates the first ``. 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 `` element. The toolbar will have two different row modes: _Single row toolbar_ ```html First Tow ``` _Multiple rows toolbar_ ```html First Row Second Row ``` This means that mixing those two row modes is no longer possible and allowed ```html First Row Second Row ``` BREAKING CHANGE: `` elements will be no longer auto-generated for the first row. Meaning that custom styling for the first `` 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 `` or place multiple `` elements. Fixes #6004. Fixes #1718. --- src/demo-app/toolbar/toolbar-demo.html | 17 ++- src/lib/toolbar/toolbar-module.ts | 4 +- src/lib/toolbar/toolbar.html | 8 +- src/lib/toolbar/toolbar.md | 29 +++-- src/lib/toolbar/toolbar.scss | 36 +++--- src/lib/toolbar/toolbar.spec.ts | 114 +++++++++++++----- src/lib/toolbar/toolbar.ts | 65 ++++++++-- .../toolbar-multirow-example.html | 4 +- 8 files changed, 196 insertions(+), 81 deletions(-) diff --git a/src/demo-app/toolbar/toolbar-demo.html b/src/demo-app/toolbar/toolbar-demo.html index e6d3f30c5172..8a4cf6147de4 100644 --- a/src/demo-app/toolbar/toolbar-demo.html +++ b/src/demo-app/toolbar/toolbar-demo.html @@ -35,19 +35,19 @@

- Custom Toolbar - - Second Line - + First Row + Second Row

- Custom Toolbar + + First Row + - Second Line + Second Row @@ -55,7 +55,7 @@ - Third Line + Third Row @@ -64,5 +64,4 @@

- - \ No newline at end of file + diff --git a/src/lib/toolbar/toolbar-module.ts b/src/lib/toolbar/toolbar-module.ts index b4eee3b54a58..b3bdfab1ad2f 100644 --- a/src/lib/toolbar/toolbar-module.ts +++ b/src/lib/toolbar/toolbar-module.ts @@ -8,11 +8,11 @@ import {NgModule} from '@angular/core'; import {MdCommonModule} from '@angular/material/core'; +import {PlatformModule} from '@angular/cdk/platform'; 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 index 88eed7dbff08..1cde768ee658 100644 --- a/src/lib/toolbar/toolbar.html +++ b/src/lib/toolbar/toolbar.html @@ -1,6 +1,2 @@ -
- - - - -
+ + diff --git a/src/lib/toolbar/toolbar.md b/src/lib/toolbar/toolbar.md index cfc2879b0c17..fc6e66a31906 100644 --- a/src/lib/toolbar/toolbar.md +++ b/src/lib/toolbar/toolbar.md @@ -2,25 +2,38 @@ -### Multiple rows -Toolbars can have multiple rows using `` elements. Any content outside of an -`` 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 - First Row - + My Application + +``` + +### 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 `` elements +inside of a ``. + +```html + - Second Row + First Row - Third Row + Second Row ``` +**Note**: Placing content outside of a `` 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 index 87becbfa16e3..e6b4cb21650b 100644 --- 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 index 6e0c6d4da7cd..fdfaa2089654 100644 --- 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 {MdToolbarModule} from './index'; - describe('MdToolbar', () => { - let fixture: ComponentFixture; - 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; + 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 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: `Test Toolbar`}) -class TestApp { +@Component({ + template: ` + + First Row + + ` +}) +class ToolbarSingleRow { toolbarColor: string; } + +@Component({ + template: ` + + First Row + Second Row + + ` +}) +class ToolbarMultipleRows {} + +@Component({ + template: ` + + First Row + Second Row + + ` +}) +class ToolbarMixedRowModes { + showToolbarRow: boolean = true; +} diff --git a/src/lib/toolbar/toolbar.ts b/src/lib/toolbar/toolbar.ts index 42c852af0460..37d495257fc8 100644 --- 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, MATERIAL_COMPATIBILITY_MODE, mixinColor} from '@angular/material/core'; - - -@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,17 +42,59 @@ export const _MdToolbarMixinBase = mixinColor(MdToolbarBase); inputs: ['color'], host: { 'class': 'mat-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, preserveWhitespaces: false, viewProviders: [{provide: MATERIAL_COMPATIBILITY_MODE, useValue: true}], }) -export class MdToolbar extends _MdToolbarMixinBase implements CanColor { +export class MdToolbar extends _MdToolbarMixinBase implements CanColor, AfterViewInit { - constructor(renderer: Renderer2, elementRef: ElementRef) { + /** Reference to all toolbar row elements that have been projected. */ + @ContentChildren(MdToolbarRow) _toolbarRows: QueryList; + + constructor(renderer: Renderer2, elementRef: ElementRef, private _platform: Platform) { super(renderer, elementRef); } + ngAfterViewInit() { + if (!isDevMode() || !this._platform.isBrowser) { + return; + } + + this._checkToolbarMixedModes(); + this._toolbarRows.changes.subscribe(() => this._checkToolbarMixedModes()); + } + + /** + * Throws an exception when developers are attempting to combine the different toolbar row modes. + */ + private _checkToolbarMixedModes() { + if (!this._toolbarRows.length) { + return; + } + + // Check if there are any other DOM nodes that can display content but aren't inside of + // a element. + const isCombinedUsage = [].slice.call(this._elementRef.nativeElement.childNodes) + .filter(node => !(node.classList && node.classList.contains('mat-toolbar-row'))) + .filter(node => node.nodeType !== Node.COMMENT_NODE) + .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 `` elements explicitly or just place content ' + + 'inside of a `` 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 index 9b178a21249f..49f9a7a96f23 100644 --- a/src/material-examples/toolbar-multirow/toolbar-multirow-example.html +++ b/src/material-examples/toolbar-multirow/toolbar-multirow-example.html @@ -1,5 +1,7 @@ - Custom Toolbar + + Custom Toolbar + Second Line