From 59129f86697b57ab955636102bc9047591c45fb0 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Wed, 24 May 2017 19:07:45 +0200 Subject: [PATCH 1/5] feat(overlay): more flexible scroll strategy API and ability to define/override custom strategies * Refactors the overlay setup to allow for scroll strategies to be passed in by name, instead of by instance. * Handles the scroll strategy dependency injection automatically. * Adds an API for registering custom scroll strategies and overriding the existing ones. * Adds a second parameter to the `attach` method, allowing for a config object to be passed in. * Throws an error if there's an attempt to attach a scroll strategy multiple times. This is mostly a sanity check to ensure that we don't cache the scroll strategy instances. Relates to #4093. --- src/lib/autocomplete/autocomplete-trigger.ts | 4 +- src/lib/core/overlay/overlay-directives.ts | 6 +- src/lib/core/overlay/overlay-ref.ts | 14 ++-- src/lib/core/overlay/overlay-state.ts | 3 +- src/lib/core/overlay/overlay.spec.ts | 46 ++++++------- src/lib/core/overlay/overlay.ts | 54 ++++++++++++++- .../scroll/block-scroll-strategy.spec.ts | 66 ++++++++++++++----- .../overlay/scroll/block-scroll-strategy.ts | 2 + .../scroll/close-scroll-strategy.spec.ts | 6 +- .../overlay/scroll/close-scroll-strategy.ts | 8 ++- .../overlay/scroll/noop-scroll-strategy.ts | 2 + .../scroll/reposition-scroll-strategy.spec.ts | 6 +- .../scroll/reposition-scroll-strategy.ts | 24 +++++-- .../overlay/scroll/scroll-dispatcher.spec.ts | 2 +- .../core/overlay/scroll/scroll-strategy.md | 24 +++++-- .../core/overlay/scroll/scroll-strategy.ts | 9 ++- src/lib/datepicker/datepicker.ts | 4 +- src/lib/dialog/dialog.ts | 5 +- src/lib/menu/menu-trigger.ts | 7 +- src/lib/tooltip/tooltip.ts | 8 ++- 20 files changed, 211 insertions(+), 89 deletions(-) diff --git a/src/lib/autocomplete/autocomplete-trigger.ts b/src/lib/autocomplete/autocomplete-trigger.ts index 5dd5c1f8fce5..d1164bc898f9 100644 --- a/src/lib/autocomplete/autocomplete-trigger.ts +++ b/src/lib/autocomplete/autocomplete-trigger.ts @@ -22,7 +22,6 @@ import {MdOptionSelectionChange, MdOption} from '../core/option/option'; import {ENTER, UP_ARROW, DOWN_ARROW, ESCAPE} from '../core/keyboard/keycodes'; import {Dir} from '../core/rtl/dir'; import {MdInputContainer} from '../input/input-container'; -import {ScrollDispatcher} from '../core/overlay/scroll/scroll-dispatcher'; import {Subscription} from 'rxjs/Subscription'; import 'rxjs/add/observable/merge'; import 'rxjs/add/observable/fromEvent'; @@ -104,7 +103,6 @@ export class MdAutocompleteTrigger implements ControlValueAccessor, OnDestroy { constructor(private _element: ElementRef, private _overlay: Overlay, private _viewContainerRef: ViewContainerRef, private _changeDetectorRef: ChangeDetectorRef, - private _scrollDispatcher: ScrollDispatcher, @Optional() private _dir: Dir, private _zone: NgZone, @Optional() @Host() private _inputContainer: MdInputContainer, @Optional() @Inject(DOCUMENT) private _document: any) {} @@ -368,7 +366,7 @@ export class MdAutocompleteTrigger implements ControlValueAccessor, OnDestroy { overlayState.positionStrategy = this._getOverlayPosition(); overlayState.width = this._getHostWidth(); overlayState.direction = this._dir ? this._dir.value : 'ltr'; - overlayState.scrollStrategy = new RepositionScrollStrategy(this._scrollDispatcher); + overlayState.scrollStrategy = 'reposition'; return overlayState; } diff --git a/src/lib/core/overlay/overlay-directives.ts b/src/lib/core/overlay/overlay-directives.ts index 06d2194d6117..e09658cbd12d 100644 --- a/src/lib/core/overlay/overlay-directives.ts +++ b/src/lib/core/overlay/overlay-directives.ts @@ -16,7 +16,7 @@ import { import {Overlay, OVERLAY_PROVIDERS} from './overlay'; import {OverlayRef} from './overlay-ref'; import {TemplatePortal} from '../portal/portal'; -import {OverlayState} from './overlay-state'; +import {OverlayState, OverlayStateScrollStrategy} from './overlay-state'; import { ConnectionPositionPair, ConnectedOverlayPositionChange @@ -29,7 +29,6 @@ import {RepositionScrollStrategy} from './scroll/reposition-scroll-strategy'; import {ScrollStrategy} from './scroll/scroll-strategy'; 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'; @@ -125,7 +124,7 @@ export class ConnectedOverlayDirective implements OnDestroy, OnChanges { @Input() backdropClass: string; /** Strategy to be used when handling scroll events while the overlay is open. */ - @Input() scrollStrategy: ScrollStrategy = new RepositionScrollStrategy(this._scrollDispatcher); + @Input() scrollStrategy: OverlayStateScrollStrategy = 'reposition'; /** Whether the overlay is open. */ @Input() open: boolean = false; @@ -157,7 +156,6 @@ export class ConnectedOverlayDirective implements OnDestroy, OnChanges { constructor( private _overlay: Overlay, private _renderer: Renderer2, - private _scrollDispatcher: ScrollDispatcher, templateRef: TemplateRef, viewContainerRef: ViewContainerRef, @Optional() private _dir: Dir) { diff --git a/src/lib/core/overlay/overlay-ref.ts b/src/lib/core/overlay/overlay-ref.ts index 6a58b6a2b467..5c7d5e017234 100644 --- a/src/lib/core/overlay/overlay-ref.ts +++ b/src/lib/core/overlay/overlay-ref.ts @@ -20,9 +20,11 @@ export class OverlayRef implements PortalHost { private _portalHost: PortalHost, private _pane: HTMLElement, private _state: OverlayState, + private _scrollStrategy: ScrollStrategy, private _ngZone: NgZone) { - this._state.scrollStrategy.attach(this); + _scrollStrategy.attach(this, + typeof _state.scrollStrategy === 'string' ? null : _state.scrollStrategy.config); } /** The overlay's HTML element */ @@ -44,7 +46,7 @@ export class OverlayRef implements PortalHost { this.updateDirection(); this.updatePosition(); this._attachments.next(); - this._state.scrollStrategy.enable(); + this._scrollStrategy.enable(); // Enable pointer events for the overlay pane element. this._togglePointerEvents(true); @@ -71,7 +73,7 @@ export class OverlayRef implements PortalHost { // This is necessary because otherwise the pane element will cover the page and disable // pointer events therefore. Depends on the position strategy and the applied pane boundaries. this._togglePointerEvents(false); - this._state.scrollStrategy.disable(); + this._scrollStrategy.disable(); this._detachments.next(); return this._portalHost.detach(); @@ -85,9 +87,13 @@ export class OverlayRef implements PortalHost { this._state.positionStrategy.dispose(); } + if (this._scrollStrategy) { + this._scrollStrategy.disable(); + this._scrollStrategy = null; + } + this.detachBackdrop(); this._portalHost.dispose(); - this._state.scrollStrategy.disable(); this._detachments.next(); this._detachments.complete(); this._attachments.complete(); diff --git a/src/lib/core/overlay/overlay-state.ts b/src/lib/core/overlay/overlay-state.ts index 8b59c38aa0a6..336dfc1ce9fd 100644 --- a/src/lib/core/overlay/overlay-state.ts +++ b/src/lib/core/overlay/overlay-state.ts @@ -3,6 +3,7 @@ import {LayoutDirection} from '../rtl/dir'; import {ScrollStrategy} from './scroll/scroll-strategy'; import {NoopScrollStrategy} from './scroll/noop-scroll-strategy'; +export type OverlayStateScrollStrategy = string | {name: string; config: any}; /** * OverlayState is a bag of values for either the initial configuration or current state of an @@ -13,7 +14,7 @@ export class OverlayState { positionStrategy: PositionStrategy; /** Strategy to be used when handling scroll events while the overlay is open. */ - scrollStrategy: ScrollStrategy = new NoopScrollStrategy(); + scrollStrategy: OverlayStateScrollStrategy = 'noop'; /** Custom class to add to the overlay pane. */ panelClass: string = ''; diff --git a/src/lib/core/overlay/overlay.spec.ts b/src/lib/core/overlay/overlay.spec.ts index ccc1424a563a..508de4902e75 100644 --- a/src/lib/core/overlay/overlay.spec.ts +++ b/src/lib/core/overlay/overlay.spec.ts @@ -27,9 +27,7 @@ describe('Overlay', () => { return {getContainerElement: () => overlayContainerElement}; }} ] - }); - - TestBed.compileComponents(); + }).compileComponents(); })); beforeEach(inject([Overlay], (o: Overlay) => { @@ -355,10 +353,31 @@ describe('Overlay', () => { 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; + } + } + beforeEach(() => { config = new OverlayState(); - fakeScrollStrategy = new FakeScrollStrategy(); - config.scrollStrategy = fakeScrollStrategy; + overlay.registerScrollStrategy('fake', FakeScrollStrategy); + config.scrollStrategy = 'fake'; }); it('should attach the overlay ref to the scroll strategy', () => { @@ -465,20 +484,3 @@ 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; - } -} diff --git a/src/lib/core/overlay/overlay.ts b/src/lib/core/overlay/overlay.ts index 1693a830eaa1..067a30b2d1d1 100644 --- a/src/lib/core/overlay/overlay.ts +++ b/src/lib/core/overlay/overlay.ts @@ -5,6 +5,7 @@ import { Injector, NgZone, Provider, + ReflectiveInjector, } from '@angular/core'; import {OverlayState} from './overlay-state'; import {DomPortalHost} from '../portal/dom-portal-host'; @@ -12,6 +13,11 @@ 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'; /** Next overlay unique ID. */ @@ -31,12 +37,22 @@ 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 _appRef: ApplicationRef, private _injector: Injector, - private _ngZone: NgZone) {} + private _ngZone: NgZone) { } /** * Creates an overlay. @@ -55,15 +71,26 @@ 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 */ private _createPaneElement(): HTMLElement { let pane = document.createElement('div'); + pane.id = `cdk-overlay-${nextUniqueId++}`; pane.classList.add('cdk-overlay-pane'); - this._overlayContainer.getContainerElement().appendChild(pane); return pane; @@ -84,7 +111,28 @@ export class Overlay { * @param state */ private _createOverlayRef(pane: HTMLElement, state: OverlayState): OverlayRef { - return new OverlayRef(this._createPortalHost(pane), pane, state, this._ngZone); + 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]); } } diff --git a/src/lib/core/overlay/scroll/block-scroll-strategy.spec.ts b/src/lib/core/overlay/scroll/block-scroll-strategy.spec.ts index e2aea1783df1..2ab9832e8009 100644 --- a/src/lib/core/overlay/scroll/block-scroll-strategy.spec.ts +++ b/src/lib/core/overlay/scroll/block-scroll-strategy.spec.ts @@ -1,20 +1,38 @@ +import {NgModule, Component} from '@angular/core'; import {inject, TestBed, async} from '@angular/core/testing'; -import {ComponentPortal, OverlayModule, BlockScrollStrategy, Platform} from '../../core'; -import {ViewportRuler} from '../position/viewport-ruler'; +import { + ComponentPortal, + OverlayModule, + PortalModule, + BlockScrollStrategy, + Platform, + ViewportRuler, + OverlayState, + Overlay, + OverlayRef, +} from '../../core'; describe('BlockScrollStrategy', () => { let platform = new Platform(); - let strategy: BlockScrollStrategy; let viewport: ViewportRuler; + let overlayRef: OverlayRef; + let componentPortal: ComponentPortal; let forceScrollElement: HTMLElement; beforeEach(async(() => { - TestBed.configureTestingModule({imports: [OverlayModule]}).compileComponents(); + TestBed.configureTestingModule({ + imports: [OverlayModule, PortalModule, OverlayTestModule] + }).compileComponents(); })); - beforeEach(inject([ViewportRuler], (viewportRuler: ViewportRuler) => { - strategy = new BlockScrollStrategy(viewportRuler); + beforeEach(inject([Overlay, ViewportRuler], (overlay: Overlay, viewportRuler: ViewportRuler) => { + let overlayState = new OverlayState(); + + overlayState.scrollStrategy = 'block'; + overlayRef = overlay.create(overlayState); + componentPortal = new ComponentPortal(FocacciaMsg); + viewport = viewportRuler; forceScrollElement = document.createElement('div'); document.body.appendChild(forceScrollElement); @@ -23,7 +41,7 @@ describe('BlockScrollStrategy', () => { })); afterEach(() => { - strategy.disable(); + overlayRef.dispose(); document.body.removeChild(forceScrollElement); setScrollPosition(0, 0); }); @@ -33,7 +51,7 @@ describe('BlockScrollStrategy', () => { expect(viewport.getViewportScrollPosition().top) .toBe(100, 'Expected viewport to be scrollable initially.'); - strategy.enable(); + overlayRef.attach(componentPortal); expect(document.documentElement.style.top) .toBe('-100px', 'Expected element to be offset by the previous scroll amount.'); @@ -41,7 +59,7 @@ describe('BlockScrollStrategy', () => { expect(viewport.getViewportScrollPosition().top) .toBe(100, 'Expected the viewport not to scroll.'); - strategy.disable(); + overlayRef.detach(); expect(viewport.getViewportScrollPosition().top) .toBe(100, 'Expected old scroll position to have bee restored after disabling.'); @@ -59,7 +77,7 @@ describe('BlockScrollStrategy', () => { expect(viewport.getViewportScrollPosition().left) .toBe(100, 'Expected viewport to be scrollable initially.'); - strategy.enable(); + overlayRef.attach(componentPortal); expect(document.documentElement.style.left) .toBe('-100px', 'Expected element to be offset by the previous scroll amount.'); @@ -67,7 +85,7 @@ describe('BlockScrollStrategy', () => { expect(viewport.getViewportScrollPosition().left) .toBe(100, 'Expected the viewport not to scroll.'); - strategy.disable(); + overlayRef.detach(); expect(viewport.getViewportScrollPosition().left) .toBe(100, 'Expected old scroll position to have bee restored after disabling.'); @@ -80,10 +98,10 @@ describe('BlockScrollStrategy', () => { it('should toggle the `cdk-global-scrollblock` class', skipIOS(() => { expect(document.documentElement.classList).not.toContain('cdk-global-scrollblock'); - strategy.enable(); + overlayRef.attach(componentPortal); expect(document.documentElement.classList).toContain('cdk-global-scrollblock'); - strategy.disable(); + overlayRef.detach(); expect(document.documentElement.classList).not.toContain('cdk-global-scrollblock'); })); @@ -93,12 +111,12 @@ describe('BlockScrollStrategy', () => { root.style.top = '13px'; root.style.left = '37px'; - strategy.enable(); + overlayRef.attach(componentPortal); expect(root.style.top).not.toBe('13px'); expect(root.style.left).not.toBe('37px'); - strategy.disable(); + overlayRef.detach(); expect(root.style.top).toBe('13px'); expect(root.style.left).toBe('37px'); @@ -106,7 +124,7 @@ describe('BlockScrollStrategy', () => { it(`should't do anything if the page isn't scrollable`, skipIOS(() => { forceScrollElement.style.display = 'none'; - strategy.enable(); + overlayRef.attach(componentPortal); expect(document.documentElement.classList).not.toContain('cdk-global-scrollblock'); })); @@ -116,7 +134,7 @@ describe('BlockScrollStrategy', () => { const previousContentWidth = document.documentElement.getBoundingClientRect().width; - strategy.enable(); + overlayRef.attach(componentPortal); expect(document.documentElement.getBoundingClientRect().width).toBe(previousContentWidth); }); @@ -151,3 +169,17 @@ describe('BlockScrollStrategy', () => { } }); + + +/** Simple component that we can attach to the overlay. */ +@Component({template: '

Focaccia

'}) +class FocacciaMsg { } + + +/** Test module to hold the component. */ +@NgModule({ + imports: [OverlayModule, PortalModule], + declarations: [FocacciaMsg], + entryComponents: [FocacciaMsg], +}) +class OverlayTestModule { } diff --git a/src/lib/core/overlay/scroll/block-scroll-strategy.ts b/src/lib/core/overlay/scroll/block-scroll-strategy.ts index 438db2efb560..c9aab0660a87 100644 --- a/src/lib/core/overlay/scroll/block-scroll-strategy.ts +++ b/src/lib/core/overlay/scroll/block-scroll-strategy.ts @@ -1,9 +1,11 @@ +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 }; diff --git a/src/lib/core/overlay/scroll/close-scroll-strategy.spec.ts b/src/lib/core/overlay/scroll/close-scroll-strategy.spec.ts index aadfe8f209d5..8d87b512121b 100644 --- a/src/lib/core/overlay/scroll/close-scroll-strategy.spec.ts +++ b/src/lib/core/overlay/scroll/close-scroll-strategy.spec.ts @@ -34,11 +34,9 @@ describe('CloseScrollStrategy', () => { TestBed.compileComponents(); })); - beforeEach(inject([Overlay, ScrollDispatcher], (overlay: Overlay, - scrollDispatcher: ScrollDispatcher) => { - + beforeEach(inject([Overlay], (overlay: Overlay) => { let overlayState = new OverlayState(); - overlayState.scrollStrategy = new CloseScrollStrategy(scrollDispatcher); + overlayState.scrollStrategy = 'close'; overlayRef = overlay.create(overlayState); componentPortal = new ComponentPortal(MozarellaMsg); })); diff --git a/src/lib/core/overlay/scroll/close-scroll-strategy.ts b/src/lib/core/overlay/scroll/close-scroll-strategy.ts index 28f6a3d012c4..fafeea4c97b0 100644 --- a/src/lib/core/overlay/scroll/close-scroll-strategy.ts +++ b/src/lib/core/overlay/scroll/close-scroll-strategy.ts @@ -1,4 +1,5 @@ -import {ScrollStrategy} from './scroll-strategy'; +import {Injectable} from '@angular/core'; +import {ScrollStrategy, getMdScrollStrategyAlreadyAttachedError} from './scroll-strategy'; import {OverlayRef} from '../overlay-ref'; import {Subscription} from 'rxjs/Subscription'; import {ScrollDispatcher} from './scroll-dispatcher'; @@ -7,6 +8,7 @@ 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; @@ -14,6 +16,10 @@ export class CloseScrollStrategy implements ScrollStrategy { constructor(private _scrollDispatcher: ScrollDispatcher) { } attach(overlayRef: OverlayRef) { + if (this._overlayRef) { + throw getMdScrollStrategyAlreadyAttachedError(); + } + this._overlayRef = overlayRef; } diff --git a/src/lib/core/overlay/scroll/noop-scroll-strategy.ts b/src/lib/core/overlay/scroll/noop-scroll-strategy.ts index 3d50a8f7743a..7f795f3532c0 100644 --- a/src/lib/core/overlay/scroll/noop-scroll-strategy.ts +++ b/src/lib/core/overlay/scroll/noop-scroll-strategy.ts @@ -1,8 +1,10 @@ +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() { } diff --git a/src/lib/core/overlay/scroll/reposition-scroll-strategy.spec.ts b/src/lib/core/overlay/scroll/reposition-scroll-strategy.spec.ts index d210e20b8900..b8c4be1a2490 100644 --- a/src/lib/core/overlay/scroll/reposition-scroll-strategy.spec.ts +++ b/src/lib/core/overlay/scroll/reposition-scroll-strategy.spec.ts @@ -34,11 +34,9 @@ describe('RepositionScrollStrategy', () => { TestBed.compileComponents(); })); - beforeEach(inject([Overlay, ScrollDispatcher], (overlay: Overlay, - scrollDispatcher: ScrollDispatcher) => { - + beforeEach(inject([Overlay], (overlay: Overlay) => { let overlayState = new OverlayState(); - overlayState.scrollStrategy = new RepositionScrollStrategy(scrollDispatcher); + overlayState.scrollStrategy = 'reposition'; overlayRef = overlay.create(overlayState); componentPortal = new ComponentPortal(PastaMsg); })); diff --git a/src/lib/core/overlay/scroll/reposition-scroll-strategy.ts b/src/lib/core/overlay/scroll/reposition-scroll-strategy.ts index 924584ceb3a3..9bb90db73870 100644 --- a/src/lib/core/overlay/scroll/reposition-scroll-strategy.ts +++ b/src/lib/core/overlay/scroll/reposition-scroll-strategy.ts @@ -1,25 +1,41 @@ +import {Injectable} from '@angular/core'; import {Subscription} from 'rxjs/Subscription'; -import {ScrollStrategy} from './scroll-strategy'; +import {ScrollStrategy, getMdScrollStrategyAlreadyAttachedError} from './scroll-strategy'; import {OverlayRef} from '../overlay-ref'; import {ScrollDispatcher} from './scroll-dispatcher'; +/** + * Config options for the RepositionScrollStrategy. + */ +export interface RepositionScrollStrategyConfig { + scrollThrottle?: number; +} /** * 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; + private _config: RepositionScrollStrategyConfig; - constructor(private _scrollDispatcher: ScrollDispatcher, private _scrollThrottle = 0) { } + constructor(private _scrollDispatcher: ScrollDispatcher) { } + + attach(overlayRef: OverlayRef, config?: RepositionScrollStrategyConfig) { + if (this._overlayRef) { + throw getMdScrollStrategyAlreadyAttachedError(); + } - attach(overlayRef: OverlayRef) { this._overlayRef = overlayRef; + this._config = config; } enable() { if (!this._scrollSubscription) { - this._scrollSubscription = this._scrollDispatcher.scrolled(this._scrollThrottle, () => { + let throttle = this._config ? this._config.scrollThrottle : 0; + + this._scrollSubscription = this._scrollDispatcher.scrolled(throttle, () => { this._overlayRef.updatePosition(); }); } diff --git a/src/lib/core/overlay/scroll/scroll-dispatcher.spec.ts b/src/lib/core/overlay/scroll/scroll-dispatcher.spec.ts index 2ce254edc3d5..eb2da7ee1fde 100644 --- a/src/lib/core/overlay/scroll/scroll-dispatcher.spec.ts +++ b/src/lib/core/overlay/scroll/scroll-dispatcher.spec.ts @@ -9,7 +9,7 @@ describe('Scroll Dispatcher', () => { beforeEach(async(() => { TestBed.configureTestingModule({ - imports: [OverlayModule, ScrollTestModule], + imports: [ScrollTestModule], }); TestBed.compileComponents(); diff --git a/src/lib/core/overlay/scroll/scroll-strategy.md b/src/lib/core/overlay/scroll/scroll-strategy.md index bfb083cc1d79..003aa61c3fb6 100644 --- a/src/lib/core/overlay/scroll/scroll-strategy.md +++ b/src/lib/core/overlay/scroll/scroll-strategy.md @@ -1,19 +1,19 @@ # Scroll strategies ## What is a scroll strategy? -A scroll strategy is a class that describes how an overlay should behave if the user scrolls +A scroll strategy is a way of describing how an overlay should behave if the user scrolls while the overlay is open. The strategy has a reference to the `OverlayRef`, allowing it to recalculate the position, close the overlay, block scrolling, etc. ## Usage -To associate an overlay with a scroll strategy, you have to pass in a `ScrollStrategy` instance -to the `OverlayState`. By default, all overlays will use the `NoopScrollStrategy` which doesn't -do anything: +To associate an overlay with a scroll strategy, you have to pass in the name of the scroll strategy +to the `OverlayState`. By default, all overlays will use the `noop` strategy which doesn't do +anything. The other available strategies are `reposition`, `block` and `close`: ```ts let overlayState = new OverlayState(); -overlayState.scrollStrategy = new BlockScrollStrategy(this._viewportRuler); +overlayState.scrollStrategy = 'block'; this._overlay.create(overlayState).attach(yourPortal); ``` @@ -25,3 +25,17 @@ interface. There are three stages of a scroll strategy's life cycle: 2. When an overlay is attached to the DOM, it'll call the `enable` method on its scroll strategy, 3. When an overlay is detached from the DOM or destroyed, it'll call the `disable` method on its scroll strategy, allowing it to clean up after itself. + +Afterwards the scroll strategy has to be registered with the `Overlay` service: + +```ts +overlay.registerScrollStrategy('custom', CustomScrollStrategy); +``` + +Finally, you can use the strategy by passing its name to the `OverlayState`: +```ts +let overlayState = new OverlayState(); + +overlayState.scrollStrategy = 'custom'; +this._overlay.create(overlayState).attach(yourPortal); +``` diff --git a/src/lib/core/overlay/scroll/scroll-strategy.ts b/src/lib/core/overlay/scroll/scroll-strategy.ts index 37fcada36e6d..3f71d48f206c 100644 --- a/src/lib/core/overlay/scroll/scroll-strategy.ts +++ b/src/lib/core/overlay/scroll/scroll-strategy.ts @@ -7,5 +7,12 @@ import {OverlayRef} from '../overlay-ref'; export interface ScrollStrategy { enable: () => void; disable: () => void; - attach: (overlayRef: OverlayRef) => void; + attach: (overlayRef: OverlayRef, config?: any) => void; +} + +/** + * Returns an error to be thrown when attempting to attach an already-attached scroll strategy. + */ +export function getMdScrollStrategyAlreadyAttachedError(): Error { + return new Error(`Scroll strategy has already been attached.`); } diff --git a/src/lib/datepicker/datepicker.ts b/src/lib/datepicker/datepicker.ts index 41302c21c0e3..7e518361da65 100644 --- a/src/lib/datepicker/datepicker.ts +++ b/src/lib/datepicker/datepicker.ts @@ -21,7 +21,6 @@ import {Dir} from '../core/rtl/dir'; import {MdDialog} from '../dialog/dialog'; import {MdDialogRef} from '../dialog/dialog-ref'; import {PositionStrategy} from '../core/overlay/position/position-strategy'; -import {RepositionScrollStrategy, ScrollDispatcher} from '../core/overlay/index'; import {MdDatepickerInput} from './datepicker-input'; import {Subscription} from 'rxjs/Subscription'; import {MdDialogConfig} from '../dialog/dialog-config'; @@ -157,7 +156,6 @@ export class MdDatepicker implements OnDestroy { private _overlay: Overlay, private _ngZone: NgZone, private _viewContainerRef: ViewContainerRef, - private _scrollDispatcher: ScrollDispatcher, @Optional() private _dateAdapter: DateAdapter, @Optional() private _dir: Dir) { if (!this._dateAdapter) { @@ -269,7 +267,7 @@ export class MdDatepicker implements OnDestroy { overlayState.hasBackdrop = true; overlayState.backdropClass = 'md-overlay-transparent-backdrop'; overlayState.direction = this._dir ? this._dir.value : 'ltr'; - overlayState.scrollStrategy = new RepositionScrollStrategy(this._scrollDispatcher); + overlayState.scrollStrategy = 'reposition'; this._popupRef = this._overlay.create(overlayState); } diff --git a/src/lib/dialog/dialog.ts b/src/lib/dialog/dialog.ts index 71bf25ace0c9..b6607014cba6 100644 --- a/src/lib/dialog/dialog.ts +++ b/src/lib/dialog/dialog.ts @@ -10,8 +10,6 @@ import {MdDialogConfig} from './dialog-config'; import {MdDialogRef} from './dialog-ref'; import {MdDialogContainer} from './dialog-container'; import {TemplatePortal} from '../core/portal/portal'; -import {BlockScrollStrategy} from '../core/overlay/scroll/block-scroll-strategy'; -import {ViewportRuler} from '../core/overlay/position/viewport-ruler'; import 'rxjs/add/operator/first'; @@ -50,7 +48,6 @@ export class MdDialog { constructor( private _overlay: Overlay, private _injector: Injector, - private _viewportRuler: ViewportRuler, @Optional() private _location: Location, @Optional() @SkipSelf() private _parentDialog: MdDialog) { @@ -123,7 +120,7 @@ export class MdDialog { let overlayState = new OverlayState(); overlayState.panelClass = dialogConfig.panelClass; overlayState.hasBackdrop = dialogConfig.hasBackdrop; - overlayState.scrollStrategy = new BlockScrollStrategy(this._viewportRuler); + overlayState.scrollStrategy = 'block'; if (dialogConfig.backdropClass) { overlayState.backdropClass = dialogConfig.backdropClass; } diff --git a/src/lib/menu/menu-trigger.ts b/src/lib/menu/menu-trigger.ts index 22e5240d72d0..365bb1fc9e64 100644 --- a/src/lib/menu/menu-trigger.ts +++ b/src/lib/menu/menu-trigger.ts @@ -22,8 +22,6 @@ import { ConnectedPositionStrategy, HorizontalConnectionPos, VerticalConnectionPos, - RepositionScrollStrategy, - ScrollDispatcher, } from '../core'; import {Subscription} from 'rxjs/Subscription'; import {MenuPositionX, MenuPositionY} from './menu-positions'; @@ -80,8 +78,7 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy { @Output() onMenuClose = new EventEmitter(); constructor(private _overlay: Overlay, private _element: ElementRef, - private _viewContainerRef: ViewContainerRef, @Optional() private _dir: Dir, - private _scrollDispatcher: ScrollDispatcher) { } + private _viewContainerRef: ViewContainerRef, @Optional() private _dir: Dir) { } ngAfterViewInit() { this._checkMenu(); @@ -219,7 +216,7 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy { overlayState.hasBackdrop = true; overlayState.backdropClass = 'cdk-overlay-transparent-backdrop'; overlayState.direction = this.dir; - overlayState.scrollStrategy = new RepositionScrollStrategy(this._scrollDispatcher); + overlayState.scrollStrategy = 'reposition'; return overlayState; } diff --git a/src/lib/tooltip/tooltip.ts b/src/lib/tooltip/tooltip.ts index e32e1cb8d589..40f4f3305ce0 100644 --- a/src/lib/tooltip/tooltip.ts +++ b/src/lib/tooltip/tooltip.ts @@ -25,7 +25,6 @@ import { ComponentPortal, OverlayConnectionPosition, OriginConnectionPosition, - RepositionScrollStrategy, } from '../core'; import {Observable} from 'rxjs/Observable'; import {Subject} from 'rxjs/Subject'; @@ -237,10 +236,13 @@ export class MdTooltip implements OnDestroy { }); let config = new OverlayState(); + config.direction = this._dir ? this._dir.value : 'ltr'; config.positionStrategy = strategy; - config.scrollStrategy = - new RepositionScrollStrategy(this._scrollDispatcher, SCROLL_THROTTLE_MS); + config.scrollStrategy = { + name: 'reposition', + config: { scrollThrottle: SCROLL_THROTTLE_MS } + }; this._overlayRef = this._overlay.create(config); } From 4d35aecbef53b30c44b66a3b821959bbab333aa1 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Thu, 1 Jun 2017 20:41:19 +0200 Subject: [PATCH 2/5] refactor: switch to new approach without ReflectiveInjector --- .../block-scroll-strategy-e2e.ts | 6 +- src/lib/core/overlay/overlay-ref.ts | 7 +- src/lib/core/overlay/overlay.spec.ts | 94 ++++++++++++------- src/lib/core/overlay/overlay.ts | 54 ++--------- .../scroll/block-scroll-strategy.spec.ts | 1 - .../overlay/scroll/block-scroll-strategy.ts | 2 - .../scroll/close-scroll-strategy.spec.ts | 1 - .../overlay/scroll/close-scroll-strategy.ts | 2 - src/lib/core/overlay/scroll/index.ts | 4 +- .../overlay/scroll/noop-scroll-strategy.ts | 2 - .../scroll/reposition-scroll-strategy.spec.ts | 1 - .../scroll/reposition-scroll-strategy.ts | 2 - .../overlay/scroll/scroll-strategy-options.ts | 31 ++++++ .../core/overlay/scroll/scroll-strategy.md | 47 ++++++++-- 14 files changed, 146 insertions(+), 108 deletions(-) create mode 100644 src/lib/core/overlay/scroll/scroll-strategy-options.ts diff --git a/src/e2e-app/block-scroll-strategy/block-scroll-strategy-e2e.ts b/src/e2e-app/block-scroll-strategy/block-scroll-strategy-e2e.ts index a9e8f4b7e3e3..8a247c6331b2 100644 --- a/src/e2e-app/block-scroll-strategy/block-scroll-strategy-e2e.ts +++ b/src/e2e-app/block-scroll-strategy/block-scroll-strategy-e2e.ts @@ -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, @@ -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'); } diff --git a/src/lib/core/overlay/overlay-ref.ts b/src/lib/core/overlay/overlay-ref.ts index 5c7d5e017234..cb61529c3457 100644 --- a/src/lib/core/overlay/overlay-ref.ts +++ b/src/lib/core/overlay/overlay-ref.ts @@ -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 */ diff --git a/src/lib/core/overlay/overlay.spec.ts b/src/lib/core/overlay/overlay.spec.ts index 508de4902e75..099cb6fc5ce3 100644 --- a/src/lib/core/overlay/overlay.spec.ts +++ b/src/lib/core/overlay/overlay.spec.ts @@ -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', () => { @@ -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(); })); @@ -352,51 +363,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.'); @@ -405,8 +395,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.'); }); @@ -484,3 +472,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; + } +} diff --git a/src/lib/core/overlay/overlay.ts b/src/lib/core/overlay/overlay.ts index 067a30b2d1d1..55b21e922281 100644 --- a/src/lib/core/overlay/overlay.ts +++ b/src/lib/core/overlay/overlay.ts @@ -5,7 +5,6 @@ import { Injector, NgZone, Provider, - ReflectiveInjector, } from '@angular/core'; import {OverlayState} from './overlay-state'; import {DomPortalHost} from '../portal/dom-portal-host'; @@ -13,11 +12,7 @@ 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. */ @@ -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) { } @@ -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 @@ -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. */ diff --git a/src/lib/core/overlay/scroll/block-scroll-strategy.spec.ts b/src/lib/core/overlay/scroll/block-scroll-strategy.spec.ts index 2ab9832e8009..895fbab5a797 100644 --- a/src/lib/core/overlay/scroll/block-scroll-strategy.spec.ts +++ b/src/lib/core/overlay/scroll/block-scroll-strategy.spec.ts @@ -4,7 +4,6 @@ import { ComponentPortal, OverlayModule, PortalModule, - BlockScrollStrategy, Platform, ViewportRuler, OverlayState, diff --git a/src/lib/core/overlay/scroll/block-scroll-strategy.ts b/src/lib/core/overlay/scroll/block-scroll-strategy.ts index c9aab0660a87..438db2efb560 100644 --- a/src/lib/core/overlay/scroll/block-scroll-strategy.ts +++ b/src/lib/core/overlay/scroll/block-scroll-strategy.ts @@ -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 }; diff --git a/src/lib/core/overlay/scroll/close-scroll-strategy.spec.ts b/src/lib/core/overlay/scroll/close-scroll-strategy.spec.ts index 8d87b512121b..ce4aa1074dd3 100644 --- a/src/lib/core/overlay/scroll/close-scroll-strategy.spec.ts +++ b/src/lib/core/overlay/scroll/close-scroll-strategy.spec.ts @@ -10,7 +10,6 @@ import { OverlayModule, ScrollStrategy, ScrollDispatcher, - CloseScrollStrategy, } from '../../core'; diff --git a/src/lib/core/overlay/scroll/close-scroll-strategy.ts b/src/lib/core/overlay/scroll/close-scroll-strategy.ts index fafeea4c97b0..7caca425cde3 100644 --- a/src/lib/core/overlay/scroll/close-scroll-strategy.ts +++ b/src/lib/core/overlay/scroll/close-scroll-strategy.ts @@ -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'; @@ -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; diff --git a/src/lib/core/overlay/scroll/index.ts b/src/lib/core/overlay/scroll/index.ts index ae291c6d5b9a..acf5039b6188 100644 --- a/src/lib/core/overlay/scroll/index.ts +++ b/src/lib/core/overlay/scroll/index.ts @@ -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'; @@ -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 { } diff --git a/src/lib/core/overlay/scroll/noop-scroll-strategy.ts b/src/lib/core/overlay/scroll/noop-scroll-strategy.ts index 7f795f3532c0..3d50a8f7743a 100644 --- a/src/lib/core/overlay/scroll/noop-scroll-strategy.ts +++ b/src/lib/core/overlay/scroll/noop-scroll-strategy.ts @@ -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() { } diff --git a/src/lib/core/overlay/scroll/reposition-scroll-strategy.spec.ts b/src/lib/core/overlay/scroll/reposition-scroll-strategy.spec.ts index b8c4be1a2490..9591b3e9dc07 100644 --- a/src/lib/core/overlay/scroll/reposition-scroll-strategy.spec.ts +++ b/src/lib/core/overlay/scroll/reposition-scroll-strategy.spec.ts @@ -10,7 +10,6 @@ import { OverlayModule, ScrollStrategy, ScrollDispatcher, - RepositionScrollStrategy, } from '../../core'; diff --git a/src/lib/core/overlay/scroll/reposition-scroll-strategy.ts b/src/lib/core/overlay/scroll/reposition-scroll-strategy.ts index 9bb90db73870..c5107c333b72 100644 --- a/src/lib/core/overlay/scroll/reposition-scroll-strategy.ts +++ b/src/lib/core/overlay/scroll/reposition-scroll-strategy.ts @@ -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'; @@ -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; diff --git a/src/lib/core/overlay/scroll/scroll-strategy-options.ts b/src/lib/core/overlay/scroll/scroll-strategy-options.ts new file mode 100644 index 000000000000..caac29a7ec87 --- /dev/null +++ b/src/lib/core/overlay/scroll/scroll-strategy-options.ts @@ -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}".`); + } +} diff --git a/src/lib/core/overlay/scroll/scroll-strategy.md b/src/lib/core/overlay/scroll/scroll-strategy.md index 003aa61c3fb6..ab359cf015a8 100644 --- a/src/lib/core/overlay/scroll/scroll-strategy.md +++ b/src/lib/core/overlay/scroll/scroll-strategy.md @@ -26,16 +26,47 @@ interface. There are three stages of a scroll strategy's life cycle: 3. When an overlay is detached from the DOM or destroyed, it'll call the `disable` method on its scroll strategy, allowing it to clean up after itself. -Afterwards the scroll strategy has to be registered with the `Overlay` service: +Afterwards you have to override the `ScrollStrategyOptions` provider, which is used to instantiate +the scroll strategies and to handle the dependency injection. ```ts -overlay.registerScrollStrategy('custom', CustomScrollStrategy); -``` +import {NgModule} from '@angular/core'; +import { + ScrollStrategy, + ScrollStrategyOptions, + ScrollDispatcher, + ViewportRuler, +} from '@angular/material'; -Finally, you can use the strategy by passing its name to the `OverlayState`: -```ts -let overlayState = new OverlayState(); +// Your custom scroll strategy. +export class CustomScrollStrategy implements ScrollStrategy { + // your implementation +} -overlayState.scrollStrategy = 'custom'; -this._overlay.create(overlayState).attach(yourPortal); +// Provider that'll instantiate your custom strategies, as well as the built-in ones from Material. +class ScrollStrategyOptionsOverride extends ScrollStrategyOptions { + constructor(scrollDispatcher: ScrollDispatcher, viewportRuler: ViewportRuler) { + super(scrollDispatcher, viewportRuler); + } + + get(strategy: string): ScrollStrategy { + if (strategy === 'custom') { + return new CustomScrollStrategy(); + } + + return super.get(strategy); + } +} + +// Register the provider with your module. +@NgModule({ + providers: [ + { + provide: ScrollStrategyOptions, + useClass: ScrollStrategyOptionsOverride, + deps: [ScrollDispatcher, ViewportRuler] + } + ] +}) +export class YourModule { } ``` From 332e9d5a309b29db6abc3a1a439d0a2064a2c13f Mon Sep 17 00:00:00 2001 From: crisbeto Date: Fri, 2 Jun 2017 07:57:00 +0200 Subject: [PATCH 3/5] refactor: switch to better approach --- .../block-scroll-strategy-e2e.ts | 2 +- src/lib/autocomplete/autocomplete-trigger.ts | 5 ++-- src/lib/core/overlay/overlay-directives.ts | 5 ++-- src/lib/core/overlay/overlay-ref.ts | 8 ++++--- src/lib/core/overlay/overlay-state.ts | 8 +++---- src/lib/core/overlay/overlay.spec.ts | 13 ++++++---- src/lib/core/overlay/overlay.ts | 20 ++++++++++++---- .../scroll/block-scroll-strategy.spec.ts | 24 ++++++++++--------- .../scroll/close-scroll-strategy.spec.ts | 7 +++--- src/lib/core/overlay/scroll/index.ts | 2 +- .../scroll/reposition-scroll-strategy.spec.ts | 7 +++--- .../overlay/scroll/scroll-strategy-options.ts | 16 +++++-------- .../core/overlay/scroll/scroll-strategy.md | 17 +++++-------- src/lib/datepicker/datepicker.ts | 4 +++- src/lib/dialog/dialog.ts | 12 ++++++++-- src/lib/menu/menu-trigger.ts | 4 +++- src/lib/tooltip/tooltip.ts | 4 +++- 17 files changed, 93 insertions(+), 65 deletions(-) diff --git a/src/e2e-app/block-scroll-strategy/block-scroll-strategy-e2e.ts b/src/e2e-app/block-scroll-strategy/block-scroll-strategy-e2e.ts index 8a247c6331b2..ba38427d3e46 100644 --- a/src/e2e-app/block-scroll-strategy/block-scroll-strategy-e2e.ts +++ b/src/e2e-app/block-scroll-strategy/block-scroll-strategy-e2e.ts @@ -9,5 +9,5 @@ import {ScrollStrategyOptions, ScrollStrategy} from '@angular/material'; }) export class BlockScrollStrategyE2E { constructor(private _scrollStrategyOptions: ScrollStrategyOptions) { } - scrollStrategy: ScrollStrategy = this._scrollStrategyOptions.get('block'); + scrollStrategy: ScrollStrategy = this._scrollStrategyOptions.block(); } diff --git a/src/lib/autocomplete/autocomplete-trigger.ts b/src/lib/autocomplete/autocomplete-trigger.ts index d1164bc898f9..646091cb66eb 100644 --- a/src/lib/autocomplete/autocomplete-trigger.ts +++ b/src/lib/autocomplete/autocomplete-trigger.ts @@ -13,7 +13,7 @@ import { } from '@angular/core'; import {ControlValueAccessor, NG_VALUE_ACCESSOR} from '@angular/forms'; import {DOCUMENT} from '@angular/platform-browser'; -import {Overlay, OverlayRef, OverlayState, TemplatePortal, RepositionScrollStrategy} from '../core'; +import {Overlay, OverlayRef, OverlayState, TemplatePortal, ScrollStrategyOptions} from '../core'; import {MdAutocomplete} from './autocomplete'; import {PositionStrategy} from '../core/overlay/position/position-strategy'; import {ConnectedPositionStrategy} from '../core/overlay/position/connected-position-strategy'; @@ -103,6 +103,7 @@ export class MdAutocompleteTrigger implements ControlValueAccessor, OnDestroy { constructor(private _element: ElementRef, private _overlay: Overlay, private _viewContainerRef: ViewContainerRef, private _changeDetectorRef: ChangeDetectorRef, + private _scrollStrategyOptions: ScrollStrategyOptions, @Optional() private _dir: Dir, private _zone: NgZone, @Optional() @Host() private _inputContainer: MdInputContainer, @Optional() @Inject(DOCUMENT) private _document: any) {} @@ -366,7 +367,7 @@ export class MdAutocompleteTrigger implements ControlValueAccessor, OnDestroy { overlayState.positionStrategy = this._getOverlayPosition(); overlayState.width = this._getHostWidth(); overlayState.direction = this._dir ? this._dir.value : 'ltr'; - overlayState.scrollStrategy = 'reposition'; + overlayState.scrollStrategy = this._scrollStrategyOptions.reposition; return overlayState; } diff --git a/src/lib/core/overlay/overlay-directives.ts b/src/lib/core/overlay/overlay-directives.ts index e09658cbd12d..9c9f3d0baca0 100644 --- a/src/lib/core/overlay/overlay-directives.ts +++ b/src/lib/core/overlay/overlay-directives.ts @@ -25,7 +25,7 @@ import {PortalModule} from '../portal/portal-directives'; import {ConnectedPositionStrategy} from './position/connected-position-strategy'; import {Dir, LayoutDirection} from '../rtl/dir'; import {Scrollable} from './scroll/scrollable'; -import {RepositionScrollStrategy} from './scroll/reposition-scroll-strategy'; +import {ScrollStrategyOptions} from './scroll/scroll-strategy-options'; import {ScrollStrategy} from './scroll/scroll-strategy'; import {coerceBooleanProperty} from '../coercion/boolean-property'; import {ESCAPE} from '../keyboard/keycodes'; @@ -124,7 +124,7 @@ export class ConnectedOverlayDirective implements OnDestroy, OnChanges { @Input() backdropClass: string; /** Strategy to be used when handling scroll events while the overlay is open. */ - @Input() scrollStrategy: OverlayStateScrollStrategy = 'reposition'; + @Input() scrollStrategy: OverlayStateScrollStrategy = this._scrollStrategyOptions.reposition; /** Whether the overlay is open. */ @Input() open: boolean = false; @@ -156,6 +156,7 @@ export class ConnectedOverlayDirective implements OnDestroy, OnChanges { constructor( private _overlay: Overlay, private _renderer: Renderer2, + private _scrollStrategyOptions: ScrollStrategyOptions, templateRef: TemplateRef, viewContainerRef: ViewContainerRef, @Optional() private _dir: Dir) { diff --git a/src/lib/core/overlay/overlay-ref.ts b/src/lib/core/overlay/overlay-ref.ts index cb61529c3457..2fdfd2cf8d2f 100644 --- a/src/lib/core/overlay/overlay-ref.ts +++ b/src/lib/core/overlay/overlay-ref.ts @@ -23,9 +23,11 @@ export class OverlayRef implements PortalHost { private _scrollStrategy: ScrollStrategy, private _ngZone: NgZone) { - let scrollStrategyConfig = typeof _state.scrollStrategy === 'string' ? - null : - _state.scrollStrategy.config; + let scrollStrategyConfig = null; + + if (_state.scrollStrategy && typeof _state.scrollStrategy !== 'function') { + scrollStrategyConfig = _state.scrollStrategy.config; + } _scrollStrategy.attach(this, scrollStrategyConfig); } diff --git a/src/lib/core/overlay/overlay-state.ts b/src/lib/core/overlay/overlay-state.ts index 336dfc1ce9fd..a4de5d42586b 100644 --- a/src/lib/core/overlay/overlay-state.ts +++ b/src/lib/core/overlay/overlay-state.ts @@ -1,9 +1,9 @@ import {PositionStrategy} from './position/position-strategy'; import {LayoutDirection} from '../rtl/dir'; -import {ScrollStrategy} from './scroll/scroll-strategy'; -import {NoopScrollStrategy} from './scroll/noop-scroll-strategy'; +import {ScrollStrategyOption} from './scroll/scroll-strategy-options'; -export type OverlayStateScrollStrategy = string | {name: string; config: any}; +export type OverlayStateScrollStrategy = {strategy: ScrollStrategyOption; config: any} | + ScrollStrategyOption; /** * OverlayState is a bag of values for either the initial configuration or current state of an @@ -14,7 +14,7 @@ export class OverlayState { positionStrategy: PositionStrategy; /** Strategy to be used when handling scroll events while the overlay is open. */ - scrollStrategy: OverlayStateScrollStrategy = 'noop'; + scrollStrategy: OverlayStateScrollStrategy; /** Custom class to add to the overlay pane. */ panelClass: string = ''; diff --git a/src/lib/core/overlay/overlay.spec.ts b/src/lib/core/overlay/overlay.spec.ts index 099cb6fc5ce3..4e8cca50e821 100644 --- a/src/lib/core/overlay/overlay.spec.ts +++ b/src/lib/core/overlay/overlay.spec.ts @@ -8,8 +8,13 @@ import {OverlayState} from './overlay-state'; import {OverlayRef} from './overlay-ref'; import {PositionStrategy} from './position/position-strategy'; import {OverlayModule} from './overlay-directives'; -import {ScrollStrategy, ScrollStrategyOptions, ScrollDispatcher} from './scroll/index'; import {ViewportRuler} from './position/viewport-ruler'; +import { + ScrollStrategy, + ScrollStrategyOptions, + ScrollStrategyOption, + ScrollDispatcher, +} from './scroll/index'; describe('Overlay', () => { @@ -370,7 +375,7 @@ describe('Overlay', () => { beforeEach(inject([ScrollStrategyOptions], (scrollOptions: ScrollStrategyOptionsOverride) => { config = new OverlayState(); - config.scrollStrategy = 'fake'; + config.scrollStrategy = scrollOptions.fake; overlayRef = overlay.create(config); fakeScrollStrategy = scrollOptions.instances[scrollOptions.instances.length - 1] as FakeScrollStrategy; @@ -500,8 +505,8 @@ class ScrollStrategyOptionsOverride extends ScrollStrategyOptions { // 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); + fake: ScrollStrategyOption = () => { + let instance = new FakeScrollStrategy(); this.instances.push(instance); return instance; } diff --git a/src/lib/core/overlay/overlay.ts b/src/lib/core/overlay/overlay.ts index 55b21e922281..d70064b78128 100644 --- a/src/lib/core/overlay/overlay.ts +++ b/src/lib/core/overlay/overlay.ts @@ -86,14 +86,24 @@ 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 scrollStrategy = this._createScrollStrategy(state); let portalHost = this._createPortalHost(pane); return new OverlayRef(portalHost, pane, state, scrollStrategy, this._ngZone); } + + /** + * Creates a scroll strategy for the given overlay state. + * @param state + */ + private _createScrollStrategy(state: OverlayState): ScrollStrategy { + if (state.scrollStrategy) { + return typeof state.scrollStrategy === 'function' ? + state.scrollStrategy() : + state.scrollStrategy.strategy(); + } + + return this._scrollStrategyOptions.noop(); + } } /** Providers for Overlay and its related injectables. */ diff --git a/src/lib/core/overlay/scroll/block-scroll-strategy.spec.ts b/src/lib/core/overlay/scroll/block-scroll-strategy.spec.ts index 895fbab5a797..b31ae23bb585 100644 --- a/src/lib/core/overlay/scroll/block-scroll-strategy.spec.ts +++ b/src/lib/core/overlay/scroll/block-scroll-strategy.spec.ts @@ -9,6 +9,7 @@ import { OverlayState, Overlay, OverlayRef, + ScrollStrategyOptions, } from '../../core'; @@ -25,19 +26,20 @@ describe('BlockScrollStrategy', () => { }).compileComponents(); })); - beforeEach(inject([Overlay, ViewportRuler], (overlay: Overlay, viewportRuler: ViewportRuler) => { - let overlayState = new OverlayState(); + beforeEach(inject([Overlay, ViewportRuler, ScrollStrategyOptions], + (o: Overlay, v: ViewportRuler, sso: ScrollStrategyOptions) => { + let overlayState = new OverlayState(); - overlayState.scrollStrategy = 'block'; - overlayRef = overlay.create(overlayState); - componentPortal = new ComponentPortal(FocacciaMsg); + overlayState.scrollStrategy = sso.block; + overlayRef = o.create(overlayState); + componentPortal = new ComponentPortal(FocacciaMsg); - viewport = viewportRuler; - forceScrollElement = document.createElement('div'); - document.body.appendChild(forceScrollElement); - forceScrollElement.style.width = '100px'; - forceScrollElement.style.height = '3000px'; - })); + viewport = v; + forceScrollElement = document.createElement('div'); + document.body.appendChild(forceScrollElement); + forceScrollElement.style.width = '100px'; + forceScrollElement.style.height = '3000px'; + })); afterEach(() => { overlayRef.dispose(); diff --git a/src/lib/core/overlay/scroll/close-scroll-strategy.spec.ts b/src/lib/core/overlay/scroll/close-scroll-strategy.spec.ts index ce4aa1074dd3..decbcf6a6d83 100644 --- a/src/lib/core/overlay/scroll/close-scroll-strategy.spec.ts +++ b/src/lib/core/overlay/scroll/close-scroll-strategy.spec.ts @@ -10,6 +10,7 @@ import { OverlayModule, ScrollStrategy, ScrollDispatcher, + ScrollStrategyOptions, } from '../../core'; @@ -33,10 +34,10 @@ describe('CloseScrollStrategy', () => { TestBed.compileComponents(); })); - beforeEach(inject([Overlay], (overlay: Overlay) => { + beforeEach(inject([Overlay, ScrollStrategyOptions], (o: Overlay, sso: ScrollStrategyOptions) => { let overlayState = new OverlayState(); - overlayState.scrollStrategy = 'close'; - overlayRef = overlay.create(overlayState); + overlayState.scrollStrategy = sso.close; + overlayRef = o.create(overlayState); componentPortal = new ComponentPortal(MozarellaMsg); })); diff --git a/src/lib/core/overlay/scroll/index.ts b/src/lib/core/overlay/scroll/index.ts index acf5039b6188..74deb21983a1 100644 --- a/src/lib/core/overlay/scroll/index.ts +++ b/src/lib/core/overlay/scroll/index.ts @@ -9,7 +9,7 @@ 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 {ScrollStrategyOptions, ScrollStrategyOption} from './scroll-strategy-options'; export {RepositionScrollStrategy} from './reposition-scroll-strategy'; export {CloseScrollStrategy} from './close-scroll-strategy'; export {NoopScrollStrategy} from './noop-scroll-strategy'; diff --git a/src/lib/core/overlay/scroll/reposition-scroll-strategy.spec.ts b/src/lib/core/overlay/scroll/reposition-scroll-strategy.spec.ts index 9591b3e9dc07..f222788b8099 100644 --- a/src/lib/core/overlay/scroll/reposition-scroll-strategy.spec.ts +++ b/src/lib/core/overlay/scroll/reposition-scroll-strategy.spec.ts @@ -10,6 +10,7 @@ import { OverlayModule, ScrollStrategy, ScrollDispatcher, + ScrollStrategyOptions, } from '../../core'; @@ -33,10 +34,10 @@ describe('RepositionScrollStrategy', () => { TestBed.compileComponents(); })); - beforeEach(inject([Overlay], (overlay: Overlay) => { + beforeEach(inject([Overlay, ScrollStrategyOptions], (o: Overlay, sso: ScrollStrategyOptions) => { let overlayState = new OverlayState(); - overlayState.scrollStrategy = 'reposition'; - overlayRef = overlay.create(overlayState); + overlayState.scrollStrategy = sso.reposition; + overlayRef = o.create(overlayState); componentPortal = new ComponentPortal(PastaMsg); })); diff --git a/src/lib/core/overlay/scroll/scroll-strategy-options.ts b/src/lib/core/overlay/scroll/scroll-strategy-options.ts index caac29a7ec87..3a19faefe65b 100644 --- a/src/lib/core/overlay/scroll/scroll-strategy-options.ts +++ b/src/lib/core/overlay/scroll/scroll-strategy-options.ts @@ -7,6 +7,8 @@ import {BlockScrollStrategy} from './block-scroll-strategy'; import {ScrollDispatcher} from './scroll-dispatcher'; import {ViewportRuler} from '../position/viewport-ruler'; +export type ScrollStrategyOption = () => ScrollStrategy; + /** * Factory that instantiates scroll strategies. Provides the built-in `reposition`, `close`, @@ -18,14 +20,8 @@ export class ScrollStrategyOptions { 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}".`); - } + noop: ScrollStrategyOption = () => new NoopScrollStrategy(); + close: ScrollStrategyOption = () => new CloseScrollStrategy(this._scrollDispatcher); + block: ScrollStrategyOption = () => new BlockScrollStrategy(this._viewportRuler); + reposition: ScrollStrategyOption = () => new RepositionScrollStrategy(this._scrollDispatcher); } diff --git a/src/lib/core/overlay/scroll/scroll-strategy.md b/src/lib/core/overlay/scroll/scroll-strategy.md index ab359cf015a8..5efcd16e648d 100644 --- a/src/lib/core/overlay/scroll/scroll-strategy.md +++ b/src/lib/core/overlay/scroll/scroll-strategy.md @@ -6,14 +6,14 @@ while the overlay is open. The strategy has a reference to the `OverlayRef`, all recalculate the position, close the overlay, block scrolling, etc. ## Usage -To associate an overlay with a scroll strategy, you have to pass in the name of the scroll strategy -to the `OverlayState`. By default, all overlays will use the `noop` strategy which doesn't do -anything. The other available strategies are `reposition`, `block` and `close`: +To associate an overlay with a scroll strategy, you have to pass in a function, that returns a +scroll strategy, to the `OverlayState`. By default, all overlays will use the `noop` strategy which +doesn't do anything. The other available strategies are `reposition`, `block` and `close`: ```ts let overlayState = new OverlayState(); -overlayState.scrollStrategy = 'block'; +overlayState.scrollStrategy = scrollStrategyOptions.block; this._overlay.create(overlayState).attach(yourPortal); ``` @@ -34,6 +34,7 @@ import {NgModule} from '@angular/core'; import { ScrollStrategy, ScrollStrategyOptions, + ScrollStrategyOption, ScrollDispatcher, ViewportRuler, } from '@angular/material'; @@ -49,13 +50,7 @@ class ScrollStrategyOptionsOverride extends ScrollStrategyOptions { super(scrollDispatcher, viewportRuler); } - get(strategy: string): ScrollStrategy { - if (strategy === 'custom') { - return new CustomScrollStrategy(); - } - - return super.get(strategy); - } + custom: ScrollStrategyOption = () => new CustomScrollStrategy(); } // Register the provider with your module. diff --git a/src/lib/datepicker/datepicker.ts b/src/lib/datepicker/datepicker.ts index 7e518361da65..7e290b26e8c1 100644 --- a/src/lib/datepicker/datepicker.ts +++ b/src/lib/datepicker/datepicker.ts @@ -25,6 +25,7 @@ import {MdDatepickerInput} from './datepicker-input'; import {Subscription} from 'rxjs/Subscription'; import {MdDialogConfig} from '../dialog/dialog-config'; import {DateAdapter} from '../core/datetime/index'; +import {ScrollStrategyOptions} from '../core/overlay/scroll/index'; import {createMissingDateImplError} from './datepicker-errors'; import {ESCAPE} from '../core/keyboard/keycodes'; import {MdCalendar} from './calendar'; @@ -156,6 +157,7 @@ export class MdDatepicker implements OnDestroy { private _overlay: Overlay, private _ngZone: NgZone, private _viewContainerRef: ViewContainerRef, + private _scrollStrategyOptions: ScrollStrategyOptions, @Optional() private _dateAdapter: DateAdapter, @Optional() private _dir: Dir) { if (!this._dateAdapter) { @@ -267,7 +269,7 @@ export class MdDatepicker implements OnDestroy { overlayState.hasBackdrop = true; overlayState.backdropClass = 'md-overlay-transparent-backdrop'; overlayState.direction = this._dir ? this._dir.value : 'ltr'; - overlayState.scrollStrategy = 'reposition'; + overlayState.scrollStrategy = this._scrollStrategyOptions.reposition; this._popupRef = this._overlay.create(overlayState); } diff --git a/src/lib/dialog/dialog.ts b/src/lib/dialog/dialog.ts index b6607014cba6..140eace73745 100644 --- a/src/lib/dialog/dialog.ts +++ b/src/lib/dialog/dialog.ts @@ -2,7 +2,14 @@ import {Injector, ComponentRef, Injectable, Optional, SkipSelf, TemplateRef} fro import {Location} from '@angular/common'; import {Observable} from 'rxjs/Observable'; import {Subject} from 'rxjs/Subject'; -import {Overlay, OverlayRef, ComponentType, OverlayState, ComponentPortal} from '../core'; +import { + Overlay, + OverlayRef, + ComponentType, + OverlayState, + ComponentPortal, + ScrollStrategyOptions, +} from '../core'; import {extendObject} from '../core/util/object-extend'; import {ESCAPE} from '../core/keyboard/keycodes'; import {DialogInjector} from './dialog-injector'; @@ -48,6 +55,7 @@ export class MdDialog { constructor( private _overlay: Overlay, private _injector: Injector, + private _scrollStrategyOptions: ScrollStrategyOptions, @Optional() private _location: Location, @Optional() @SkipSelf() private _parentDialog: MdDialog) { @@ -120,7 +128,7 @@ export class MdDialog { let overlayState = new OverlayState(); overlayState.panelClass = dialogConfig.panelClass; overlayState.hasBackdrop = dialogConfig.hasBackdrop; - overlayState.scrollStrategy = 'block'; + overlayState.scrollStrategy = this._scrollStrategyOptions.block; if (dialogConfig.backdropClass) { overlayState.backdropClass = dialogConfig.backdropClass; } diff --git a/src/lib/menu/menu-trigger.ts b/src/lib/menu/menu-trigger.ts index 365bb1fc9e64..1291273a1ff9 100644 --- a/src/lib/menu/menu-trigger.ts +++ b/src/lib/menu/menu-trigger.ts @@ -22,6 +22,7 @@ import { ConnectedPositionStrategy, HorizontalConnectionPos, VerticalConnectionPos, + ScrollStrategyOptions, } from '../core'; import {Subscription} from 'rxjs/Subscription'; import {MenuPositionX, MenuPositionY} from './menu-positions'; @@ -78,6 +79,7 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy { @Output() onMenuClose = new EventEmitter(); constructor(private _overlay: Overlay, private _element: ElementRef, + private _scrollStrategyOptions: ScrollStrategyOptions, private _viewContainerRef: ViewContainerRef, @Optional() private _dir: Dir) { } ngAfterViewInit() { @@ -216,7 +218,7 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy { overlayState.hasBackdrop = true; overlayState.backdropClass = 'cdk-overlay-transparent-backdrop'; overlayState.direction = this.dir; - overlayState.scrollStrategy = 'reposition'; + overlayState.scrollStrategy = this._scrollStrategyOptions.reposition; return overlayState; } diff --git a/src/lib/tooltip/tooltip.ts b/src/lib/tooltip/tooltip.ts index 40f4f3305ce0..49de1f38590a 100644 --- a/src/lib/tooltip/tooltip.ts +++ b/src/lib/tooltip/tooltip.ts @@ -25,6 +25,7 @@ import { ComponentPortal, OverlayConnectionPosition, OriginConnectionPosition, + ScrollStrategyOptions, } from '../core'; import {Observable} from 'rxjs/Observable'; import {Subject} from 'rxjs/Subject'; @@ -155,6 +156,7 @@ export class MdTooltip implements OnDestroy { private _ngZone: NgZone, private _renderer: Renderer2, private _platform: Platform, + private _scrollStrategyOptions: ScrollStrategyOptions, @Optional() private _dir: Dir) { // The mouse events shouldn't be bound on iOS devices, because @@ -240,7 +242,7 @@ export class MdTooltip implements OnDestroy { config.direction = this._dir ? this._dir.value : 'ltr'; config.positionStrategy = strategy; config.scrollStrategy = { - name: 'reposition', + strategy: this._scrollStrategyOptions.reposition, config: { scrollThrottle: SCROLL_THROTTLE_MS } }; From accf5594d06c37810d4c8cb733923fcfe1c5b0c1 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Fri, 2 Jun 2017 21:10:56 +0200 Subject: [PATCH 4/5] refactor: switch to even simpler approach --- .../block-scroll-strategy-e2e.ts | 6 +-- src/lib/autocomplete/autocomplete-trigger.ts | 5 +- src/lib/core/overlay/overlay-directives.ts | 6 +-- src/lib/core/overlay/overlay-ref.ts | 8 +-- src/lib/core/overlay/overlay-state.ts | 6 +-- src/lib/core/overlay/overlay.spec.ts | 53 ++++--------------- src/lib/core/overlay/overlay.ts | 20 ++----- .../scroll/block-scroll-strategy.spec.ts | 10 ++-- .../scroll/close-scroll-strategy.spec.ts | 7 ++- src/lib/core/overlay/scroll/index.ts | 2 +- .../scroll/reposition-scroll-strategy.spec.ts | 7 ++- .../scroll/reposition-scroll-strategy.ts | 8 +-- .../overlay/scroll/scroll-strategy-options.ts | 17 +++--- .../core/overlay/scroll/scroll-strategy.md | 36 ++----------- .../core/overlay/scroll/scroll-strategy.ts | 2 +- src/lib/datepicker/datepicker.ts | 4 +- src/lib/dialog/dialog.ts | 4 +- src/lib/menu/menu-trigger.ts | 4 +- src/lib/tooltip/tooltip.ts | 9 ++-- 19 files changed, 58 insertions(+), 156 deletions(-) diff --git a/src/e2e-app/block-scroll-strategy/block-scroll-strategy-e2e.ts b/src/e2e-app/block-scroll-strategy/block-scroll-strategy-e2e.ts index ba38427d3e46..927ba58b006b 100644 --- a/src/e2e-app/block-scroll-strategy/block-scroll-strategy-e2e.ts +++ b/src/e2e-app/block-scroll-strategy/block-scroll-strategy-e2e.ts @@ -1,5 +1,5 @@ import {Component} from '@angular/core'; -import {ScrollStrategyOptions, ScrollStrategy} from '@angular/material'; +import {Overlay, ScrollStrategy} from '@angular/material'; @Component({ moduleId: module.id, @@ -8,6 +8,6 @@ import {ScrollStrategyOptions, ScrollStrategy} from '@angular/material'; styleUrls: ['block-scroll-strategy-e2e.css'], }) export class BlockScrollStrategyE2E { - constructor(private _scrollStrategyOptions: ScrollStrategyOptions) { } - scrollStrategy: ScrollStrategy = this._scrollStrategyOptions.block(); + constructor(private _overlay: Overlay) { } + scrollStrategy: ScrollStrategy = this._overlay.scrollStrategies.block(); } diff --git a/src/lib/autocomplete/autocomplete-trigger.ts b/src/lib/autocomplete/autocomplete-trigger.ts index 646091cb66eb..1ccf317ab1ae 100644 --- a/src/lib/autocomplete/autocomplete-trigger.ts +++ b/src/lib/autocomplete/autocomplete-trigger.ts @@ -13,7 +13,7 @@ import { } from '@angular/core'; import {ControlValueAccessor, NG_VALUE_ACCESSOR} from '@angular/forms'; import {DOCUMENT} from '@angular/platform-browser'; -import {Overlay, OverlayRef, OverlayState, TemplatePortal, ScrollStrategyOptions} from '../core'; +import {Overlay, OverlayRef, OverlayState, TemplatePortal} from '../core'; import {MdAutocomplete} from './autocomplete'; import {PositionStrategy} from '../core/overlay/position/position-strategy'; import {ConnectedPositionStrategy} from '../core/overlay/position/connected-position-strategy'; @@ -103,7 +103,6 @@ export class MdAutocompleteTrigger implements ControlValueAccessor, OnDestroy { constructor(private _element: ElementRef, private _overlay: Overlay, private _viewContainerRef: ViewContainerRef, private _changeDetectorRef: ChangeDetectorRef, - private _scrollStrategyOptions: ScrollStrategyOptions, @Optional() private _dir: Dir, private _zone: NgZone, @Optional() @Host() private _inputContainer: MdInputContainer, @Optional() @Inject(DOCUMENT) private _document: any) {} @@ -367,7 +366,7 @@ export class MdAutocompleteTrigger implements ControlValueAccessor, OnDestroy { overlayState.positionStrategy = this._getOverlayPosition(); overlayState.width = this._getHostWidth(); overlayState.direction = this._dir ? this._dir.value : 'ltr'; - overlayState.scrollStrategy = this._scrollStrategyOptions.reposition; + overlayState.scrollStrategy = this._overlay.scrollStrategies.reposition(); return overlayState; } diff --git a/src/lib/core/overlay/overlay-directives.ts b/src/lib/core/overlay/overlay-directives.ts index 9c9f3d0baca0..a7a66df277b8 100644 --- a/src/lib/core/overlay/overlay-directives.ts +++ b/src/lib/core/overlay/overlay-directives.ts @@ -16,7 +16,7 @@ import { import {Overlay, OVERLAY_PROVIDERS} from './overlay'; import {OverlayRef} from './overlay-ref'; import {TemplatePortal} from '../portal/portal'; -import {OverlayState, OverlayStateScrollStrategy} from './overlay-state'; +import {OverlayState} from './overlay-state'; import { ConnectionPositionPair, ConnectedOverlayPositionChange @@ -25,7 +25,6 @@ import {PortalModule} from '../portal/portal-directives'; import {ConnectedPositionStrategy} from './position/connected-position-strategy'; import {Dir, LayoutDirection} from '../rtl/dir'; import {Scrollable} from './scroll/scrollable'; -import {ScrollStrategyOptions} from './scroll/scroll-strategy-options'; import {ScrollStrategy} from './scroll/scroll-strategy'; import {coerceBooleanProperty} from '../coercion/boolean-property'; import {ESCAPE} from '../keyboard/keycodes'; @@ -124,7 +123,7 @@ export class ConnectedOverlayDirective implements OnDestroy, OnChanges { @Input() backdropClass: string; /** Strategy to be used when handling scroll events while the overlay is open. */ - @Input() scrollStrategy: OverlayStateScrollStrategy = this._scrollStrategyOptions.reposition; + @Input() scrollStrategy: ScrollStrategy = this._overlay.scrollStrategies.reposition(); /** Whether the overlay is open. */ @Input() open: boolean = false; @@ -156,7 +155,6 @@ export class ConnectedOverlayDirective implements OnDestroy, OnChanges { constructor( private _overlay: Overlay, private _renderer: Renderer2, - private _scrollStrategyOptions: ScrollStrategyOptions, templateRef: TemplateRef, viewContainerRef: ViewContainerRef, @Optional() private _dir: Dir) { diff --git a/src/lib/core/overlay/overlay-ref.ts b/src/lib/core/overlay/overlay-ref.ts index 2fdfd2cf8d2f..23b435e8d50d 100644 --- a/src/lib/core/overlay/overlay-ref.ts +++ b/src/lib/core/overlay/overlay-ref.ts @@ -23,13 +23,7 @@ export class OverlayRef implements PortalHost { private _scrollStrategy: ScrollStrategy, private _ngZone: NgZone) { - let scrollStrategyConfig = null; - - if (_state.scrollStrategy && typeof _state.scrollStrategy !== 'function') { - scrollStrategyConfig = _state.scrollStrategy.config; - } - - _scrollStrategy.attach(this, scrollStrategyConfig); + _scrollStrategy.attach(this); } /** The overlay's HTML element */ diff --git a/src/lib/core/overlay/overlay-state.ts b/src/lib/core/overlay/overlay-state.ts index a4de5d42586b..930e4293f974 100644 --- a/src/lib/core/overlay/overlay-state.ts +++ b/src/lib/core/overlay/overlay-state.ts @@ -1,9 +1,7 @@ import {PositionStrategy} from './position/position-strategy'; import {LayoutDirection} from '../rtl/dir'; -import {ScrollStrategyOption} from './scroll/scroll-strategy-options'; +import {ScrollStrategy} from './scroll/scroll-strategy'; -export type OverlayStateScrollStrategy = {strategy: ScrollStrategyOption; config: any} | - ScrollStrategyOption; /** * OverlayState is a bag of values for either the initial configuration or current state of an @@ -14,7 +12,7 @@ export class OverlayState { positionStrategy: PositionStrategy; /** Strategy to be used when handling scroll events while the overlay is open. */ - scrollStrategy: OverlayStateScrollStrategy; + scrollStrategy: ScrollStrategy; /** Custom class to add to the overlay pane. */ panelClass: string = ''; diff --git a/src/lib/core/overlay/overlay.spec.ts b/src/lib/core/overlay/overlay.spec.ts index 4e8cca50e821..fd2d0140fb56 100644 --- a/src/lib/core/overlay/overlay.spec.ts +++ b/src/lib/core/overlay/overlay.spec.ts @@ -9,12 +9,7 @@ import {OverlayRef} from './overlay-ref'; import {PositionStrategy} from './position/position-strategy'; import {OverlayModule} from './overlay-directives'; import {ViewportRuler} from './position/viewport-ruler'; -import { - ScrollStrategy, - ScrollStrategyOptions, - ScrollStrategyOption, - ScrollDispatcher, -} from './scroll/index'; +import {ScrollStrategy, ScrollDispatcher} from './scroll/index'; describe('Overlay', () => { @@ -27,22 +22,13 @@ describe('Overlay', () => { beforeEach(async(() => { TestBed.configureTestingModule({ imports: [OverlayModule, PortalModule, OverlayTestModule], - providers: [ - ScrollDispatcher, - ViewportRuler, - { - provide: ScrollStrategyOptions, - useClass: ScrollStrategyOptionsOverride, - deps: [ScrollDispatcher, ViewportRuler] - }, - { - provide: OverlayContainer, - useFactory: () => { - overlayContainerElement = document.createElement('div'); - return {getContainerElement: () => overlayContainerElement}; - } + providers: [{ + provide: OverlayContainer, + useFactory: () => { + overlayContainerElement = document.createElement('div'); + return {getContainerElement: () => overlayContainerElement}; } - ] + }] }).compileComponents(); })); @@ -371,15 +357,10 @@ describe('Overlay', () => { let overlayRef: OverlayRef; beforeEach(() => { - }); - - beforeEach(inject([ScrollStrategyOptions], (scrollOptions: ScrollStrategyOptionsOverride) => { config = new OverlayState(); - config.scrollStrategy = scrollOptions.fake; + fakeScrollStrategy = config.scrollStrategy = new FakeScrollStrategy(); overlayRef = overlay.create(config); - fakeScrollStrategy = - scrollOptions.instances[scrollOptions.instances.length - 1] as FakeScrollStrategy; - })); + }); it('should attach the overlay ref to the scroll strategy', () => { expect(fakeScrollStrategy.overlayRef).toBe(overlayRef, @@ -495,19 +476,3 @@ class FakeScrollStrategy implements ScrollStrategy { 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[] = []; - - fake: ScrollStrategyOption = () => { - let instance = new FakeScrollStrategy(); - this.instances.push(instance); - return instance; - } -} diff --git a/src/lib/core/overlay/overlay.ts b/src/lib/core/overlay/overlay.ts index d70064b78128..0cdcfd1df12c 100644 --- a/src/lib/core/overlay/overlay.ts +++ b/src/lib/core/overlay/overlay.ts @@ -32,10 +32,10 @@ let defaultState = new OverlayState(); */ @Injectable() export class Overlay { - constructor(private _overlayContainer: OverlayContainer, + constructor(public scrollStrategies: ScrollStrategyOptions, + private _overlayContainer: OverlayContainer, private _componentFactoryResolver: ComponentFactoryResolver, private _positionBuilder: OverlayPositionBuilder, - private _scrollStrategyOptions: ScrollStrategyOptions, private _appRef: ApplicationRef, private _injector: Injector, private _ngZone: NgZone) { } @@ -86,24 +86,10 @@ export class Overlay { * @param state */ private _createOverlayRef(pane: HTMLElement, state: OverlayState): OverlayRef { - let scrollStrategy = this._createScrollStrategy(state); + let scrollStrategy = state.scrollStrategy || this.scrollStrategies.noop(); let portalHost = this._createPortalHost(pane); return new OverlayRef(portalHost, pane, state, scrollStrategy, this._ngZone); } - - /** - * Creates a scroll strategy for the given overlay state. - * @param state - */ - private _createScrollStrategy(state: OverlayState): ScrollStrategy { - if (state.scrollStrategy) { - return typeof state.scrollStrategy === 'function' ? - state.scrollStrategy() : - state.scrollStrategy.strategy(); - } - - return this._scrollStrategyOptions.noop(); - } } /** Providers for Overlay and its related injectables. */ diff --git a/src/lib/core/overlay/scroll/block-scroll-strategy.spec.ts b/src/lib/core/overlay/scroll/block-scroll-strategy.spec.ts index b31ae23bb585..434280144ec7 100644 --- a/src/lib/core/overlay/scroll/block-scroll-strategy.spec.ts +++ b/src/lib/core/overlay/scroll/block-scroll-strategy.spec.ts @@ -9,7 +9,6 @@ import { OverlayState, Overlay, OverlayRef, - ScrollStrategyOptions, } from '../../core'; @@ -26,15 +25,14 @@ describe('BlockScrollStrategy', () => { }).compileComponents(); })); - beforeEach(inject([Overlay, ViewportRuler, ScrollStrategyOptions], - (o: Overlay, v: ViewportRuler, sso: ScrollStrategyOptions) => { + beforeEach(inject([Overlay, ViewportRuler], (overlay: Overlay, viewportRuler: ViewportRuler) => { let overlayState = new OverlayState(); - overlayState.scrollStrategy = sso.block; - overlayRef = o.create(overlayState); + overlayState.scrollStrategy = overlay.scrollStrategies.block(); + overlayRef = overlay.create(overlayState); componentPortal = new ComponentPortal(FocacciaMsg); - viewport = v; + viewport = viewportRuler; forceScrollElement = document.createElement('div'); document.body.appendChild(forceScrollElement); forceScrollElement.style.width = '100px'; diff --git a/src/lib/core/overlay/scroll/close-scroll-strategy.spec.ts b/src/lib/core/overlay/scroll/close-scroll-strategy.spec.ts index decbcf6a6d83..91b55eca1094 100644 --- a/src/lib/core/overlay/scroll/close-scroll-strategy.spec.ts +++ b/src/lib/core/overlay/scroll/close-scroll-strategy.spec.ts @@ -10,7 +10,6 @@ import { OverlayModule, ScrollStrategy, ScrollDispatcher, - ScrollStrategyOptions, } from '../../core'; @@ -34,10 +33,10 @@ describe('CloseScrollStrategy', () => { TestBed.compileComponents(); })); - beforeEach(inject([Overlay, ScrollStrategyOptions], (o: Overlay, sso: ScrollStrategyOptions) => { + beforeEach(inject([Overlay], (overlay: Overlay) => { let overlayState = new OverlayState(); - overlayState.scrollStrategy = sso.close; - overlayRef = o.create(overlayState); + overlayState.scrollStrategy = overlay.scrollStrategies.close(); + overlayRef = overlay.create(overlayState); componentPortal = new ComponentPortal(MozarellaMsg); })); diff --git a/src/lib/core/overlay/scroll/index.ts b/src/lib/core/overlay/scroll/index.ts index 74deb21983a1..acf5039b6188 100644 --- a/src/lib/core/overlay/scroll/index.ts +++ b/src/lib/core/overlay/scroll/index.ts @@ -9,7 +9,7 @@ export {ScrollDispatcher} from './scroll-dispatcher'; // Export pre-defined scroll strategies and interface to build custom ones. export {ScrollStrategy} from './scroll-strategy'; -export {ScrollStrategyOptions, ScrollStrategyOption} from './scroll-strategy-options'; +export {ScrollStrategyOptions} from './scroll-strategy-options'; export {RepositionScrollStrategy} from './reposition-scroll-strategy'; export {CloseScrollStrategy} from './close-scroll-strategy'; export {NoopScrollStrategy} from './noop-scroll-strategy'; diff --git a/src/lib/core/overlay/scroll/reposition-scroll-strategy.spec.ts b/src/lib/core/overlay/scroll/reposition-scroll-strategy.spec.ts index f222788b8099..1f2812a7b06c 100644 --- a/src/lib/core/overlay/scroll/reposition-scroll-strategy.spec.ts +++ b/src/lib/core/overlay/scroll/reposition-scroll-strategy.spec.ts @@ -10,7 +10,6 @@ import { OverlayModule, ScrollStrategy, ScrollDispatcher, - ScrollStrategyOptions, } from '../../core'; @@ -34,10 +33,10 @@ describe('RepositionScrollStrategy', () => { TestBed.compileComponents(); })); - beforeEach(inject([Overlay, ScrollStrategyOptions], (o: Overlay, sso: ScrollStrategyOptions) => { + beforeEach(inject([Overlay], (overlay: Overlay) => { let overlayState = new OverlayState(); - overlayState.scrollStrategy = sso.reposition; - overlayRef = o.create(overlayState); + overlayState.scrollStrategy = overlay.scrollStrategies.reposition(); + overlayRef = overlay.create(overlayState); componentPortal = new ComponentPortal(PastaMsg); })); diff --git a/src/lib/core/overlay/scroll/reposition-scroll-strategy.ts b/src/lib/core/overlay/scroll/reposition-scroll-strategy.ts index c5107c333b72..23a7e70d49e4 100644 --- a/src/lib/core/overlay/scroll/reposition-scroll-strategy.ts +++ b/src/lib/core/overlay/scroll/reposition-scroll-strategy.ts @@ -16,17 +16,17 @@ export interface RepositionScrollStrategyConfig { export class RepositionScrollStrategy implements ScrollStrategy { private _scrollSubscription: Subscription|null = null; private _overlayRef: OverlayRef; - private _config: RepositionScrollStrategyConfig; - constructor(private _scrollDispatcher: ScrollDispatcher) { } + constructor( + private _scrollDispatcher: ScrollDispatcher, + private _config: RepositionScrollStrategyConfig) { } - attach(overlayRef: OverlayRef, config?: RepositionScrollStrategyConfig) { + attach(overlayRef: OverlayRef) { if (this._overlayRef) { throw getMdScrollStrategyAlreadyAttachedError(); } this._overlayRef = overlayRef; - this._config = config; } enable() { diff --git a/src/lib/core/overlay/scroll/scroll-strategy-options.ts b/src/lib/core/overlay/scroll/scroll-strategy-options.ts index 3a19faefe65b..5076ad381777 100644 --- a/src/lib/core/overlay/scroll/scroll-strategy-options.ts +++ b/src/lib/core/overlay/scroll/scroll-strategy-options.ts @@ -1,13 +1,14 @@ 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'; - -export type ScrollStrategyOption = () => ScrollStrategy; +import { + RepositionScrollStrategy, + RepositionScrollStrategyConfig, +} from './reposition-scroll-strategy'; /** @@ -20,8 +21,10 @@ export class ScrollStrategyOptions { private _scrollDispatcher: ScrollDispatcher, private _viewportRuler: ViewportRuler) { } - noop: ScrollStrategyOption = () => new NoopScrollStrategy(); - close: ScrollStrategyOption = () => new CloseScrollStrategy(this._scrollDispatcher); - block: ScrollStrategyOption = () => new BlockScrollStrategy(this._viewportRuler); - reposition: ScrollStrategyOption = () => new RepositionScrollStrategy(this._scrollDispatcher); + noop = () => new NoopScrollStrategy(); + close = () => new CloseScrollStrategy(this._scrollDispatcher); + block = () => new BlockScrollStrategy(this._viewportRuler); + reposition = (config?: RepositionScrollStrategyConfig) => { + return new RepositionScrollStrategy(this._scrollDispatcher, config); + } } diff --git a/src/lib/core/overlay/scroll/scroll-strategy.md b/src/lib/core/overlay/scroll/scroll-strategy.md index 5efcd16e648d..b39772b0b431 100644 --- a/src/lib/core/overlay/scroll/scroll-strategy.md +++ b/src/lib/core/overlay/scroll/scroll-strategy.md @@ -13,7 +13,7 @@ doesn't do anything. The other available strategies are `reposition`, `block` an ```ts let overlayState = new OverlayState(); -overlayState.scrollStrategy = scrollStrategyOptions.block; +overlayState.scrollStrategy = overlay.scrollStrategies.block(); this._overlay.create(overlayState).attach(yourPortal); ``` @@ -26,42 +26,14 @@ interface. There are three stages of a scroll strategy's life cycle: 3. When an overlay is detached from the DOM or destroyed, it'll call the `disable` method on its scroll strategy, allowing it to clean up after itself. -Afterwards you have to override the `ScrollStrategyOptions` provider, which is used to instantiate -the scroll strategies and to handle the dependency injection. +Afterwards you can pass in the new scroll strategy to your overlay state: ```ts -import {NgModule} from '@angular/core'; -import { - ScrollStrategy, - ScrollStrategyOptions, - ScrollStrategyOption, - ScrollDispatcher, - ViewportRuler, -} from '@angular/material'; - // Your custom scroll strategy. export class CustomScrollStrategy implements ScrollStrategy { // your implementation } -// Provider that'll instantiate your custom strategies, as well as the built-in ones from Material. -class ScrollStrategyOptionsOverride extends ScrollStrategyOptions { - constructor(scrollDispatcher: ScrollDispatcher, viewportRuler: ViewportRuler) { - super(scrollDispatcher, viewportRuler); - } - - custom: ScrollStrategyOption = () => new CustomScrollStrategy(); -} - -// Register the provider with your module. -@NgModule({ - providers: [ - { - provide: ScrollStrategyOptions, - useClass: ScrollStrategyOptionsOverride, - deps: [ScrollDispatcher, ViewportRuler] - } - ] -}) -export class YourModule { } +overlayState.scrollStrategy = new CustomScrollStrategy(); +this._overlay.create(overlayState).attach(yourPortal); ``` diff --git a/src/lib/core/overlay/scroll/scroll-strategy.ts b/src/lib/core/overlay/scroll/scroll-strategy.ts index 3f71d48f206c..b2c47ea3821a 100644 --- a/src/lib/core/overlay/scroll/scroll-strategy.ts +++ b/src/lib/core/overlay/scroll/scroll-strategy.ts @@ -7,7 +7,7 @@ import {OverlayRef} from '../overlay-ref'; export interface ScrollStrategy { enable: () => void; disable: () => void; - attach: (overlayRef: OverlayRef, config?: any) => void; + attach: (overlayRef: OverlayRef) => void; } /** diff --git a/src/lib/datepicker/datepicker.ts b/src/lib/datepicker/datepicker.ts index 7e290b26e8c1..e5783aa6bdfa 100644 --- a/src/lib/datepicker/datepicker.ts +++ b/src/lib/datepicker/datepicker.ts @@ -25,7 +25,6 @@ import {MdDatepickerInput} from './datepicker-input'; import {Subscription} from 'rxjs/Subscription'; import {MdDialogConfig} from '../dialog/dialog-config'; import {DateAdapter} from '../core/datetime/index'; -import {ScrollStrategyOptions} from '../core/overlay/scroll/index'; import {createMissingDateImplError} from './datepicker-errors'; import {ESCAPE} from '../core/keyboard/keycodes'; import {MdCalendar} from './calendar'; @@ -157,7 +156,6 @@ export class MdDatepicker implements OnDestroy { private _overlay: Overlay, private _ngZone: NgZone, private _viewContainerRef: ViewContainerRef, - private _scrollStrategyOptions: ScrollStrategyOptions, @Optional() private _dateAdapter: DateAdapter, @Optional() private _dir: Dir) { if (!this._dateAdapter) { @@ -269,7 +267,7 @@ export class MdDatepicker implements OnDestroy { overlayState.hasBackdrop = true; overlayState.backdropClass = 'md-overlay-transparent-backdrop'; overlayState.direction = this._dir ? this._dir.value : 'ltr'; - overlayState.scrollStrategy = this._scrollStrategyOptions.reposition; + overlayState.scrollStrategy = this._overlay.scrollStrategies.reposition(); this._popupRef = this._overlay.create(overlayState); } diff --git a/src/lib/dialog/dialog.ts b/src/lib/dialog/dialog.ts index 140eace73745..fa86910ad900 100644 --- a/src/lib/dialog/dialog.ts +++ b/src/lib/dialog/dialog.ts @@ -8,7 +8,6 @@ import { ComponentType, OverlayState, ComponentPortal, - ScrollStrategyOptions, } from '../core'; import {extendObject} from '../core/util/object-extend'; import {ESCAPE} from '../core/keyboard/keycodes'; @@ -55,7 +54,6 @@ export class MdDialog { constructor( private _overlay: Overlay, private _injector: Injector, - private _scrollStrategyOptions: ScrollStrategyOptions, @Optional() private _location: Location, @Optional() @SkipSelf() private _parentDialog: MdDialog) { @@ -128,7 +126,7 @@ export class MdDialog { let overlayState = new OverlayState(); overlayState.panelClass = dialogConfig.panelClass; overlayState.hasBackdrop = dialogConfig.hasBackdrop; - overlayState.scrollStrategy = this._scrollStrategyOptions.block; + overlayState.scrollStrategy = this._overlay.scrollStrategies.block(); if (dialogConfig.backdropClass) { overlayState.backdropClass = dialogConfig.backdropClass; } diff --git a/src/lib/menu/menu-trigger.ts b/src/lib/menu/menu-trigger.ts index 1291273a1ff9..a54df373ef40 100644 --- a/src/lib/menu/menu-trigger.ts +++ b/src/lib/menu/menu-trigger.ts @@ -22,7 +22,6 @@ import { ConnectedPositionStrategy, HorizontalConnectionPos, VerticalConnectionPos, - ScrollStrategyOptions, } from '../core'; import {Subscription} from 'rxjs/Subscription'; import {MenuPositionX, MenuPositionY} from './menu-positions'; @@ -79,7 +78,6 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy { @Output() onMenuClose = new EventEmitter(); constructor(private _overlay: Overlay, private _element: ElementRef, - private _scrollStrategyOptions: ScrollStrategyOptions, private _viewContainerRef: ViewContainerRef, @Optional() private _dir: Dir) { } ngAfterViewInit() { @@ -218,7 +216,7 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy { overlayState.hasBackdrop = true; overlayState.backdropClass = 'cdk-overlay-transparent-backdrop'; overlayState.direction = this.dir; - overlayState.scrollStrategy = this._scrollStrategyOptions.reposition; + overlayState.scrollStrategy = this._overlay.scrollStrategies.reposition(); return overlayState; } diff --git a/src/lib/tooltip/tooltip.ts b/src/lib/tooltip/tooltip.ts index 49de1f38590a..d8e77732d494 100644 --- a/src/lib/tooltip/tooltip.ts +++ b/src/lib/tooltip/tooltip.ts @@ -25,7 +25,6 @@ import { ComponentPortal, OverlayConnectionPosition, OriginConnectionPosition, - ScrollStrategyOptions, } from '../core'; import {Observable} from 'rxjs/Observable'; import {Subject} from 'rxjs/Subject'; @@ -156,7 +155,6 @@ export class MdTooltip implements OnDestroy { private _ngZone: NgZone, private _renderer: Renderer2, private _platform: Platform, - private _scrollStrategyOptions: ScrollStrategyOptions, @Optional() private _dir: Dir) { // The mouse events shouldn't be bound on iOS devices, because @@ -241,10 +239,9 @@ export class MdTooltip implements OnDestroy { config.direction = this._dir ? this._dir.value : 'ltr'; config.positionStrategy = strategy; - config.scrollStrategy = { - strategy: this._scrollStrategyOptions.reposition, - config: { scrollThrottle: SCROLL_THROTTLE_MS } - }; + config.scrollStrategy = this._overlay.scrollStrategies.reposition({ + scrollThrottle: SCROLL_THROTTLE_MS + }); this._overlayRef = this._overlay.create(config); } From 2087d70b4d13c1a686a2de24f918af18faef3847 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Fri, 2 Jun 2017 22:45:24 +0200 Subject: [PATCH 5/5] chore: address feedback --- .../overlay/scroll/scroll-strategy-options.ts | 22 ++++++++++++++----- .../core/overlay/scroll/scroll-strategy.ts | 2 +- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/lib/core/overlay/scroll/scroll-strategy-options.ts b/src/lib/core/overlay/scroll/scroll-strategy-options.ts index 5076ad381777..9123cfe27fcf 100644 --- a/src/lib/core/overlay/scroll/scroll-strategy-options.ts +++ b/src/lib/core/overlay/scroll/scroll-strategy-options.ts @@ -12,8 +12,10 @@ import { /** - * Factory that instantiates scroll strategies. Provides the built-in `reposition`, `close`, - * `noop` and `block` strategies by default. + * Options for how an overlay will handle scrolling. + * + * Users can provide a custom value for `ScrollStrategyOptions` to replace the default + * behaviors. This class primarily acts as a factory for ScrollStrategy instances. */ @Injectable() export class ScrollStrategyOptions { @@ -21,10 +23,20 @@ export class ScrollStrategyOptions { private _scrollDispatcher: ScrollDispatcher, private _viewportRuler: ViewportRuler) { } + /** Do nothing on scroll. */ noop = () => new NoopScrollStrategy(); + + /** Close the overlay as soon as the user scrolls. */ close = () => new CloseScrollStrategy(this._scrollDispatcher); + + /** Block scrolling. */ block = () => new BlockScrollStrategy(this._viewportRuler); - reposition = (config?: RepositionScrollStrategyConfig) => { - return new RepositionScrollStrategy(this._scrollDispatcher, config); - } + + /** + * Update the overlay's position on scroll. + * @param config Configuration to be used inside the scroll strategy. + * Allows debouncing the reposition calls. + */ + reposition = (config?: RepositionScrollStrategyConfig) => + new RepositionScrollStrategy(this._scrollDispatcher, config) } diff --git a/src/lib/core/overlay/scroll/scroll-strategy.ts b/src/lib/core/overlay/scroll/scroll-strategy.ts index b2c47ea3821a..d8631ffafb0a 100644 --- a/src/lib/core/overlay/scroll/scroll-strategy.ts +++ b/src/lib/core/overlay/scroll/scroll-strategy.ts @@ -14,5 +14,5 @@ export interface ScrollStrategy { * Returns an error to be thrown when attempting to attach an already-attached scroll strategy. */ export function getMdScrollStrategyAlreadyAttachedError(): Error { - return new Error(`Scroll strategy has already been attached.`); + return Error(`Scroll strategy has already been attached.`); }