Skip to content

Commit 7e91270

Browse files
crisbetoandrewseguin
crisbeto
authored and
andrewseguin
committed
feat(overlay): more flexible scroll strategy API and ability to define/override custom strategies (#4855)
* 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. * refactor: switch to new approach without ReflectiveInjector * refactor: switch to better approach * refactor: switch to even simpler approach * chore: address feedback
1 parent 70b31f1 commit 7e91270

21 files changed

+201
-99
lines changed

‎src/e2e-app/block-scroll-strategy/block-scroll-strategy-e2e.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import {Component} from '@angular/core';
2-
import {BlockScrollStrategy, ViewportRuler} from '@angular/material';
2+
import {Overlay, ScrollStrategy} from '@angular/material';
33

44
@Component({
55
moduleId: module.id,
@@ -8,6 +8,6 @@ import {BlockScrollStrategy, ViewportRuler} from '@angular/material';
88
styleUrls: ['block-scroll-strategy-e2e.css'],
99
})
1010
export class BlockScrollStrategyE2E {
11-
constructor(private _viewportRuler: ViewportRuler) { }
12-
scrollStrategy = new BlockScrollStrategy(this._viewportRuler);
11+
constructor(private _overlay: Overlay) { }
12+
scrollStrategy: ScrollStrategy = this._overlay.scrollStrategies.block();
1313
}

‎src/lib/autocomplete/autocomplete-trigger.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {
1313
} from '@angular/core';
1414
import {ControlValueAccessor, NG_VALUE_ACCESSOR} from '@angular/forms';
1515
import {DOCUMENT} from '@angular/platform-browser';
16-
import {Overlay, OverlayRef, OverlayState, TemplatePortal, RepositionScrollStrategy} from '../core';
16+
import {Overlay, OverlayRef, OverlayState, TemplatePortal} from '../core';
1717
import {MdAutocomplete} from './autocomplete';
1818
import {PositionStrategy} from '../core/overlay/position/position-strategy';
1919
import {ConnectedPositionStrategy} from '../core/overlay/position/connected-position-strategy';
@@ -22,7 +22,6 @@ import {MdOptionSelectionChange, MdOption} from '../core/option/option';
2222
import {ENTER, UP_ARROW, DOWN_ARROW, ESCAPE} from '../core/keyboard/keycodes';
2323
import {Dir} from '../core/rtl/dir';
2424
import {MdInputContainer} from '../input/input-container';
25-
import {ScrollDispatcher} from '../core/overlay/scroll/scroll-dispatcher';
2625
import {Subscription} from 'rxjs/Subscription';
2726
import 'rxjs/add/observable/merge';
2827
import 'rxjs/add/observable/fromEvent';
@@ -113,7 +112,6 @@ export class MdAutocompleteTrigger implements ControlValueAccessor, OnDestroy {
113112
constructor(private _element: ElementRef, private _overlay: Overlay,
114113
private _viewContainerRef: ViewContainerRef,
115114
private _changeDetectorRef: ChangeDetectorRef,
116-
private _scrollDispatcher: ScrollDispatcher,
117115
@Optional() private _dir: Dir, private _zone: NgZone,
118116
@Optional() @Host() private _inputContainer: MdInputContainer,
119117
@Optional() @Inject(DOCUMENT) private _document: any) {}
@@ -381,7 +379,7 @@ export class MdAutocompleteTrigger implements ControlValueAccessor, OnDestroy {
381379
overlayState.positionStrategy = this._getOverlayPosition();
382380
overlayState.width = this._getHostWidth();
383381
overlayState.direction = this._dir ? this._dir.value : 'ltr';
384-
overlayState.scrollStrategy = new RepositionScrollStrategy(this._scrollDispatcher);
382+
overlayState.scrollStrategy = this._overlay.scrollStrategies.reposition();
385383
return overlayState;
386384
}
387385

‎src/lib/core/overlay/overlay-directives.ts‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,9 @@ import {PortalModule} from '../portal/portal-directives';
2525
import {ConnectedPositionStrategy} from './position/connected-position-strategy';
2626
import {Dir, LayoutDirection} from '../rtl/dir';
2727
import {Scrollable} from './scroll/scrollable';
28-
import {RepositionScrollStrategy} from './scroll/reposition-scroll-strategy';
2928
import {ScrollStrategy} from './scroll/scroll-strategy';
3029
import {coerceBooleanProperty} from '../coercion/boolean-property';
3130
import {ESCAPE} from '../keyboard/keycodes';
32-
import {ScrollDispatcher} from './scroll/scroll-dispatcher';
3331
import {Subscription} from 'rxjs/Subscription';
3432
import {ScrollDispatchModule} from './scroll/index';
3533

@@ -125,7 +123,7 @@ export class ConnectedOverlayDirective implements OnDestroy, OnChanges {
125123
@Input() backdropClass: string;
126124

127125
/** Strategy to be used when handling scroll events while the overlay is open. */
128-
@Input() scrollStrategy: ScrollStrategy = new RepositionScrollStrategy(this._scrollDispatcher);
126+
@Input() scrollStrategy: ScrollStrategy = this._overlay.scrollStrategies.reposition();
129127

130128
/** Whether the overlay is open. */
131129
@Input() open: boolean = false;
@@ -157,7 +155,6 @@ export class ConnectedOverlayDirective implements OnDestroy, OnChanges {
157155
constructor(
158156
private _overlay: Overlay,
159157
private _renderer: Renderer2,
160-
private _scrollDispatcher: ScrollDispatcher,
161158
templateRef: TemplateRef<any>,
162159
viewContainerRef: ViewContainerRef,
163160
@Optional() private _dir: Dir) {

‎src/lib/core/overlay/overlay-ref.ts‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,10 @@ export class OverlayRef implements PortalHost {
2020
private _portalHost: PortalHost,
2121
private _pane: HTMLElement,
2222
private _state: OverlayState,
23+
private _scrollStrategy: ScrollStrategy,
2324
private _ngZone: NgZone) {
2425

25-
this._state.scrollStrategy.attach(this);
26+
_scrollStrategy.attach(this);
2627
}
2728

2829
/** The overlay's HTML element */
@@ -44,7 +45,7 @@ export class OverlayRef implements PortalHost {
4445
this.updateDirection();
4546
this.updatePosition();
4647
this._attachments.next();
47-
this._state.scrollStrategy.enable();
48+
this._scrollStrategy.enable();
4849

4950
// Enable pointer events for the overlay pane element.
5051
this._togglePointerEvents(true);
@@ -71,7 +72,7 @@ export class OverlayRef implements PortalHost {
7172
// This is necessary because otherwise the pane element will cover the page and disable
7273
// pointer events therefore. Depends on the position strategy and the applied pane boundaries.
7374
this._togglePointerEvents(false);
74-
this._state.scrollStrategy.disable();
75+
this._scrollStrategy.disable();
7576
this._detachments.next();
7677

7778
return this._portalHost.detach();
@@ -85,9 +86,13 @@ export class OverlayRef implements PortalHost {
8586
this._state.positionStrategy.dispose();
8687
}
8788

89+
if (this._scrollStrategy) {
90+
this._scrollStrategy.disable();
91+
this._scrollStrategy = null;
92+
}
93+
8894
this.detachBackdrop();
8995
this._portalHost.dispose();
90-
this._state.scrollStrategy.disable();
9196
this._detachments.next();
9297
this._detachments.complete();
9398
this._attachments.complete();

‎src/lib/core/overlay/overlay-state.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import {PositionStrategy} from './position/position-strategy';
22
import {LayoutDirection} from '../rtl/dir';
33
import {ScrollStrategy} from './scroll/scroll-strategy';
4-
import {NoopScrollStrategy} from './scroll/noop-scroll-strategy';
54

65

76
/**
@@ -13,7 +12,7 @@ export class OverlayState {
1312
positionStrategy: PositionStrategy;
1413

1514
/** Strategy to be used when handling scroll events while the overlay is open. */
16-
scrollStrategy: ScrollStrategy = new NoopScrollStrategy();
15+
scrollStrategy: ScrollStrategy;
1716

1817
/** Custom class to add to the overlay pane. */
1918
panelClass: string = '';

‎src/lib/core/overlay/overlay.spec.ts‎

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ import {OverlayState} from './overlay-state';
88
import {OverlayRef} from './overlay-ref';
99
import {PositionStrategy} from './position/position-strategy';
1010
import {OverlayModule} from './overlay-directives';
11-
import {ScrollStrategy} from './scroll/scroll-strategy';
11+
import {ViewportRuler} from './position/viewport-ruler';
12+
import {ScrollStrategy, ScrollDispatcher} from './scroll/index';
1213

1314

1415
describe('Overlay', () => {
@@ -21,15 +22,14 @@ describe('Overlay', () => {
2122
beforeEach(async(() => {
2223
TestBed.configureTestingModule({
2324
imports: [OverlayModule, PortalModule, OverlayTestModule],
24-
providers: [
25-
{provide: OverlayContainer, useFactory: () => {
25+
providers: [{
26+
provide: OverlayContainer,
27+
useFactory: () => {
2628
overlayContainerElement = document.createElement('div');
2729
return {getContainerElement: () => overlayContainerElement};
28-
}}
29-
]
30-
});
31-
32-
TestBed.compileComponents();
30+
}
31+
}]
32+
}).compileComponents();
3333
}));
3434

3535
beforeEach(inject([Overlay], (o: Overlay) => {
@@ -354,30 +354,25 @@ describe('Overlay', () => {
354354
describe('scroll strategy', () => {
355355
let fakeScrollStrategy: FakeScrollStrategy;
356356
let config: OverlayState;
357+
let overlayRef: OverlayRef;
357358

358359
beforeEach(() => {
359360
config = new OverlayState();
360-
fakeScrollStrategy = new FakeScrollStrategy();
361-
config.scrollStrategy = fakeScrollStrategy;
361+
fakeScrollStrategy = config.scrollStrategy = new FakeScrollStrategy();
362+
overlayRef = overlay.create(config);
362363
});
363364

364365
it('should attach the overlay ref to the scroll strategy', () => {
365-
let overlayRef = overlay.create(config);
366-
367366
expect(fakeScrollStrategy.overlayRef).toBe(overlayRef,
368367
'Expected scroll strategy to have been attached to the current overlay ref.');
369368
});
370369

371370
it('should enable the scroll strategy when the overlay is attached', () => {
372-
let overlayRef = overlay.create(config);
373-
374371
overlayRef.attach(componentPortal);
375372
expect(fakeScrollStrategy.isEnabled).toBe(true, 'Expected scroll strategy to be enabled.');
376373
});
377374

378375
it('should disable the scroll strategy once the overlay is detached', () => {
379-
let overlayRef = overlay.create(config);
380-
381376
overlayRef.attach(componentPortal);
382377
expect(fakeScrollStrategy.isEnabled).toBe(true, 'Expected scroll strategy to be enabled.');
383378

@@ -386,8 +381,6 @@ describe('Overlay', () => {
386381
});
387382

388383
it('should disable the scroll strategy when the overlay is destroyed', () => {
389-
let overlayRef = overlay.create(config);
390-
391384
overlayRef.dispose();
392385
expect(fakeScrollStrategy.isEnabled).toBe(false, 'Expected scroll strategy to be disabled.');
393386
});
@@ -466,6 +459,7 @@ class FakePositionStrategy implements PositionStrategy {
466459
dispose() {}
467460
}
468461

462+
469463
class FakeScrollStrategy implements ScrollStrategy {
470464
isEnabled = false;
471465
overlayRef: OverlayRef;

‎src/lib/core/overlay/overlay.ts‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {OverlayRef} from './overlay-ref';
1212
import {OverlayPositionBuilder} from './position/overlay-position-builder';
1313
import {VIEWPORT_RULER_PROVIDER} from './position/viewport-ruler';
1414
import {OverlayContainer, OVERLAY_CONTAINER_PROVIDER} from './overlay-container';
15+
import {ScrollStrategy, ScrollStrategyOptions} from './scroll/index';
1516

1617

1718
/** Next overlay unique ID. */
@@ -31,12 +32,13 @@ let defaultState = new OverlayState();
3132
*/
3233
@Injectable()
3334
export class Overlay {
34-
constructor(private _overlayContainer: OverlayContainer,
35+
constructor(public scrollStrategies: ScrollStrategyOptions,
36+
private _overlayContainer: OverlayContainer,
3537
private _componentFactoryResolver: ComponentFactoryResolver,
3638
private _positionBuilder: OverlayPositionBuilder,
3739
private _appRef: ApplicationRef,
3840
private _injector: Injector,
39-
private _ngZone: NgZone) {}
41+
private _ngZone: NgZone) { }
4042

4143
/**
4244
* Creates an overlay.
@@ -61,9 +63,9 @@ export class Overlay {
6163
*/
6264
private _createPaneElement(): HTMLElement {
6365
let pane = document.createElement('div');
66+
6467
pane.id = `cdk-overlay-${nextUniqueId++}`;
6568
pane.classList.add('cdk-overlay-pane');
66-
6769
this._overlayContainer.getContainerElement().appendChild(pane);
6870

6971
return pane;
@@ -84,7 +86,9 @@ export class Overlay {
8486
* @param state
8587
*/
8688
private _createOverlayRef(pane: HTMLElement, state: OverlayState): OverlayRef {
87-
return new OverlayRef(this._createPortalHost(pane), pane, state, this._ngZone);
89+
let scrollStrategy = state.scrollStrategy || this.scrollStrategies.noop();
90+
let portalHost = this._createPortalHost(pane);
91+
return new OverlayRef(portalHost, pane, state, scrollStrategy, this._ngZone);
8892
}
8993
}
9094

0 commit comments

Comments
 (0)