Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(universal): gate several browser-specific bits on being on the browser #4251

Merged
merged 3 commits into from
May 11, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/lib/button-toggle/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,20 @@ import {MdButtonToggleGroup, MdButtonToggleGroupMultiple, MdButtonToggle} from '
import {
UNIQUE_SELECTION_DISPATCHER_PROVIDER,
MdCommonModule,
FocusOriginMonitor,
StyleModule,
} from '../core';


@NgModule({
imports: [FormsModule, MdCommonModule],
imports: [FormsModule, MdCommonModule, StyleModule],
exports: [
MdButtonToggleGroup,
MdButtonToggleGroupMultiple,
MdButtonToggle,
MdCommonModule,
],
declarations: [MdButtonToggleGroup, MdButtonToggleGroupMultiple, MdButtonToggle],
providers: [UNIQUE_SELECTION_DISPATCHER_PROVIDER, FocusOriginMonitor]
providers: [UNIQUE_SELECTION_DISPATCHER_PROVIDER]
})
export class MdButtonToggleModule {}

Expand Down
44 changes: 24 additions & 20 deletions src/lib/button/button.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
Renderer2,
ViewEncapsulation
} from '@angular/core';
import {coerceBooleanProperty, FocusOriginMonitor} from '../core';
import {coerceBooleanProperty, FocusOriginMonitor, Platform} from '../core';
import {mixinDisabled, CanDisable} from '../core/common-behaviors/disabled';


Expand All @@ -22,9 +22,7 @@ import {mixinDisabled, CanDisable} from '../core/common-behaviors/disabled';
*/
@Directive({
selector: 'button[md-button], button[mat-button], a[md-button], a[mat-button]',
host: {
'[class.mat-button]': 'true'
}
host: {'class': 'mat-button'}
})
export class MdButtonCssMatStyler {}

Expand All @@ -36,9 +34,7 @@ export class MdButtonCssMatStyler {}
selector:
'button[md-raised-button], button[mat-raised-button], ' +
'a[md-raised-button], a[mat-raised-button]',
host: {
'[class.mat-raised-button]': 'true'
}
host: {'class': 'mat-raised-button'}
})
export class MdRaisedButtonCssMatStyler {}

Expand All @@ -49,9 +45,7 @@ export class MdRaisedButtonCssMatStyler {}
@Directive({
selector:
'button[md-icon-button], button[mat-icon-button], a[md-icon-button], a[mat-icon-button]',
host: {
'[class.mat-icon-button]': 'true',
}
host: {'class': 'mat-icon-button'}
})
export class MdIconButtonCssMatStyler {}

Expand All @@ -61,9 +55,7 @@ export class MdIconButtonCssMatStyler {}
*/
@Directive({
selector: 'button[md-fab], button[mat-fab], a[md-fab], a[mat-fab]',
host: {
'[class.mat-fab]': 'true'
}
host: {'class': 'mat-fab'}
})
export class MdFabCssMatStyler {}

Expand All @@ -73,9 +65,7 @@ export class MdFabCssMatStyler {}
*/
@Directive({
selector: 'button[md-mini-fab], button[mat-mini-fab], a[md-mini-fab], a[mat-mini-fab]',
host: {
'[class.mat-mini-fab]': 'true'
}
host: {'class': 'mat-mini-fab'}
})
export class MdMiniFabCssMatStyler {}

Expand Down Expand Up @@ -120,8 +110,11 @@ export class MdButton extends _MdButtonMixinBase implements OnDestroy, CanDisabl
get disableRipple() { return this._disableRipple; }
set disableRipple(v) { this._disableRipple = coerceBooleanProperty(v); }

constructor(private _elementRef: ElementRef, private _renderer: Renderer2,
private _focusOriginMonitor: FocusOriginMonitor) {
constructor(
private _elementRef: ElementRef,
private _renderer: Renderer2,
private _platform: Platform,
private _focusOriginMonitor: FocusOriginMonitor) {
super();
this._focusOriginMonitor.monitor(this._elementRef.nativeElement, this._renderer, true);
}
Expand Down Expand Up @@ -169,6 +162,13 @@ export class MdButton extends _MdButtonMixinBase implements OnDestroy, CanDisabl
* with either an 'md-' or 'mat-' prefix.
*/
_hasAttributeWithPrefix(...unprefixedAttributeNames: string[]) {
// If not on the browser, say that there are none of the attributes present.
// Since these only affect how the ripple displays (and ripples only happen on the client),
// detecting these attributes isn't necessary when not on the browser.
if (!this._platform.isBrowser) {
return false;
}

return unprefixedAttributeNames.some(suffix => {
const el = this._getHostElement();

Expand All @@ -195,8 +195,12 @@ export class MdButton extends _MdButtonMixinBase implements OnDestroy, CanDisabl
encapsulation: ViewEncapsulation.None
})
export class MdAnchor extends MdButton {
constructor(elementRef: ElementRef, renderer: Renderer2, focusOriginMonitor: FocusOriginMonitor) {
super(elementRef, renderer, focusOriginMonitor);
constructor(
elementRef: ElementRef,
renderer: Renderer2,
platform: Platform,
focusOriginMonitor: FocusOriginMonitor) {
super(elementRef, renderer, platform, focusOriginMonitor);
}

/** @docs-private */
Expand Down
9 changes: 4 additions & 5 deletions src/lib/core/a11y/live-announcer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {inject, fakeAsync, tick, ComponentFixture, TestBed} from '@angular/core/
import {Component} from '@angular/core';
import {By} from '@angular/platform-browser';
import {LiveAnnouncer, LIVE_ANNOUNCER_ELEMENT_TOKEN} from './live-announcer';
import {A11yModule} from '../index';


describe('LiveAnnouncer', () => {
Expand All @@ -11,8 +12,8 @@ describe('LiveAnnouncer', () => {

describe('with default element', () => {
beforeEach(() => TestBed.configureTestingModule({
imports: [A11yModule],
declarations: [TestApp],
providers: [LiveAnnouncer]
}));

beforeEach(fakeAsync(inject([LiveAnnouncer], (la: LiveAnnouncer) => {
Expand Down Expand Up @@ -77,11 +78,9 @@ describe('LiveAnnouncer', () => {
customLiveElement = document.createElement('div');

return TestBed.configureTestingModule({
imports: [A11yModule],
declarations: [TestApp],
providers: [
{provide: LIVE_ANNOUNCER_ELEMENT_TOKEN, useValue: customLiveElement},
LiveAnnouncer,
],
providers: [{provide: LIVE_ANNOUNCER_ELEMENT_TOKEN, useValue: customLiveElement}],
});
});

Expand Down
26 changes: 17 additions & 9 deletions src/lib/core/a11y/live-announcer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import {
Inject,
SkipSelf,
} from '@angular/core';
import {Platform} from '../platform/platform';


export const LIVE_ANNOUNCER_ELEMENT_TOKEN = new InjectionToken<HTMLElement>('liveAnnouncerElement');

Expand All @@ -16,12 +18,16 @@ export class LiveAnnouncer {

private _liveElement: Element;

constructor(@Optional() @Inject(LIVE_ANNOUNCER_ELEMENT_TOKEN) elementToken: any) {

// We inject the live element as `any` because the constructor signature cannot reference
// browser globals (HTMLElement) on non-browser environments, since having a class decorator
// causes TypeScript to preserve the constructor signature types.
this._liveElement = elementToken || this._createLiveElement();
constructor(
@Optional() @Inject(LIVE_ANNOUNCER_ELEMENT_TOKEN) elementToken: any,
platform: Platform) {
// Only do anything if we're on the browser platform.
if (platform.isBrowser) {
// We inject the live element as `any` because the constructor signature cannot reference
// browser globals (HTMLElement) on non-browser environments, since having a class decorator
// causes TypeScript to preserve the constructor signature types.
this._liveElement = elementToken || this._createLiveElement();
}
}

/**
Expand Down Expand Up @@ -64,16 +70,18 @@ export class LiveAnnouncer {

}

export function LIVE_ANNOUNCER_PROVIDER_FACTORY(parentDispatcher: LiveAnnouncer, liveElement: any) {
return parentDispatcher || new LiveAnnouncer(liveElement);
export function LIVE_ANNOUNCER_PROVIDER_FACTORY(
parentDispatcher: LiveAnnouncer, liveElement: any, platform: Platform) {
return parentDispatcher || new LiveAnnouncer(liveElement, platform);
}

export const LIVE_ANNOUNCER_PROVIDER = {
// If there is already a LiveAnnouncer available, use that. Otherwise, provide a new one.
provide: LiveAnnouncer,
deps: [
[new Optional(), new SkipSelf(), LiveAnnouncer],
[new Optional(), new Inject(LIVE_ANNOUNCER_ELEMENT_TOKEN)]
[new Optional(), new Inject(LIVE_ANNOUNCER_ELEMENT_TOKEN)],
Platform,
],
useFactory: LIVE_ANNOUNCER_PROVIDER_FACTORY
};
7 changes: 4 additions & 3 deletions src/lib/core/overlay/overlay-directives.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {coerceBooleanProperty} from '../coercion/boolean-property';
import {ESCAPE} from '../keyboard/keycodes';
import {ScrollDispatcher} from './scroll/scroll-dispatcher';
import {Subscription} from 'rxjs/Subscription';
import {ScrollDispatchModule} from './scroll/index';


/** Default set of positions for the overlay. Follows the behavior of a dropdown. */
Expand Down Expand Up @@ -323,9 +324,9 @@ export class ConnectedOverlayDirective implements OnDestroy {


@NgModule({
imports: [PortalModule],
exports: [ConnectedOverlayDirective, OverlayOrigin, Scrollable],
declarations: [ConnectedOverlayDirective, OverlayOrigin, Scrollable],
imports: [PortalModule, ScrollDispatchModule],
exports: [ConnectedOverlayDirective, OverlayOrigin, ScrollDispatchModule],
declarations: [ConnectedOverlayDirective, OverlayOrigin],
providers: [OVERLAY_PROVIDERS],
})
export class OverlayModule {}
2 changes: 0 additions & 2 deletions src/lib/core/overlay/overlay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {OverlayRef} from './overlay-ref';
import {OverlayPositionBuilder} from './position/overlay-position-builder';
import {VIEWPORT_RULER_PROVIDER} from './position/viewport-ruler';
import {OverlayContainer, OVERLAY_CONTAINER_PROVIDER} from './overlay-container';
import {SCROLL_DISPATCHER_PROVIDER} from './scroll/scroll-dispatcher';


/** Next overlay unique ID. */
Expand Down Expand Up @@ -94,6 +93,5 @@ export const OVERLAY_PROVIDERS: Provider[] = [
Overlay,
OverlayPositionBuilder,
VIEWPORT_RULER_PROVIDER,
SCROLL_DISPATCHER_PROVIDER,
OVERLAY_CONTAINER_PROVIDER,
];
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {Scrollable} from '../scroll/scrollable';
import {Subscription} from 'rxjs/Subscription';
import {TestBed, inject} from '@angular/core/testing';
import Spy = jasmine.Spy;
import {SCROLL_DISPATCHER_PROVIDER} from '../scroll/scroll-dispatcher';
import {ScrollDispatchModule} from '../scroll/index';


// Default width and height of the overlay and origin panels throughout these tests.
Expand All @@ -23,7 +23,8 @@ describe('ConnectedPositionStrategy', () => {
let viewportRuler: ViewportRuler;

beforeEach(() => TestBed.configureTestingModule({
providers: [VIEWPORT_RULER_PROVIDER, SCROLL_DISPATCHER_PROVIDER]
imports: [ScrollDispatchModule],
providers: [VIEWPORT_RULER_PROVIDER]
}));

beforeEach(inject([ViewportRuler], (_ruler: ViewportRuler) => {
Expand Down
6 changes: 4 additions & 2 deletions src/lib/core/overlay/position/viewport-ruler.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {ViewportRuler, VIEWPORT_RULER_PROVIDER} from './viewport-ruler';
import {TestBed, inject} from '@angular/core/testing';
import {SCROLL_DISPATCHER_PROVIDER} from '../scroll/scroll-dispatcher';
import {ScrollDispatchModule} from '../scroll/index';


// For all tests, we assume the browser window is 1024x786 (outerWidth x outerHeight).
// The karma config has been set to this for local tests, and it is the default size
Expand All @@ -22,7 +23,8 @@ describe('ViewportRuler', () => {
veryLargeElement.style.height = '6000px';

beforeEach(() => TestBed.configureTestingModule({
providers: [VIEWPORT_RULER_PROVIDER, SCROLL_DISPATCHER_PROVIDER]
imports: [ScrollDispatchModule],
providers: [VIEWPORT_RULER_PROVIDER]
}));

beforeEach(inject([ViewportRuler], (viewportRuler: ViewportRuler) => {
Expand Down
15 changes: 12 additions & 3 deletions src/lib/core/overlay/position/viewport-ruler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,18 @@ export class ViewportRuler {
private _documentRect?: ClientRect;

constructor(scrollDispatcher: ScrollDispatcher) {
// Initially cache the document rectangle.
this._cacheViewportGeometry();

// Subscribe to scroll and resize events and update the document rectangle on changes.
scrollDispatcher.scrolled(null, () => this._cacheViewportGeometry());
}

/** Gets a ClientRect for the viewport's bounds. */
getViewportRect(documentRect = this._documentRect): ClientRect {
// Cache the document bounding rect so that we don't recompute it for multiple calls.
if (!documentRect) {
this._cacheViewportGeometry();
documentRect = this._documentRect;
}

// Use the document element's bounding rect rather than the window scroll properties
// (e.g. pageYOffset, scrollY) due to in issue in Chrome and IE where window scroll
// properties and client coordinates (boundingClientRect, clientX/Y, etc.) are in different
Expand Down Expand Up @@ -51,6 +54,12 @@ export class ViewportRuler {
* @param documentRect
*/
getViewportScrollPosition(documentRect = this._documentRect) {
// Cache the document bounding rect so that we don't recompute it for multiple calls.
if (!documentRect) {
this._cacheViewportGeometry();
Copy link
Member

@crisbeto crisbeto Apr 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be a little shorter if we made _cacheViewportGeometry return the documentRect and then changed the method definition to:

getViewportScrollPosition(documentRect = this._cacheViewportGeometry()) {
  // continue as usual
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would then no longer be using the cached version if there is one.

documentRect = this._documentRect;
}

// The top-left-corner of the viewport is determined by the scroll position of the document
// body, normally just (scrollLeft, scrollTop). However, Chrome and Firefox disagree about
// whether `document.body` or `document.documentElement` is the scrolled element, so reading
Expand Down
15 changes: 15 additions & 0 deletions src/lib/core/overlay/scroll/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import {NgModule} from '@angular/core';
import {SCROLL_DISPATCHER_PROVIDER} from './scroll-dispatcher';
import {Scrollable} from './scrollable';
import {PlatformModule} from '../../platform/index';

export {Scrollable} from './scrollable';
export {ScrollDispatcher} from './scroll-dispatcher';

@NgModule({
imports: [PlatformModule],
exports: [Scrollable],
declarations: [Scrollable],
providers: [SCROLL_DISPATCHER_PROVIDER],
})
export class ScrollDispatchModule { }
18 changes: 12 additions & 6 deletions src/lib/core/overlay/scroll/scroll-dispatcher.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {Injectable, ElementRef, Optional, SkipSelf, NgZone} from '@angular/core';
import {ElementRef, Injectable, NgZone, Optional, SkipSelf} from '@angular/core';
import {Platform} from '../../platform/index';
import {Scrollable} from './scrollable';
import {Subject} from 'rxjs/Subject';
import {Observable} from 'rxjs/Observable';
Expand All @@ -17,7 +18,7 @@ export const DEFAULT_SCROLL_TIME = 20;
*/
@Injectable()
export class ScrollDispatcher {
constructor(private _ngZone: NgZone) { }
constructor(private _ngZone: NgZone, private _platform: Platform) { }

/** Subject for notifying that a registered scrollable reference element has been scrolled. */
_scrolled: Subject<void> = new Subject<void>();
Expand Down Expand Up @@ -62,6 +63,11 @@ export class ScrollDispatcher {
* to override the default "throttle" time.
*/
scrolled(auditTimeInMs: number = DEFAULT_SCROLL_TIME, callback: () => any): Subscription {
// Scroll events can only happen on the browser, so do nothing if we're not on the browser.
if (!this._platform.isBrowser) {
return Subscription.EMPTY;
}

// In the case of a 0ms delay, use an observable without auditTime
// since it does add a perceptible delay in processing overhead.
let observable = auditTimeInMs > 0 ?
Expand Down Expand Up @@ -126,14 +132,14 @@ export class ScrollDispatcher {
}
}

export function SCROLL_DISPATCHER_PROVIDER_FACTORY(parentDispatcher: ScrollDispatcher,
ngZone: NgZone) {
return parentDispatcher || new ScrollDispatcher(ngZone);
export function SCROLL_DISPATCHER_PROVIDER_FACTORY(
parentDispatcher: ScrollDispatcher, ngZone: NgZone, platform: Platform) {
return parentDispatcher || new ScrollDispatcher(ngZone, platform);
}

export const SCROLL_DISPATCHER_PROVIDER = {
// If there is already a ScrollDispatcher available, use that. Otherwise, provide a new one.
provide: ScrollDispatcher,
deps: [[new Optional(), new SkipSelf(), ScrollDispatcher], NgZone],
deps: [[new Optional(), new SkipSelf(), ScrollDispatcher], NgZone, Platform],
useFactory: SCROLL_DISPATCHER_PROVIDER_FACTORY
};
2 changes: 2 additions & 0 deletions src/lib/core/platform/platform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ const hasV8BreakIterator = typeof(window) !== 'undefined' ?
*/
@Injectable()
export class Platform {
isBrowser: boolean = typeof document === 'object' && !!document;

/** Layout Engines */
EDGE = /(edge)/i.test(navigator.userAgent);
TRIDENT = /(msie|trident)/i.test(navigator.userAgent);
Expand Down
Loading