Skip to content

Commit

Permalink
refactor: switch to new approach without ReflectiveInjector
Browse files Browse the repository at this point in the history
  • Loading branch information
crisbeto committed Jun 1, 2017
1 parent b13aff8 commit 3fe7dbf
Show file tree
Hide file tree
Showing 14 changed files with 146 additions and 108 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {Component} from '@angular/core';
import {BlockScrollStrategy, ViewportRuler} from '@angular/material';
import {ScrollStrategyOptions, ScrollStrategy} from '@angular/material';

@Component({
moduleId: module.id,
Expand All @@ -8,6 +8,6 @@ import {BlockScrollStrategy, ViewportRuler} from '@angular/material';
styleUrls: ['block-scroll-strategy-e2e.css'],
})
export class BlockScrollStrategyE2E {
constructor(private _viewportRuler: ViewportRuler) { }
scrollStrategy = new BlockScrollStrategy(this._viewportRuler);
constructor(private _scrollStrategyOptions: ScrollStrategyOptions) { }
scrollStrategy: ScrollStrategy = this._scrollStrategyOptions.get('block');
}
7 changes: 5 additions & 2 deletions src/lib/core/overlay/overlay-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,11 @@ export class OverlayRef implements PortalHost {
private _scrollStrategy: ScrollStrategy,
private _ngZone: NgZone) {

_scrollStrategy.attach(this,
typeof _state.scrollStrategy === 'string' ? null : _state.scrollStrategy.config);
let scrollStrategyConfig = typeof _state.scrollStrategy === 'string' ?
null :
_state.scrollStrategy.config;

_scrollStrategy.attach(this, scrollStrategyConfig);
}

/** The overlay's HTML element */
Expand Down
94 changes: 58 additions & 36 deletions src/lib/core/overlay/overlay.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import {OverlayState} from './overlay-state';
import {OverlayRef} from './overlay-ref';
import {PositionStrategy} from './position/position-strategy';
import {OverlayModule} from './overlay-directives';
import {ScrollStrategy} from './scroll/scroll-strategy';
import {ScrollStrategy, ScrollStrategyOptions, ScrollDispatcher} from './scroll/index';
import {ViewportRuler} from './position/viewport-ruler';


describe('Overlay', () => {
Expand All @@ -22,10 +23,20 @@ describe('Overlay', () => {
TestBed.configureTestingModule({
imports: [OverlayModule, PortalModule, OverlayTestModule],
providers: [
{provide: OverlayContainer, useFactory: () => {
overlayContainerElement = document.createElement('div');
return {getContainerElement: () => overlayContainerElement};
}}
ScrollDispatcher,
ViewportRuler,
{
provide: ScrollStrategyOptions,
useClass: ScrollStrategyOptionsOverride,
deps: [ScrollDispatcher, ViewportRuler]
},
{
provide: OverlayContainer,
useFactory: () => {
overlayContainerElement = document.createElement('div');
return {getContainerElement: () => overlayContainerElement};
}
}
]
}).compileComponents();
}));
Expand Down Expand Up @@ -337,51 +348,30 @@ describe('Overlay', () => {
describe('scroll strategy', () => {
let fakeScrollStrategy: FakeScrollStrategy;
let config: OverlayState;

class FakeScrollStrategy implements ScrollStrategy {
isEnabled = false;
overlayRef: OverlayRef;

constructor() {
fakeScrollStrategy = this;
}

attach(overlayRef: OverlayRef) {
this.overlayRef = overlayRef;
}

enable() {
this.isEnabled = true;
}

disable() {
this.isEnabled = false;
}
}
let overlayRef: OverlayRef;

beforeEach(() => {
});

beforeEach(inject([ScrollStrategyOptions], (scrollOptions: ScrollStrategyOptionsOverride) => {
config = new OverlayState();
overlay.registerScrollStrategy('fake', FakeScrollStrategy);
config.scrollStrategy = 'fake';
});
overlayRef = overlay.create(config);
fakeScrollStrategy =
scrollOptions.instances[scrollOptions.instances.length - 1] as FakeScrollStrategy;
}));

it('should attach the overlay ref to the scroll strategy', () => {
let overlayRef = overlay.create(config);

expect(fakeScrollStrategy.overlayRef).toBe(overlayRef,
'Expected scroll strategy to have been attached to the current overlay ref.');
});

it('should enable the scroll strategy when the overlay is attached', () => {
let overlayRef = overlay.create(config);

overlayRef.attach(componentPortal);
expect(fakeScrollStrategy.isEnabled).toBe(true, 'Expected scroll strategy to be enabled.');
});

it('should disable the scroll strategy once the overlay is detached', () => {
let overlayRef = overlay.create(config);

overlayRef.attach(componentPortal);
expect(fakeScrollStrategy.isEnabled).toBe(true, 'Expected scroll strategy to be enabled.');

Expand All @@ -390,8 +380,6 @@ describe('Overlay', () => {
});

it('should disable the scroll strategy when the overlay is destroyed', () => {
let overlayRef = overlay.create(config);

overlayRef.dispose();
expect(fakeScrollStrategy.isEnabled).toBe(false, 'Expected scroll strategy to be disabled.');
});
Expand Down Expand Up @@ -469,3 +457,37 @@ class FakePositionStrategy implements PositionStrategy {

dispose() {}
}


class FakeScrollStrategy implements ScrollStrategy {
isEnabled = false;
overlayRef: OverlayRef;

attach(overlayRef: OverlayRef) {
this.overlayRef = overlayRef;
}

enable() {
this.isEnabled = true;
}

disable() {
this.isEnabled = false;
}
}


class ScrollStrategyOptionsOverride extends ScrollStrategyOptions {
constructor(scrollDispatcher: ScrollDispatcher, viewportRuler: ViewportRuler) {
super(scrollDispatcher, viewportRuler);
}

// used for accessing the current instance in unit tests.
public instances: ScrollStrategy[] = [];

get(strategy: string): ScrollStrategy {
let instance = strategy === 'fake' ? new FakeScrollStrategy() : super.get(strategy);
this.instances.push(instance);
return instance;
}
}
54 changes: 7 additions & 47 deletions src/lib/core/overlay/overlay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,14 @@ import {
Injector,
NgZone,
Provider,
ReflectiveInjector,
} from '@angular/core';
import {OverlayState} from './overlay-state';
import {DomPortalHost} from '../portal/dom-portal-host';
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 {ScrollStrategy} from './scroll/scroll-strategy';
import {RepositionScrollStrategy} from './scroll/reposition-scroll-strategy';
import {BlockScrollStrategy} from './scroll/block-scroll-strategy';
import {CloseScrollStrategy} from './scroll/close-scroll-strategy';
import {NoopScrollStrategy} from './scroll/noop-scroll-strategy';
import {ScrollStrategy, ScrollStrategyOptions} from './scroll/index';


/** Next overlay unique ID. */
Expand All @@ -37,19 +32,10 @@ let defaultState = new OverlayState();
*/
@Injectable()
export class Overlay {
// Create a child ReflectiveInjector, allowing us to instantiate scroll
// strategies without going throught the injector cache.
private _reflectiveInjector = ReflectiveInjector.resolveAndCreate([], this._injector);
private _scrollStrategies = {
reposition: RepositionScrollStrategy,
block: BlockScrollStrategy,
close: CloseScrollStrategy,
noop: NoopScrollStrategy
};

constructor(private _overlayContainer: OverlayContainer,
private _componentFactoryResolver: ComponentFactoryResolver,
private _positionBuilder: OverlayPositionBuilder,
private _scrollStrategyOptions: ScrollStrategyOptions,
private _appRef: ApplicationRef,
private _injector: Injector,
private _ngZone: NgZone) { }
Expand All @@ -71,17 +57,6 @@ export class Overlay {
return this._positionBuilder;
}

/**
* Registers a scroll strategy to be available for use when creating an overlay.
* @param name Name of the scroll strategy.
* @param constructor Class to be used to instantiate the scroll strategy.
*/
registerScrollStrategy(name: string, constructor: Function): void {
if (name && constructor) {
this._scrollStrategies[name] = constructor;
}
}

/**
* Creates the DOM element for an overlay and appends it to the overlay container.
* @returns Newly-created pane element
Expand Down Expand Up @@ -111,29 +86,14 @@ export class Overlay {
* @param state
*/
private _createOverlayRef(pane: HTMLElement, state: OverlayState): OverlayRef {
let scrollStrategyName = typeof state.scrollStrategy === 'string' ?
state.scrollStrategy :
state.scrollStrategy.name;

let scrollStrategy = this._scrollStrategyOptions.get(scrollStrategyName);
let portalHost = this._createPortalHost(pane);
let scrollStrategy = this._createScrollStrategy(state);
return new OverlayRef(portalHost, pane, state, scrollStrategy, this._ngZone);
}

/**
* Resolves the scroll strategy of an overlay state.
* @param state State for which to resolve the scroll strategy.
*/
private _createScrollStrategy(state: OverlayState): ScrollStrategy {
let strategyName = typeof state.scrollStrategy === 'string' ?
state.scrollStrategy :
state.scrollStrategy.name;

if (!this._scrollStrategies.hasOwnProperty(strategyName)) {
throw new Error(`Unsupported scroll strategy "${strategyName}". The available scroll ` +
`strategies are ${Object.keys(this._scrollStrategies).join(', ')}.`);
}

// Note that we use `resolveAndInstantiate` which will instantiate
// the scroll strategy without putting it in the injector cache.
return this._reflectiveInjector.resolveAndInstantiate(this._scrollStrategies[strategyName]);
}
}

/** Providers for Overlay and its related injectables. */
Expand Down
1 change: 0 additions & 1 deletion src/lib/core/overlay/scroll/block-scroll-strategy.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import {
ComponentPortal,
OverlayModule,
PortalModule,
BlockScrollStrategy,
Platform,
ViewportRuler,
OverlayState,
Expand Down
2 changes: 0 additions & 2 deletions src/lib/core/overlay/scroll/block-scroll-strategy.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
import {Injectable} from '@angular/core';
import {ScrollStrategy} from './scroll-strategy';
import {ViewportRuler} from '../position/viewport-ruler';

/**
* Strategy that will prevent the user from scrolling while the overlay is visible.
*/
@Injectable()
export class BlockScrollStrategy implements ScrollStrategy {
private _previousHTMLStyles = { top: null, left: null };
private _previousScrollPosition: { top: number, left: number };
Expand Down
1 change: 0 additions & 1 deletion src/lib/core/overlay/scroll/close-scroll-strategy.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
OverlayModule,
ScrollStrategy,
ScrollDispatcher,
CloseScrollStrategy,
} from '../../core';


Expand Down
2 changes: 0 additions & 2 deletions src/lib/core/overlay/scroll/close-scroll-strategy.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import {Injectable} from '@angular/core';
import {ScrollStrategy, getMdScrollStrategyAlreadyAttachedError} from './scroll-strategy';
import {OverlayRef} from '../overlay-ref';
import {Subscription} from 'rxjs/Subscription';
Expand All @@ -8,7 +7,6 @@ import {ScrollDispatcher} from './scroll-dispatcher';
/**
* Strategy that will close the overlay as soon as the user starts scrolling.
*/
@Injectable()
export class CloseScrollStrategy implements ScrollStrategy {
private _scrollSubscription: Subscription|null = null;
private _overlayRef: OverlayRef;
Expand Down
4 changes: 3 additions & 1 deletion src/lib/core/overlay/scroll/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@ import {NgModule} from '@angular/core';
import {SCROLL_DISPATCHER_PROVIDER} from './scroll-dispatcher';
import {Scrollable} from './scrollable';
import {PlatformModule} from '../../platform/index';
import {ScrollStrategyOptions} from './scroll-strategy-options';

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

// Export pre-defined scroll strategies and interface to build custom ones.
export {ScrollStrategy} from './scroll-strategy';
export {ScrollStrategyOptions} from './scroll-strategy-options';
export {RepositionScrollStrategy} from './reposition-scroll-strategy';
export {CloseScrollStrategy} from './close-scroll-strategy';
export {NoopScrollStrategy} from './noop-scroll-strategy';
Expand All @@ -17,6 +19,6 @@ export {BlockScrollStrategy} from './block-scroll-strategy';
imports: [PlatformModule],
exports: [Scrollable],
declarations: [Scrollable],
providers: [SCROLL_DISPATCHER_PROVIDER],
providers: [SCROLL_DISPATCHER_PROVIDER, ScrollStrategyOptions],
})
export class ScrollDispatchModule { }
2 changes: 0 additions & 2 deletions src/lib/core/overlay/scroll/noop-scroll-strategy.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
import {Injectable} from '@angular/core';
import {ScrollStrategy} from './scroll-strategy';

/**
* Scroll strategy that doesn't do anything.
*/
@Injectable()
export class NoopScrollStrategy implements ScrollStrategy {
enable() { }
disable() { }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
OverlayModule,
ScrollStrategy,
ScrollDispatcher,
RepositionScrollStrategy,
} from '../../core';


Expand Down
2 changes: 0 additions & 2 deletions src/lib/core/overlay/scroll/reposition-scroll-strategy.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import {Injectable} from '@angular/core';
import {Subscription} from 'rxjs/Subscription';
import {ScrollStrategy, getMdScrollStrategyAlreadyAttachedError} from './scroll-strategy';
import {OverlayRef} from '../overlay-ref';
Expand All @@ -14,7 +13,6 @@ export interface RepositionScrollStrategyConfig {
/**
* Strategy that will update the element position as the user is scrolling.
*/
@Injectable()
export class RepositionScrollStrategy implements ScrollStrategy {
private _scrollSubscription: Subscription|null = null;
private _overlayRef: OverlayRef;
Expand Down
31 changes: 31 additions & 0 deletions src/lib/core/overlay/scroll/scroll-strategy-options.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import {Injectable} from '@angular/core';
import {ScrollStrategy} from './scroll-strategy';
import {RepositionScrollStrategy} from './reposition-scroll-strategy';
import {CloseScrollStrategy} from './close-scroll-strategy';
import {NoopScrollStrategy} from './noop-scroll-strategy';
import {BlockScrollStrategy} from './block-scroll-strategy';
import {ScrollDispatcher} from './scroll-dispatcher';
import {ViewportRuler} from '../position/viewport-ruler';


/**
* Factory that instantiates scroll strategies. Provides the built-in `reposition`, `close`,
* `noop` and `block` strategies by default.
*/
@Injectable()
export class ScrollStrategyOptions {
constructor(
private _scrollDispatcher: ScrollDispatcher,
private _viewportRuler: ViewportRuler) { }

get(strategy: string): ScrollStrategy {
switch (strategy) {
case 'reposition': return new RepositionScrollStrategy(this._scrollDispatcher);
case 'close': return new CloseScrollStrategy(this._scrollDispatcher);
case 'noop': return new NoopScrollStrategy();
case 'block': return new BlockScrollStrategy(this._viewportRuler);
}

throw new Error(`Unsupported scroll strategy "${strategy}".`);
}
}
Loading

0 comments on commit 3fe7dbf

Please sign in to comment.