Skip to content

Commit

Permalink
fix(toolbar): don't auto-generate first row if first row would be empty
Browse files Browse the repository at this point in the history
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 first toolbar row will be only auto-generated if there is user content that can be projected into the default first row. This means that developers can now do the following:

```html
<md-toolbar>
  <md-toolbar-row>First Row</md-toolbar-row>
  <md-toolbar-row>Second Row</md-toolbar-row>
</md-toolbar>
```

Fixes angular#6004. Fixes angular#1718.
  • Loading branch information
devversion committed Aug 27, 2017
1 parent ec4ea06 commit 6520e9b
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 39 deletions.
3 changes: 2 additions & 1 deletion src/lib/toolbar/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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],
})
Expand Down
2 changes: 1 addition & 1 deletion src/lib/toolbar/toolbar.html
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<div class="mat-toolbar-layout">
<md-toolbar-row>
<md-toolbar-row #defaultFirstRow>
<ng-content></ng-content>
</md-toolbar-row>
<ng-content select="md-toolbar-row, mat-toolbar-row"></ng-content>
Expand Down
13 changes: 13 additions & 0 deletions src/lib/toolbar/toolbar.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,19 @@ Each toolbar row is a `display: flex` container.
</md-toolbar>
```

In some situations, developers need full control over all `<md-toolbar-row>` elements
(e.g. for use with flex-layout).

To have full control over the first `<md-toolbar-row>`, which is usually auto-generated,
there should be only row elements in the `<md-toolbar>`.

```html
<md-toolbar>
<md-toolbar-row>First Row</md-toolbar-row>
<md-toolbar-row>Second Row</md-toolbar-row>
</md-toolbar>
```

### 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.
Expand Down
85 changes: 58 additions & 27 deletions src/lib/toolbar/toolbar.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,55 +3,86 @@ 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: [ToolbarWithDefaultFirstRow, ToolbarWithoutDefaultFirstRow],
});

TestBed.compileComponents();
}));

beforeEach(() => {
fixture = TestBed.createComponent(TestApp);
testComponent = fixture.debugElement.componentInstance;
toolbarElement = fixture.debugElement.query(By.css('md-toolbar')).nativeElement;
});
describe('with default first row', () => {
let fixture: ComponentFixture<ToolbarWithDefaultFirstRow>;
let testComponent: ToolbarWithDefaultFirstRow;
let toolbarElement: HTMLElement;
let toolbarRowElements: HTMLElement[];

beforeEach(() => {
fixture = TestBed.createComponent(ToolbarWithDefaultFirstRow);
testComponent = fixture.debugElement.componentInstance;
toolbarElement = fixture.debugElement.query(By.css('.mat-toolbar')).nativeElement;
toolbarRowElements = fixture.debugElement.queryAll(By.css('.mat-toolbar-row'))
.map(rowDebugElement => rowDebugElement.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 generate a default first row if content is projected', () => {
expect(toolbarRowElements.length)
.toBe(2, 'Expected a default first row and a second row to be present.');
});
});

it('should set the toolbar role on the host', () => {
expect(toolbarElement.getAttribute('role')).toBe('toolbar');
describe('without default first row', () => {

it('should not generate a default first row if no content is projected', () => {
const fixture = TestBed.createComponent(ToolbarWithoutDefaultFirstRow);

expect(fixture.debugElement.queryAll(By.css('.mat-toolbar-row')).length)
.toBe(2, 'Expected one toolbar row to be present while no content is projected.');
});
});


});


@Component({template: `<md-toolbar [color]="toolbarColor">Test Toolbar</md-toolbar>`})
class TestApp {
@Component({template: `
<md-toolbar [color]="toolbarColor">
First Row
<md-toolbar-row>Second Row</md-toolbar-row>
</md-toolbar>`})
class ToolbarWithDefaultFirstRow {
toolbarColor: string;
}

@Component({template: `
<md-toolbar [color]="toolbarColor">
<md-toolbar-row>First Row</md-toolbar-row>
</md-toolbar>
`})
class ToolbarWithoutDefaultFirstRow {}
40 changes: 30 additions & 10 deletions src/lib/toolbar/toolbar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,11 @@ import {
Directive,
ElementRef,
Renderer2,
ViewChild,
AfterViewInit,
} 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 */
Expand All @@ -30,7 +26,6 @@ export class MdToolbarBase {
}
export const _MdToolbarMixinBase = mixinColor(MdToolbarBase);


@Component({
moduleId: module.id,
selector: 'md-toolbar, mat-toolbar',
Expand All @@ -44,10 +39,35 @@ export const _MdToolbarMixinBase = mixinColor(MdToolbarBase);
changeDetection: ChangeDetectionStrategy.OnPush,
encapsulation: ViewEncapsulation.None
})
export class MdToolbar extends _MdToolbarMixinBase implements CanColor {
export class MdToolbar extends _MdToolbarMixinBase implements CanColor, AfterViewInit {

constructor(renderer: Renderer2, elementRef: ElementRef) {
/** Element reference to the default first row of the toolbar. */
@ViewChild('defaultFirstRow', { read: ElementRef }) _defaultFirstRow: ElementRef;

constructor(renderer: Renderer2, elementRef: ElementRef, private _platform: Platform) {
super(renderer, elementRef);
}

ngAfterViewInit() {
// Do nothing if we're not on the browser platform.
if (!this._platform.isBrowser) {
return;
}

const firstRowElement = this._defaultFirstRow.nativeElement;

// If the first toolbar row, which will be auto-generated, does not have any projected content,
// then the auto-generated first toolbar row should be removed. This gives developers
// full control over the first toolbar row (e.g. for use with flex-layout).
if (!firstRowElement.innerHTML.trim()) {
firstRowElement.parentNode.removeChild(firstRowElement);
}
}
}

@Directive({
selector: 'md-toolbar-row, mat-toolbar-row',
host: {'class': 'mat-toolbar-row'},
})
export class MdToolbarRow {}

0 comments on commit 6520e9b

Please sign in to comment.