From 1ed5a16d1b8d2912b193c6f9881d9fc7feaeb079 Mon Sep 17 00:00:00 2001 From: Andrew Seguin Date: Mon, 12 Dec 2016 16:23:45 -0800 Subject: [PATCH 01/22] feat(scroll): provide directive and service to listen to scrolling --- src/lib/core/scroll/scroll.spec.ts | 76 ++++++++++++++++++++++++++++++ src/lib/core/scroll/scroll.ts | 59 +++++++++++++++++++++++ src/lib/core/scroll/scrollable.ts | 53 +++++++++++++++++++++ 3 files changed, 188 insertions(+) create mode 100644 src/lib/core/scroll/scroll.spec.ts create mode 100644 src/lib/core/scroll/scroll.ts create mode 100644 src/lib/core/scroll/scrollable.ts diff --git a/src/lib/core/scroll/scroll.spec.ts b/src/lib/core/scroll/scroll.spec.ts new file mode 100644 index 000000000000..91821a950a29 --- /dev/null +++ b/src/lib/core/scroll/scroll.spec.ts @@ -0,0 +1,76 @@ +import {inject, TestBed, async, ComponentFixture} from '@angular/core/testing'; +import {NgModule, Component, ViewChild, ElementRef} from '@angular/core'; +import {Scroll} from './scroll'; +import {ScrollModule, Scrollable} from './scrollable'; + +describe('Scrollable', () => { + let scroll: Scroll; + let fixture: ComponentFixture; + + beforeEach(async(() => { + TestBed.configureTestingModule({ + imports: [ScrollModule.forRoot(), ScrollTestModule], + }); + + TestBed.compileComponents(); + })); + + beforeEach(inject([Scroll], (s: Scroll) => { + scroll = s; + + fixture = TestBed.createComponent(ScrollingComponent); + fixture.detectChanges(); + })); + + it('should register the scrollable directive with the scroll service', () => { + const componentScrollable = fixture.componentInstance.scrollable; + expect(scroll.scrollableReferences.has(componentScrollable)).toBe(true); + }); + + it('should deregister the scrollable directive when the component is destroyed', () => { + const componentScrollable = fixture.componentInstance.scrollable; + expect(scroll.scrollableReferences.has(componentScrollable)).toBe(true); + + fixture.destroy(); + expect(scroll.scrollableReferences.has(componentScrollable)).toBe(false); + }); + + it('should notify through the directive and service that a scroll event occurred', () => { + let hasDirectiveScrollNotified = false; + // Listen for notifications from scroll directive + let scrollable = fixture.componentInstance.scrollable; + scrollable.elementScrolled().subscribe(() => { hasDirectiveScrollNotified = true; }); + + // Listen for notifications from scroll service + let hasServiceScrollNotified = false; + scroll.scrolled().subscribe(() => { hasServiceScrollNotified = true; }); + + // Emit a scroll event from the scrolling element in our component. + // This event should be picked up by the scrollable directive and notify. + // The notification should be picked up by the service. + fixture.componentInstance.scrollingElement.nativeElement.dispatchEvent(new Event('scroll')); + + expect(hasDirectiveScrollNotified).toBe(true); + expect(hasServiceScrollNotified).toBe(true); + }); +}); + + +/** Simple component that contains a large div and can be scrolled. */ +@Component({ + template: `
` +}) +class ScrollingComponent { + @ViewChild(Scrollable) scrollable: Scrollable; + @ViewChild('scrollingElement') scrollingElement: ElementRef; +} + +const TEST_COMPONENTS = [ScrollingComponent]; +@NgModule({ + imports: [ScrollModule], + providers: [Scroll], + exports: TEST_COMPONENTS, + declarations: TEST_COMPONENTS, + entryComponents: TEST_COMPONENTS, +}) +class ScrollTestModule { } diff --git a/src/lib/core/scroll/scroll.ts b/src/lib/core/scroll/scroll.ts new file mode 100644 index 000000000000..2e9c823a66d5 --- /dev/null +++ b/src/lib/core/scroll/scroll.ts @@ -0,0 +1,59 @@ +import {Injectable} from '@angular/core'; +import {Scrollable} from './scrollable'; +import {Subject} from 'rxjs/Subject'; +import {Observable} from 'rxjs/Observable'; +import {Subscription} from 'rxjs/Subscription'; + + +/** + * Service contained all registered Scrollable references and emits an event when any one of the + * Scrollable references emit a scrolled event. + */ +@Injectable() +export class Scroll { + /** Subject for notifying that a registered scrollable reference element has been scrolled. */ + _scrolled: Subject = new Subject(); + + /** + * Map of all the scrollable references that are registered with the service and their + * scroll event subscriptions. + */ + scrollableReferences: Map = new Map(); + + constructor() { + // By default, notify a scroll event when the document is scrolled or the window is resized. + window.document.addEventListener('scroll', this._notify.bind(this)); + window.addEventListener('resize', this._notify.bind(this)); + } + + /** + * Registers a Scrollable with the service and listens for its scrolled events. When the + * scrollable is scrolled, the service emits the event in its scrolled observable. + */ + register(scrollable: Scrollable): void { + const scrollSubscription = scrollable.elementScrolled().subscribe(this._notify.bind(this)); + this.scrollableReferences.set(scrollable, scrollSubscription); + } + + /** + * Deregisters a Scrollable reference and unsubscribes from its scroll event observable. + */ + deregister(scrollable: Scrollable): void { + this.scrollableReferences.get(scrollable).unsubscribe(); + this.scrollableReferences.delete(scrollable); + } + + /** + * Returns an observable that emits an event whenever any of the registered Scrollable + * references (or window, document, or body) fire a scrolled event. + * TODO: Add an event limiter that includes throttle with the leading and trailing events. + */ + scrolled(): Observable { + return this._scrolled.asObservable(); + } + + /** Sends a notification that a scroll event has been fired. */ + _notify(e: Event) { + this._scrolled.next(e); + } +} diff --git a/src/lib/core/scroll/scrollable.ts b/src/lib/core/scroll/scrollable.ts new file mode 100644 index 000000000000..5998a8a17874 --- /dev/null +++ b/src/lib/core/scroll/scrollable.ts @@ -0,0 +1,53 @@ +import { + Directive, ElementRef, OnInit, OnDestroy, ModuleWithProviders, + NgModule +} from '@angular/core'; +import {Subject} from 'rxjs/Subject'; +import {Observable} from 'rxjs/Observable'; +import {Scroll} from './scroll'; + + +/** + * Sends an event when the directive's element is scrolled. Registers itself with the Scroll + * service to include itself as part of its collection of scrolling events that it can be listened + * to through the service. + */ +@Directive({ + selector: '[md-scrollable]' +}) +export class Scrollable implements OnInit, OnDestroy { + /** Subject for notifying that the element has been scrolled. */ + private _elementScrolled: Subject = new Subject(); + + constructor(private _elementRef: ElementRef, private _scroll: Scroll) {} + + ngOnInit() { + this._scroll.register(this); + this._elementRef.nativeElement.addEventListener('scroll', (e: Event) => { + this._elementScrolled.next(e); + }); + } + + ngOnDestroy() { + this._scroll.deregister(this); + } + + /** Returns observable that emits an event when the scroll event is fired on the host element. */ + elementScrolled(): Observable { + return this._elementScrolled.asObservable(); + } +} + + +@NgModule({ + exports: [Scrollable], + declarations: [Scrollable], +}) +export class ScrollModule { + static forRoot(): ModuleWithProviders { + return { + ngModule: ScrollModule, + providers: [Scroll] + }; + } +} From 1834c1d0b987e53248fb4be99e5eea798847c87d Mon Sep 17 00:00:00 2001 From: Andrew Seguin Date: Tue, 13 Dec 2016 13:21:29 -0800 Subject: [PATCH 02/22] review response --- src/lib/core/core.ts | 1 + ...roll.spec.ts => scroll-dispatcher.spec.ts} | 20 +++++++++------- .../{scroll.ts => scroll-dispatcher.ts} | 19 ++++++++------- src/lib/core/scroll/scrollable.ts | 24 +++++++------------ src/lib/sidenav/sidenav.ts | 9 +++++++ 5 files changed, 40 insertions(+), 33 deletions(-) rename src/lib/core/scroll/{scroll.spec.ts => scroll-dispatcher.spec.ts} (78%) rename src/lib/core/scroll/{scroll.ts => scroll-dispatcher.ts} (79%) diff --git a/src/lib/core/core.ts b/src/lib/core/core.ts index c8c20f9b6228..ad8a91acb70a 100644 --- a/src/lib/core/core.ts +++ b/src/lib/core/core.ts @@ -7,6 +7,7 @@ import {PortalModule} from './portal/portal-directives'; import {OverlayModule} from './overlay/overlay-directives'; import {A11yModule, A11Y_PROVIDERS} from './a11y/index'; import {OVERLAY_PROVIDERS} from './overlay/overlay'; +import {ScrollDispatcher} from './scroll/scroll-dispatcher'; // RTL diff --git a/src/lib/core/scroll/scroll.spec.ts b/src/lib/core/scroll/scroll-dispatcher.spec.ts similarity index 78% rename from src/lib/core/scroll/scroll.spec.ts rename to src/lib/core/scroll/scroll-dispatcher.spec.ts index 91821a950a29..2d476e22deaa 100644 --- a/src/lib/core/scroll/scroll.spec.ts +++ b/src/lib/core/scroll/scroll-dispatcher.spec.ts @@ -1,10 +1,10 @@ import {inject, TestBed, async, ComponentFixture} from '@angular/core/testing'; import {NgModule, Component, ViewChild, ElementRef} from '@angular/core'; -import {Scroll} from './scroll'; +import {ScrollDispatcher} from './scroll-dispatcher'; import {ScrollModule, Scrollable} from './scrollable'; -describe('Scrollable', () => { - let scroll: Scroll; +describe('Scroll Dispatcher', () => { + let scroll: ScrollDispatcher; let fixture: ComponentFixture; beforeEach(async(() => { @@ -15,19 +15,19 @@ describe('Scrollable', () => { TestBed.compileComponents(); })); - beforeEach(inject([Scroll], (s: Scroll) => { + beforeEach(inject([ScrollDispatcher], (s: ScrollDispatcher) => { scroll = s; fixture = TestBed.createComponent(ScrollingComponent); fixture.detectChanges(); })); - it('should register the scrollable directive with the scroll service', () => { + it('should be registered with the scrollable directive with the scroll service', () => { const componentScrollable = fixture.componentInstance.scrollable; expect(scroll.scrollableReferences.has(componentScrollable)).toBe(true); }); - it('should deregister the scrollable directive when the component is destroyed', () => { + it('should have the scrollable directive deregistered when the component is destroyed', () => { const componentScrollable = fixture.componentInstance.scrollable; expect(scroll.scrollableReferences.has(componentScrollable)).toBe(true); @@ -48,7 +48,9 @@ describe('Scrollable', () => { // Emit a scroll event from the scrolling element in our component. // This event should be picked up by the scrollable directive and notify. // The notification should be picked up by the service. - fixture.componentInstance.scrollingElement.nativeElement.dispatchEvent(new Event('scroll')); + const scrollEvent = document.createEvent('UIEvents'); + scrollEvent.initUIEvent('scroll', true, true, window, 0); + fixture.componentInstance.scrollingElement.nativeElement.dispatchEvent(scrollEvent); expect(hasDirectiveScrollNotified).toBe(true); expect(hasServiceScrollNotified).toBe(true); @@ -58,7 +60,7 @@ describe('Scrollable', () => { /** Simple component that contains a large div and can be scrolled. */ @Component({ - template: `
` + template: `
` }) class ScrollingComponent { @ViewChild(Scrollable) scrollable: Scrollable; @@ -68,7 +70,7 @@ class ScrollingComponent { const TEST_COMPONENTS = [ScrollingComponent]; @NgModule({ imports: [ScrollModule], - providers: [Scroll], + providers: [ScrollDispatcher], exports: TEST_COMPONENTS, declarations: TEST_COMPONENTS, entryComponents: TEST_COMPONENTS, diff --git a/src/lib/core/scroll/scroll.ts b/src/lib/core/scroll/scroll-dispatcher.ts similarity index 79% rename from src/lib/core/scroll/scroll.ts rename to src/lib/core/scroll/scroll-dispatcher.ts index 2e9c823a66d5..ebf49fb7c5cf 100644 --- a/src/lib/core/scroll/scroll.ts +++ b/src/lib/core/scroll/scroll-dispatcher.ts @@ -3,6 +3,7 @@ import {Scrollable} from './scrollable'; import {Subject} from 'rxjs/Subject'; import {Observable} from 'rxjs/Observable'; import {Subscription} from 'rxjs/Subscription'; +import 'rxjs/add/observable/fromEvent'; /** @@ -10,20 +11,20 @@ import {Subscription} from 'rxjs/Subscription'; * Scrollable references emit a scrolled event. */ @Injectable() -export class Scroll { +export class ScrollDispatcher { /** Subject for notifying that a registered scrollable reference element has been scrolled. */ - _scrolled: Subject = new Subject(); + _scrolled: Subject = new Subject(); /** * Map of all the scrollable references that are registered with the service and their * scroll event subscriptions. */ - scrollableReferences: Map = new Map(); + scrollableReferences: WeakMap = new WeakMap(); constructor() { // By default, notify a scroll event when the document is scrolled or the window is resized. - window.document.addEventListener('scroll', this._notify.bind(this)); - window.addEventListener('resize', this._notify.bind(this)); + Observable.fromEvent(window.document, 'scroll').subscribe(() => this._notify()); + Observable.fromEvent(window, 'resize').subscribe(() => this._notify()); } /** @@ -31,7 +32,7 @@ export class Scroll { * scrollable is scrolled, the service emits the event in its scrolled observable. */ register(scrollable: Scrollable): void { - const scrollSubscription = scrollable.elementScrolled().subscribe(this._notify.bind(this)); + const scrollSubscription = scrollable.elementScrolled().subscribe(() => this._notify()); this.scrollableReferences.set(scrollable, scrollSubscription); } @@ -48,12 +49,12 @@ export class Scroll { * references (or window, document, or body) fire a scrolled event. * TODO: Add an event limiter that includes throttle with the leading and trailing events. */ - scrolled(): Observable { + scrolled(): Observable { return this._scrolled.asObservable(); } /** Sends a notification that a scroll event has been fired. */ - _notify(e: Event) { - this._scrolled.next(e); + _notify() { + this._scrolled.next(); } } diff --git a/src/lib/core/scroll/scrollable.ts b/src/lib/core/scroll/scrollable.ts index 5998a8a17874..eb0c78b13f4d 100644 --- a/src/lib/core/scroll/scrollable.ts +++ b/src/lib/core/scroll/scrollable.ts @@ -2,39 +2,33 @@ import { Directive, ElementRef, OnInit, OnDestroy, ModuleWithProviders, NgModule } from '@angular/core'; -import {Subject} from 'rxjs/Subject'; import {Observable} from 'rxjs/Observable'; -import {Scroll} from './scroll'; +import {ScrollDispatcher} from './scroll-dispatcher'; +import 'rxjs/add/observable/fromEvent'; /** - * Sends an event when the directive's element is scrolled. Registers itself with the Scroll + * Sends an event when the directive's element is scrolled. Registers itself with the ScrollDispatcher * service to include itself as part of its collection of scrolling events that it can be listened * to through the service. */ @Directive({ - selector: '[md-scrollable]' + selector: '[cdk-scrollable]' }) export class Scrollable implements OnInit, OnDestroy { - /** Subject for notifying that the element has been scrolled. */ - private _elementScrolled: Subject = new Subject(); - - constructor(private _elementRef: ElementRef, private _scroll: Scroll) {} + constructor(private _elementRef: ElementRef, private _scroll: ScrollDispatcher) {} ngOnInit() { this._scroll.register(this); - this._elementRef.nativeElement.addEventListener('scroll', (e: Event) => { - this._elementScrolled.next(e); - }); } ngOnDestroy() { this._scroll.deregister(this); } - /** Returns observable that emits an event when the scroll event is fired on the host element. */ - elementScrolled(): Observable { - return this._elementScrolled.asObservable(); + /** Returns observable that emits when the scroll event is fired on the host element. */ + elementScrolled(): Observable { + return Observable.fromEvent(this._elementRef.nativeElement, 'scroll'); } } @@ -47,7 +41,7 @@ export class ScrollModule { static forRoot(): ModuleWithProviders { return { ngModule: ScrollModule, - providers: [Scroll] + providers: [ScrollDispatcher] }; } } diff --git a/src/lib/sidenav/sidenav.ts b/src/lib/sidenav/sidenav.ts index 89852ac84dd4..d315276ae1cc 100644 --- a/src/lib/sidenav/sidenav.ts +++ b/src/lib/sidenav/sidenav.ts @@ -22,7 +22,12 @@ import {FocusTrap} from '../core/a11y/focus-trap'; import {ESCAPE} from '../core/keyboard/keycodes'; import {OverlayModule} from '../core/overlay/overlay-directives'; import {InteractivityChecker} from '../core/a11y/interactivity-checker'; +<<<<<<< 38f2302c080b3941ee7be5289c9a47a0d1a3bcef import {ScrollDispatcher} from '../core/overlay/scroll/scroll-dispatcher'; +======= +import {MdLiveAnnouncer} from '../core/a11y/live-announcer'; +import {ScrollDispatcher} from '../core/scroll/scroll-dispatcher'; +>>>>>>> review response /** Exception thrown when two MdSidenav are matching the same side. */ @@ -516,7 +521,11 @@ export class MdSidenavModule { static forRoot(): ModuleWithProviders { return { ngModule: MdSidenavModule, +<<<<<<< 38f2302c080b3941ee7be5289c9a47a0d1a3bcef providers: [InteractivityChecker, ScrollDispatcher] +======= + providers: [MdLiveAnnouncer, InteractivityChecker, ScrollDispatcher] +>>>>>>> review response }; } } From 7aa6ea45f1730047198cac463912453eca88aef2 Mon Sep 17 00:00:00 2001 From: Andrew Seguin Date: Tue, 13 Dec 2016 13:25:28 -0800 Subject: [PATCH 03/22] fix lint --- src/lib/core/scroll/scrollable.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib/core/scroll/scrollable.ts b/src/lib/core/scroll/scrollable.ts index eb0c78b13f4d..6d26ecb47c07 100644 --- a/src/lib/core/scroll/scrollable.ts +++ b/src/lib/core/scroll/scrollable.ts @@ -8,9 +8,9 @@ import 'rxjs/add/observable/fromEvent'; /** - * Sends an event when the directive's element is scrolled. Registers itself with the ScrollDispatcher - * service to include itself as part of its collection of scrolling events that it can be listened - * to through the service. + * Sends an event when the directive's element is scrolled. Registers itself with the + * ScrollDispatcher service to include itself as part of its collection of scrolling events that it + * can be listened to through the service. */ @Directive({ selector: '[cdk-scrollable]' From 33dbc4c4ccd604367b73a24586931d7041f83487 Mon Sep 17 00:00:00 2001 From: Andrew Seguin Date: Tue, 13 Dec 2016 14:17:50 -0800 Subject: [PATCH 04/22] move scroll to overlay --- src/lib/core/core.ts | 1 - src/lib/core/scroll/scroll-dispatcher.spec.ts | 78 ------------------- src/lib/core/scroll/scroll-dispatcher.ts | 60 -------------- src/lib/core/scroll/scrollable.ts | 47 ----------- src/lib/sidenav/sidenav.ts | 4 + 5 files changed, 4 insertions(+), 186 deletions(-) delete mode 100644 src/lib/core/scroll/scroll-dispatcher.spec.ts delete mode 100644 src/lib/core/scroll/scroll-dispatcher.ts delete mode 100644 src/lib/core/scroll/scrollable.ts diff --git a/src/lib/core/core.ts b/src/lib/core/core.ts index ad8a91acb70a..c8c20f9b6228 100644 --- a/src/lib/core/core.ts +++ b/src/lib/core/core.ts @@ -7,7 +7,6 @@ import {PortalModule} from './portal/portal-directives'; import {OverlayModule} from './overlay/overlay-directives'; import {A11yModule, A11Y_PROVIDERS} from './a11y/index'; import {OVERLAY_PROVIDERS} from './overlay/overlay'; -import {ScrollDispatcher} from './scroll/scroll-dispatcher'; // RTL diff --git a/src/lib/core/scroll/scroll-dispatcher.spec.ts b/src/lib/core/scroll/scroll-dispatcher.spec.ts deleted file mode 100644 index 2d476e22deaa..000000000000 --- a/src/lib/core/scroll/scroll-dispatcher.spec.ts +++ /dev/null @@ -1,78 +0,0 @@ -import {inject, TestBed, async, ComponentFixture} from '@angular/core/testing'; -import {NgModule, Component, ViewChild, ElementRef} from '@angular/core'; -import {ScrollDispatcher} from './scroll-dispatcher'; -import {ScrollModule, Scrollable} from './scrollable'; - -describe('Scroll Dispatcher', () => { - let scroll: ScrollDispatcher; - let fixture: ComponentFixture; - - beforeEach(async(() => { - TestBed.configureTestingModule({ - imports: [ScrollModule.forRoot(), ScrollTestModule], - }); - - TestBed.compileComponents(); - })); - - beforeEach(inject([ScrollDispatcher], (s: ScrollDispatcher) => { - scroll = s; - - fixture = TestBed.createComponent(ScrollingComponent); - fixture.detectChanges(); - })); - - it('should be registered with the scrollable directive with the scroll service', () => { - const componentScrollable = fixture.componentInstance.scrollable; - expect(scroll.scrollableReferences.has(componentScrollable)).toBe(true); - }); - - it('should have the scrollable directive deregistered when the component is destroyed', () => { - const componentScrollable = fixture.componentInstance.scrollable; - expect(scroll.scrollableReferences.has(componentScrollable)).toBe(true); - - fixture.destroy(); - expect(scroll.scrollableReferences.has(componentScrollable)).toBe(false); - }); - - it('should notify through the directive and service that a scroll event occurred', () => { - let hasDirectiveScrollNotified = false; - // Listen for notifications from scroll directive - let scrollable = fixture.componentInstance.scrollable; - scrollable.elementScrolled().subscribe(() => { hasDirectiveScrollNotified = true; }); - - // Listen for notifications from scroll service - let hasServiceScrollNotified = false; - scroll.scrolled().subscribe(() => { hasServiceScrollNotified = true; }); - - // Emit a scroll event from the scrolling element in our component. - // This event should be picked up by the scrollable directive and notify. - // The notification should be picked up by the service. - const scrollEvent = document.createEvent('UIEvents'); - scrollEvent.initUIEvent('scroll', true, true, window, 0); - fixture.componentInstance.scrollingElement.nativeElement.dispatchEvent(scrollEvent); - - expect(hasDirectiveScrollNotified).toBe(true); - expect(hasServiceScrollNotified).toBe(true); - }); -}); - - -/** Simple component that contains a large div and can be scrolled. */ -@Component({ - template: `
` -}) -class ScrollingComponent { - @ViewChild(Scrollable) scrollable: Scrollable; - @ViewChild('scrollingElement') scrollingElement: ElementRef; -} - -const TEST_COMPONENTS = [ScrollingComponent]; -@NgModule({ - imports: [ScrollModule], - providers: [ScrollDispatcher], - exports: TEST_COMPONENTS, - declarations: TEST_COMPONENTS, - entryComponents: TEST_COMPONENTS, -}) -class ScrollTestModule { } diff --git a/src/lib/core/scroll/scroll-dispatcher.ts b/src/lib/core/scroll/scroll-dispatcher.ts deleted file mode 100644 index ebf49fb7c5cf..000000000000 --- a/src/lib/core/scroll/scroll-dispatcher.ts +++ /dev/null @@ -1,60 +0,0 @@ -import {Injectable} from '@angular/core'; -import {Scrollable} from './scrollable'; -import {Subject} from 'rxjs/Subject'; -import {Observable} from 'rxjs/Observable'; -import {Subscription} from 'rxjs/Subscription'; -import 'rxjs/add/observable/fromEvent'; - - -/** - * Service contained all registered Scrollable references and emits an event when any one of the - * Scrollable references emit a scrolled event. - */ -@Injectable() -export class ScrollDispatcher { - /** Subject for notifying that a registered scrollable reference element has been scrolled. */ - _scrolled: Subject = new Subject(); - - /** - * Map of all the scrollable references that are registered with the service and their - * scroll event subscriptions. - */ - scrollableReferences: WeakMap = new WeakMap(); - - constructor() { - // By default, notify a scroll event when the document is scrolled or the window is resized. - Observable.fromEvent(window.document, 'scroll').subscribe(() => this._notify()); - Observable.fromEvent(window, 'resize').subscribe(() => this._notify()); - } - - /** - * Registers a Scrollable with the service and listens for its scrolled events. When the - * scrollable is scrolled, the service emits the event in its scrolled observable. - */ - register(scrollable: Scrollable): void { - const scrollSubscription = scrollable.elementScrolled().subscribe(() => this._notify()); - this.scrollableReferences.set(scrollable, scrollSubscription); - } - - /** - * Deregisters a Scrollable reference and unsubscribes from its scroll event observable. - */ - deregister(scrollable: Scrollable): void { - this.scrollableReferences.get(scrollable).unsubscribe(); - this.scrollableReferences.delete(scrollable); - } - - /** - * Returns an observable that emits an event whenever any of the registered Scrollable - * references (or window, document, or body) fire a scrolled event. - * TODO: Add an event limiter that includes throttle with the leading and trailing events. - */ - scrolled(): Observable { - return this._scrolled.asObservable(); - } - - /** Sends a notification that a scroll event has been fired. */ - _notify() { - this._scrolled.next(); - } -} diff --git a/src/lib/core/scroll/scrollable.ts b/src/lib/core/scroll/scrollable.ts deleted file mode 100644 index 6d26ecb47c07..000000000000 --- a/src/lib/core/scroll/scrollable.ts +++ /dev/null @@ -1,47 +0,0 @@ -import { - Directive, ElementRef, OnInit, OnDestroy, ModuleWithProviders, - NgModule -} from '@angular/core'; -import {Observable} from 'rxjs/Observable'; -import {ScrollDispatcher} from './scroll-dispatcher'; -import 'rxjs/add/observable/fromEvent'; - - -/** - * Sends an event when the directive's element is scrolled. Registers itself with the - * ScrollDispatcher service to include itself as part of its collection of scrolling events that it - * can be listened to through the service. - */ -@Directive({ - selector: '[cdk-scrollable]' -}) -export class Scrollable implements OnInit, OnDestroy { - constructor(private _elementRef: ElementRef, private _scroll: ScrollDispatcher) {} - - ngOnInit() { - this._scroll.register(this); - } - - ngOnDestroy() { - this._scroll.deregister(this); - } - - /** Returns observable that emits when the scroll event is fired on the host element. */ - elementScrolled(): Observable { - return Observable.fromEvent(this._elementRef.nativeElement, 'scroll'); - } -} - - -@NgModule({ - exports: [Scrollable], - declarations: [Scrollable], -}) -export class ScrollModule { - static forRoot(): ModuleWithProviders { - return { - ngModule: ScrollModule, - providers: [ScrollDispatcher] - }; - } -} diff --git a/src/lib/sidenav/sidenav.ts b/src/lib/sidenav/sidenav.ts index d315276ae1cc..c19286d242a0 100644 --- a/src/lib/sidenav/sidenav.ts +++ b/src/lib/sidenav/sidenav.ts @@ -26,8 +26,12 @@ import {InteractivityChecker} from '../core/a11y/interactivity-checker'; import {ScrollDispatcher} from '../core/overlay/scroll/scroll-dispatcher'; ======= import {MdLiveAnnouncer} from '../core/a11y/live-announcer'; +<<<<<<< 7f62f0a674630555a66a123367e701089f68e338 import {ScrollDispatcher} from '../core/scroll/scroll-dispatcher'; >>>>>>> review response +======= +import {ScrollDispatcher} from '../core/overlay/scroll/scroll-dispatcher'; +>>>>>>> move scroll to overlay /** Exception thrown when two MdSidenav are matching the same side. */ From 8be6a50aa4d3fdeb34e0685ffcf57eb61526a12e Mon Sep 17 00:00:00 2001 From: Andrew Seguin Date: Thu, 15 Dec 2016 15:28:30 -0800 Subject: [PATCH 05/22] feat(scroll): hide tooltip when clipped by scrollable container --- src/demo-app/tooltip/tooltip-demo.html | 29 ++++++++++++++-- src/demo-app/tooltip/tooltip-demo.scss | 11 +++++++ .../position/connected-position-strategy.ts | 33 ++++++++++++++++++- .../overlay/position/connected-position.ts | 3 +- .../core/overlay/scroll/scroll-dispatcher.ts | 31 +++++++++++++++-- src/lib/core/overlay/scroll/scrollable.ts | 15 +++++++-- src/lib/tooltip/tooltip.ts | 11 ++++++- 7 files changed, 124 insertions(+), 9 deletions(-) diff --git a/src/demo-app/tooltip/tooltip-demo.html b/src/demo-app/tooltip/tooltip-demo.html index 13031ea41301..f56767ee6aaa 100644 --- a/src/demo-app/tooltip/tooltip-demo.html +++ b/src/demo-app/tooltip/tooltip-demo.html @@ -1,4 +1,4 @@ -
+

Tooltip Demo

@@ -40,7 +40,7 @@

Tooltip Demo

Mouse over to -
+ +
+ Scroll to see tooltip button. + +
+
+ Scroll to see tooltip button. + +
\ No newline at end of file diff --git a/src/demo-app/tooltip/tooltip-demo.scss b/src/demo-app/tooltip/tooltip-demo.scss index 39c9602a9e72..ea5bf54f70ef 100644 --- a/src/demo-app/tooltip/tooltip-demo.scss +++ b/src/demo-app/tooltip/tooltip-demo.scss @@ -6,3 +6,14 @@ display: block; } } + +.scrolling-container { + margin: 300px; + height: 300px; + width: 300px; + overflow: auto; + + button { + margin: 500px; + } +} diff --git a/src/lib/core/overlay/position/connected-position-strategy.ts b/src/lib/core/overlay/position/connected-position-strategy.ts index d6bb569b828c..42fb8496e299 100644 --- a/src/lib/core/overlay/position/connected-position-strategy.ts +++ b/src/lib/core/overlay/position/connected-position-strategy.ts @@ -9,6 +9,14 @@ import { } from './connected-position'; import {Subject} from 'rxjs/Subject'; import {Observable} from 'rxjs/Observable'; +import {Scrollable} from '../scroll/scrollable'; + +export type ElementBoundingPositions = { + top: number; + right: number; + bottom: number; + left: number; +} /** * A strategy for positioning overlays. Using this strategy, an overlay is given an @@ -26,6 +34,9 @@ export class ConnectedPositionStrategy implements PositionStrategy { /** The offset in pixels for the overlay connection point on the y-axis */ private _offsetY: number = 0; + /** The Scrollable containers that may cause the overlay's connectedTo element to be clipped */ + private scrollables: Scrollable[]; + /** Whether the we're dealing with an RTL context */ get _isRtl() { return this._dir === 'rtl'; @@ -95,7 +106,8 @@ export class ConnectedPositionStrategy implements PositionStrategy { // If the overlay in the calculated position fits on-screen, put it there and we're done. if (overlayPoint.fitsInViewport) { this._setElementPosition(element, overlayPoint); - this._onPositionChange.next(new ConnectedOverlayPositionChange(pos)); + const isClipped = this.isConnectedToElementClipped(); + this._onPositionChange.next(new ConnectedOverlayPositionChange(pos, isClipped)); return Promise.resolve(null); } else if (!fallbackPoint || fallbackPoint.visibleArea < overlayPoint.visibleArea) { fallbackPoint = overlayPoint; @@ -110,6 +122,15 @@ export class ConnectedPositionStrategy implements PositionStrategy { } /** + * Sets the list of scrolling or resizing containers that host the connectedTo element so that + * on reposition we can evaluate if it has been clipped when repositioned. + */ + withScrollableContainers(scrollables: Scrollable[]) { + this.scrollables = scrollables; + } + + /** + <<<<<<< b07c918b15021de7b9b5af2928d994203174cbe7 * Adds a new preferred fallback position. * @param originPos * @param overlayPos @@ -251,6 +272,16 @@ export class ConnectedPositionStrategy implements PositionStrategy { element.style.top = overlayPoint.y + 'px'; } + private _getElementBounds(element: ElementRef): ElementBoundingPositions { + const boundingClientRect = element.nativeElement.getBoundingClientRect(); + return { + top: boundingClientRect.top, + right: boundingClientRect.left + boundingClientRect.width, + bottom: boundingClientRect.top + boundingClientRect.height, + left: boundingClientRect.left + }; + } + /** * Subtracts the amount that an element is overflowing on an axis from it's length. */ diff --git a/src/lib/core/overlay/position/connected-position.ts b/src/lib/core/overlay/position/connected-position.ts index b2e5b9b4a000..4d4e0b3e4cf5 100644 --- a/src/lib/core/overlay/position/connected-position.ts +++ b/src/lib/core/overlay/position/connected-position.ts @@ -34,5 +34,6 @@ export class ConnectionPositionPair { /** The change event emitted by the strategy when a fallback position is used. */ export class ConnectedOverlayPositionChange { - constructor(public connectionPair: ConnectionPositionPair) {} + constructor(public connectionPair: ConnectionPositionPair, + public isConnectedToElementClipped: boolean) {} } diff --git a/src/lib/core/overlay/scroll/scroll-dispatcher.ts b/src/lib/core/overlay/scroll/scroll-dispatcher.ts index 199cee3bb12e..e501e6e66ac0 100644 --- a/src/lib/core/overlay/scroll/scroll-dispatcher.ts +++ b/src/lib/core/overlay/scroll/scroll-dispatcher.ts @@ -1,9 +1,10 @@ -import {Injectable} from '@angular/core'; +import {Injectable, ElementRef} from '@angular/core'; import {Scrollable} from './scrollable'; import {Subject} from 'rxjs/Subject'; import {Observable} from 'rxjs/Observable'; import {Subscription} from 'rxjs/Subscription'; import 'rxjs/add/observable/fromEvent'; +import 'rxjs/add/observable/combineLatest'; /** @@ -19,7 +20,7 @@ export class ScrollDispatcher { * Map of all the scrollable references that are registered with the service and their * scroll event subscriptions. */ - scrollableReferences: WeakMap = new WeakMap(); + scrollableReferences: Map = new Map(); constructor() { // By default, notify a scroll event when the document is scrolled or the window is resized. @@ -57,8 +58,34 @@ export class ScrollDispatcher { return this._scrolled.asObservable(); } + /** Returns all registered Scrollables that contain the provided element. */ + getScrollableContainers(elementRef: ElementRef): Scrollable[] { + const scrollingContainers: Scrollable[] = []; + + this.scrollableReferences.forEach((subscription: Subscription, scrollable: Scrollable) => { + if (this.scrollableContainsElement(scrollable, elementRef)) { + scrollingContainers.push(scrollable); + } + }); + + return scrollingContainers; + } + + /** Returns true if the element is contained within the provided Scrollable. */ + scrollableContainsElement(scrollable: Scrollable, elementRef: ElementRef): boolean { + let element = elementRef.nativeElement; + let scrollableElement = scrollable.getElementRef().nativeElement; + + // Traverse through the element parents until we reach null, checking if any of the elements + // are the scrollable's element. + do { + if (element == scrollableElement) { return true; } + } while (element = element.parentElement); + } + /** Sends a notification that a scroll event has been fired. */ _notify() { this._scrolled.next(); } } + diff --git a/src/lib/core/overlay/scroll/scrollable.ts b/src/lib/core/overlay/scroll/scrollable.ts index cf13dec05002..0e187b94c327 100644 --- a/src/lib/core/overlay/scroll/scrollable.ts +++ b/src/lib/core/overlay/scroll/scrollable.ts @@ -1,5 +1,5 @@ import { - Directive, ElementRef, OnInit, OnDestroy + Directive, ElementRef, OnInit, OnDestroy, Optional, SkipSelf } from '@angular/core'; import {Observable} from 'rxjs/Observable'; import {ScrollDispatcher} from './scroll-dispatcher'; @@ -15,7 +15,9 @@ import 'rxjs/add/observable/fromEvent'; selector: '[cdk-scrollable]' }) export class Scrollable implements OnInit, OnDestroy { - constructor(private _elementRef: ElementRef, private _scroll: ScrollDispatcher) {} + constructor(private _elementRef: ElementRef, + @SkipSelf() @Optional() private parentScrollable: Scrollable, + private _scroll: ScrollDispatcher) {} ngOnInit() { this._scroll.register(this); @@ -31,4 +33,13 @@ export class Scrollable implements OnInit, OnDestroy { elementScrolled(): Observable { return Observable.fromEvent(this._elementRef.nativeElement, 'scroll'); } + + getElementRef(): ElementRef { + return this._elementRef; + } + + /** Returns this scrollable along with all the scrollables that this is contained within. */ + getAllScrollables(): Scrollable[] { + return this.parentScrollable ? this.parentScrollable.getAllScrollables().concat(this) : [this]; + } } diff --git a/src/lib/tooltip/tooltip.ts b/src/lib/tooltip/tooltip.ts index 264b7f7851ed..f1a61c175563 100644 --- a/src/lib/tooltip/tooltip.ts +++ b/src/lib/tooltip/tooltip.ts @@ -109,7 +109,7 @@ export class MdTooltip implements OnInit, OnDestroy { private _elementRef: ElementRef, private _viewContainerRef: ViewContainerRef, private _ngZone: NgZone, - @Optional() private _dir: Dir) {} + @Optional() private _dir: Dir) { } ngOnInit() { // When a scroll on the page occurs, update the position in case this tooltip needs @@ -178,7 +178,16 @@ export class MdTooltip implements OnInit, OnDestroy { private _createOverlay(): void { let origin = this._getOrigin(); let position = this._getOverlayPosition(); + + // Create connected strategy that listens for scroll events to reposition. After position + // changes occur, check if the scrolling trigger has been clipped and close the tooltip. let strategy = this._overlay.position().connectedTo(this._elementRef, origin, position); + strategy.withScrollableContainers( + this._scrollDispatcher.getScrollableContainers(this._elementRef)); + strategy.onPositionChange.subscribe(change => { + if (change.isConnectedToElementClipped) { this.hide(0); } + }); + let config = new OverlayState(); config.positionStrategy = strategy; From e86755d608ea3e0c3c9ba150ebc578e6b2cb9e39 Mon Sep 17 00:00:00 2001 From: Andrew Seguin Date: Fri, 16 Dec 2016 12:01:50 -0800 Subject: [PATCH 06/22] use overlay for clipping instead of trigger --- .../position/connected-position-strategy.ts | 23 +++++++++++++++---- .../overlay/position/connected-position.ts | 3 +-- .../core/overlay/scroll/scroll-dispatcher.ts | 2 +- src/lib/tooltip/tooltip.ts | 9 +++----- tools/gulp/tasks/components.ts | 1 + 5 files changed, 25 insertions(+), 13 deletions(-) diff --git a/src/lib/core/overlay/position/connected-position-strategy.ts b/src/lib/core/overlay/position/connected-position-strategy.ts index 42fb8496e299..8ea4ea44e27f 100644 --- a/src/lib/core/overlay/position/connected-position-strategy.ts +++ b/src/lib/core/overlay/position/connected-position-strategy.ts @@ -35,7 +35,7 @@ export class ConnectedPositionStrategy implements PositionStrategy { private _offsetY: number = 0; /** The Scrollable containers that may cause the overlay's connectedTo element to be clipped */ - private scrollables: Scrollable[]; + private scrollables: Scrollable[] = []; /** Whether the we're dealing with an RTL context */ get _isRtl() { @@ -106,7 +106,7 @@ export class ConnectedPositionStrategy implements PositionStrategy { // If the overlay in the calculated position fits on-screen, put it there and we're done. if (overlayPoint.fitsInViewport) { this._setElementPosition(element, overlayPoint); - const isClipped = this.isConnectedToElementClipped(); + const isClipped = this.isOverlayElementClipped(element); this._onPositionChange.next(new ConnectedOverlayPositionChange(pos, isClipped)); return Promise.resolve(null); } else if (!fallbackPoint || fallbackPoint.visibleArea < overlayPoint.visibleArea) { @@ -262,6 +262,21 @@ export class ConnectedPositionStrategy implements PositionStrategy { return {x, y, fitsInViewport, visibleArea}; } + /** Whether the overlay element is clipped out of view of one of the scrollable containers. */ + private isOverlayElementClipped(element: HTMLElement): boolean { + const elementBounds = this._getElementBounds(element); + return this.scrollables.some((scrollable: Scrollable) => { + const scrollingContainerBounds = this._getElementBounds(scrollable.getElementRef().nativeElement); + + const clippedAbove = elementBounds.top < scrollingContainerBounds.top; + const clippedBelow = elementBounds.bottom > scrollingContainerBounds.bottom; + const clippedLeft = elementBounds.left < scrollingContainerBounds.left; + const clippedRight = elementBounds.right > scrollingContainerBounds.right; + + return clippedAbove || clippedBelow || clippedLeft || clippedRight; + }); + } + /** * Physically positions the overlay element to the given coordinate. * @param element @@ -272,8 +287,8 @@ export class ConnectedPositionStrategy implements PositionStrategy { element.style.top = overlayPoint.y + 'px'; } - private _getElementBounds(element: ElementRef): ElementBoundingPositions { - const boundingClientRect = element.nativeElement.getBoundingClientRect(); + private _getElementBounds(element: HTMLElement): ElementBoundingPositions { + const boundingClientRect = element.getBoundingClientRect(); return { top: boundingClientRect.top, right: boundingClientRect.left + boundingClientRect.width, diff --git a/src/lib/core/overlay/position/connected-position.ts b/src/lib/core/overlay/position/connected-position.ts index 4d4e0b3e4cf5..f03df353cc74 100644 --- a/src/lib/core/overlay/position/connected-position.ts +++ b/src/lib/core/overlay/position/connected-position.ts @@ -34,6 +34,5 @@ export class ConnectionPositionPair { /** The change event emitted by the strategy when a fallback position is used. */ export class ConnectedOverlayPositionChange { - constructor(public connectionPair: ConnectionPositionPair, - public isConnectedToElementClipped: boolean) {} + constructor(public connectionPair: ConnectionPositionPair, public isClipped: boolean) {} } diff --git a/src/lib/core/overlay/scroll/scroll-dispatcher.ts b/src/lib/core/overlay/scroll/scroll-dispatcher.ts index e501e6e66ac0..b9b339de2559 100644 --- a/src/lib/core/overlay/scroll/scroll-dispatcher.ts +++ b/src/lib/core/overlay/scroll/scroll-dispatcher.ts @@ -59,7 +59,7 @@ export class ScrollDispatcher { } /** Returns all registered Scrollables that contain the provided element. */ - getScrollableContainers(elementRef: ElementRef): Scrollable[] { + getScrollContainers(elementRef: ElementRef): Scrollable[] { const scrollingContainers: Scrollable[] = []; this.scrollableReferences.forEach((subscription: Subscription, scrollable: Scrollable) => { diff --git a/src/lib/tooltip/tooltip.ts b/src/lib/tooltip/tooltip.ts index f1a61c175563..35e87b07983d 100644 --- a/src/lib/tooltip/tooltip.ts +++ b/src/lib/tooltip/tooltip.ts @@ -180,13 +180,10 @@ export class MdTooltip implements OnInit, OnDestroy { let position = this._getOverlayPosition(); // Create connected strategy that listens for scroll events to reposition. After position - // changes occur, check if the scrolling trigger has been clipped and close the tooltip. + // changes occur and the overlay is clipped then close the tooltip. let strategy = this._overlay.position().connectedTo(this._elementRef, origin, position); - strategy.withScrollableContainers( - this._scrollDispatcher.getScrollableContainers(this._elementRef)); - strategy.onPositionChange.subscribe(change => { - if (change.isConnectedToElementClipped) { this.hide(0); } - }); + strategy.withScrollableContainers(this._scrollDispatcher.getScrollContainers(this._elementRef)); + strategy.onPositionChange.subscribe(change => { if (change.isClipped) { this.hide(0); } }); let config = new OverlayState(); config.positionStrategy = strategy; diff --git a/tools/gulp/tasks/components.ts b/tools/gulp/tasks/components.ts index 66ea3e82ca1a..27c3b071f7fa 100644 --- a/tools/gulp/tasks/components.ts +++ b/tools/gulp/tasks/components.ts @@ -81,6 +81,7 @@ task(':build:components:rollup', () => { 'rxjs/add/operator/finally': 'Rx.Observable.prototype', 'rxjs/add/operator/catch': 'Rx.Observable.prototype', 'rxjs/add/operator/first': 'Rx.Observable.prototype', + 'rxjs/add/operator/combineLatest': 'Rx.Observable.prototype', 'rxjs/Observable': 'Rx' }; From e0612110ff3971effa984228acd2e2688f232bea Mon Sep 17 00:00:00 2001 From: Andrew Seguin Date: Fri, 16 Dec 2016 14:22:05 -0800 Subject: [PATCH 07/22] add tooltip test --- .../position/connected-position-strategy.ts | 72 ++++++++++++++----- .../overlay/position/connected-position.ts | 10 ++- src/lib/tooltip/tooltip.spec.ts | 67 ++++++++++++++++- src/lib/tooltip/tooltip.ts | 12 +++- 4 files changed, 136 insertions(+), 25 deletions(-) diff --git a/src/lib/core/overlay/position/connected-position-strategy.ts b/src/lib/core/overlay/position/connected-position-strategy.ts index 8ea4ea44e27f..0abd5ccd836c 100644 --- a/src/lib/core/overlay/position/connected-position-strategy.ts +++ b/src/lib/core/overlay/position/connected-position-strategy.ts @@ -2,10 +2,10 @@ import {PositionStrategy} from './position-strategy'; import {ElementRef} from '@angular/core'; import {ViewportRuler} from './viewport-ruler'; import { - ConnectionPositionPair, - OriginConnectionPosition, - OverlayConnectionPosition, - ConnectedOverlayPositionChange + ConnectionPositionPair, + OriginConnectionPosition, + OverlayConnectionPosition, + ConnectedOverlayPositionChange, ScrollableViewProperties } from './connected-position'; import {Subject} from 'rxjs/Subject'; import {Observable} from 'rxjs/Observable'; @@ -35,7 +35,7 @@ export class ConnectedPositionStrategy implements PositionStrategy { private _offsetY: number = 0; /** The Scrollable containers that may cause the overlay's connectedTo element to be clipped */ - private scrollables: Scrollable[] = []; + private scrollables: Scrollable[]; /** Whether the we're dealing with an RTL context */ get _isRtl() { @@ -48,7 +48,7 @@ export class ConnectedPositionStrategy implements PositionStrategy { /** The origin element against which the overlay will be positioned. */ private _origin: HTMLElement; - private _onPositionChange: + _onPositionChange: Subject = new Subject(); /** Emits an event when the connection point changes. */ @@ -106,8 +106,12 @@ export class ConnectedPositionStrategy implements PositionStrategy { // If the overlay in the calculated position fits on-screen, put it there and we're done. if (overlayPoint.fitsInViewport) { this._setElementPosition(element, overlayPoint); - const isClipped = this.isOverlayElementClipped(element); - this._onPositionChange.next(new ConnectedOverlayPositionChange(pos, isClipped)); + + // Notify that the position has been changed along with its change properties. + const scrollableViewProperties = this.getScrollableViewProperties(element); + const positionChange = new ConnectedOverlayPositionChange(pos, scrollableViewProperties); + this._onPositionChange.next(positionChange); + return Promise.resolve(null); } else if (!fallbackPoint || fallbackPoint.visibleArea < overlayPoint.visibleArea) { fallbackPoint = overlayPoint; @@ -262,16 +266,48 @@ export class ConnectedPositionStrategy implements PositionStrategy { return {x, y, fitsInViewport, visibleArea}; } - /** Whether the overlay element is clipped out of view of one of the scrollable containers. */ - private isOverlayElementClipped(element: HTMLElement): boolean { - const elementBounds = this._getElementBounds(element); - return this.scrollables.some((scrollable: Scrollable) => { - const scrollingContainerBounds = this._getElementBounds(scrollable.getElementRef().nativeElement); + /** + * Gets the view properties of the trigger and overlay, including whether they are clipped + * or completely outside the view of any of the strategy's scrollables. + */ + private getScrollableViewProperties(overlay: HTMLElement): ScrollableViewProperties { + const triggerBounds = this._getElementBounds(this._connectedTo.nativeElement); + const overlayBounds = this._getElementBounds(overlay); + const scrollContainerBounds = this.scrollables.map((scrollable: Scrollable) => { + return this._getElementBounds(scrollable.getElementRef().nativeElement); + }); - const clippedAbove = elementBounds.top < scrollingContainerBounds.top; - const clippedBelow = elementBounds.bottom > scrollingContainerBounds.bottom; - const clippedLeft = elementBounds.left < scrollingContainerBounds.left; - const clippedRight = elementBounds.right > scrollingContainerBounds.right; + return { + isTriggerClipped: this.isElementClipped(triggerBounds, scrollContainerBounds), + isTriggerOutsideView: this.isElementOutsideView(triggerBounds, scrollContainerBounds), + isOverlayClipped: this.isElementClipped(overlayBounds, scrollContainerBounds), + isOverlayOutsideView: this.isElementOutsideView(overlayBounds, scrollContainerBounds), + }; + } + + /** Whether the element is completely out of the view of any of the containers. */ + private isElementOutsideView( + elementBounds: ElementBoundingPositions, + containersBounds: ElementBoundingPositions[]): boolean { + return containersBounds.some((containerBounds: ElementBoundingPositions) => { + const outsideAbove = elementBounds.bottom < containerBounds.top; + const outsideBelow = elementBounds.top > containerBounds.bottom; + const outsideLeft = elementBounds.right < containerBounds.left; + const outsideRight = elementBounds.left > containerBounds.right; + + return outsideAbove || outsideBelow || outsideLeft || outsideRight; + }); + } + + /** Whether the element is clipped by any of the containers. */ + private isElementClipped( + elementBounds: ElementBoundingPositions, + containersBounds: ElementBoundingPositions[]): boolean { + return containersBounds.some((containerBounds: ElementBoundingPositions) => { + const clippedAbove = elementBounds.top < containerBounds.top; + const clippedBelow = elementBounds.bottom > containerBounds.bottom; + const clippedLeft = elementBounds.left < containerBounds.left; + const clippedRight = elementBounds.right > containerBounds.right; return clippedAbove || clippedBelow || clippedLeft || clippedRight; }); @@ -311,7 +347,7 @@ export class ConnectedPositionStrategy implements PositionStrategy { interface Point { x: number; y: number; -}; +} /** * Expands the simple (x, y) coordinate by adding info about whether the diff --git a/src/lib/core/overlay/position/connected-position.ts b/src/lib/core/overlay/position/connected-position.ts index f03df353cc74..0778c45d53cc 100644 --- a/src/lib/core/overlay/position/connected-position.ts +++ b/src/lib/core/overlay/position/connected-position.ts @@ -32,7 +32,15 @@ export class ConnectionPositionPair { } } +export class ScrollableViewProperties { + isTriggerClipped: boolean; + isTriggerOutsideView: boolean; + isOverlayClipped: boolean; + isOverlayOutsideView: boolean; +} + /** The change event emitted by the strategy when a fallback position is used. */ export class ConnectedOverlayPositionChange { - constructor(public connectionPair: ConnectionPositionPair, public isClipped: boolean) {} + constructor(public connectionPair: ConnectionPositionPair, + public scrollableViewProperties: ScrollableViewProperties) {} } diff --git a/src/lib/tooltip/tooltip.spec.ts b/src/lib/tooltip/tooltip.spec.ts index 83c9ac5a1657..b48092a644b9 100644 --- a/src/lib/tooltip/tooltip.spec.ts +++ b/src/lib/tooltip/tooltip.spec.ts @@ -6,11 +6,13 @@ import { fakeAsync, flushMicrotasks } from '@angular/core/testing'; -import {Component, DebugElement, AnimationTransitionEvent} from '@angular/core'; +import {Component, DebugElement, AnimationTransitionEvent, ViewChild} from '@angular/core'; import {By} from '@angular/platform-browser'; import {TooltipPosition, MdTooltip, MdTooltipModule} from './tooltip'; import {OverlayContainer} from '../core'; import {Dir, LayoutDirection} from '../core/rtl/dir'; +import {OverlayModule} from '../core/overlay/overlay-directives'; +import {Scrollable} from '../core/overlay/scroll/scrollable'; const initialTooltipMessage = 'initial tooltip message'; @@ -20,8 +22,8 @@ describe('MdTooltip', () => { beforeEach(async(() => { TestBed.configureTestingModule({ - imports: [MdTooltipModule.forRoot()], - declarations: [BasicTooltipDemo], + imports: [MdTooltipModule.forRoot(), OverlayModule], + declarations: [BasicTooltipDemo, ScrollableTooltipDemo], providers: [ {provide: OverlayContainer, useFactory: () => { overlayContainerElement = document.createElement('div'); @@ -296,6 +298,34 @@ describe('MdTooltip', () => { }).toThrowError('Tooltip position "everywhere" is invalid.'); }); }); + + + fdescribe('scrollable usage', () => { + let fixture: ComponentFixture; + let buttonDebugElement: DebugElement; + let buttonElement: HTMLButtonElement; + let tooltipDirective: MdTooltip; + + beforeEach(() => { + fixture = TestBed.createComponent(ScrollableTooltipDemo); + fixture.detectChanges(); + buttonDebugElement = fixture.debugElement.query(By.css('button')); + buttonElement = buttonDebugElement.nativeElement; + tooltipDirective = buttonDebugElement.injector.get(MdTooltip); + }); + + it('should hide tooltip if clipped after changing positions', fakeAsync(() => { + expect(tooltipDirective._tooltipInstance).toBeUndefined(); + + tooltipDirective.show(); + tick(0); // Tick for the show delay (default is 0) + expect(tooltipDirective._isTooltipVisible()).toBe(true); + + fixture.componentInstance.scrollDown(); + tick(); + expect(tooltipDirective._isTooltipVisible()).toBe(false); + })); + }); }); @Component({ @@ -312,3 +342,34 @@ class BasicTooltipDemo { message: string = initialTooltipMessage; showButton: boolean = true; } + +@Component({ + selector: 'app', + template: ` +
+ +
` +}) +class ScrollableTooltipDemo { + position: string = 'below'; + message: string = initialTooltipMessage; + showButton: boolean = true; + + @ViewChild(Scrollable) scrollingContainer: Scrollable; + + scrollDown() { + const scrollingContainerEl = this.scrollingContainer.getElementRef().nativeElement; + scrollingContainerEl.scrollTop = 50; + + // Emit a scroll event from the scrolling element in our component. + // This event should be picked up by the scrollable directive and notify. + // The notification should be picked up by the service. + const scrollEvent = document.createEvent('UIEvents'); + scrollEvent.initUIEvent('scroll', true, true, window, 0); + scrollingContainerEl.dispatchEvent(scrollEvent); + } +} diff --git a/src/lib/tooltip/tooltip.ts b/src/lib/tooltip/tooltip.ts index 35e87b07983d..65000b686053 100644 --- a/src/lib/tooltip/tooltip.ts +++ b/src/lib/tooltip/tooltip.ts @@ -33,6 +33,7 @@ import {Subject} from 'rxjs/Subject'; import {Dir} from '../core/rtl/dir'; import {ScrollDispatcher} from '../core/overlay/scroll/scroll-dispatcher'; import {OVERLAY_PROVIDERS} from '../core/overlay/overlay'; +import {ConnectedOverlayPositionChange} from '../core/overlay/position/connected-position'; import 'rxjs/add/operator/first'; export type TooltipPosition = 'left' | 'right' | 'above' | 'below' | 'before' | 'after'; @@ -179,11 +180,16 @@ export class MdTooltip implements OnInit, OnDestroy { let origin = this._getOrigin(); let position = this._getOverlayPosition(); - // Create connected strategy that listens for scroll events to reposition. After position - // changes occur and the overlay is clipped then close the tooltip. + // Create connected position strategy that listens for scroll events to reposition. + // After position changes occur and the overlay is clipped by a parent scrollable then + // close the tooltip. let strategy = this._overlay.position().connectedTo(this._elementRef, origin, position); strategy.withScrollableContainers(this._scrollDispatcher.getScrollContainers(this._elementRef)); - strategy.onPositionChange.subscribe(change => { if (change.isClipped) { this.hide(0); } }); + strategy.onPositionChange.subscribe((change: ConnectedOverlayPositionChange) => { + if (change.scrollableViewProperties.isOverlayClipped) { + this.hide(0); + } + }); let config = new OverlayState(); config.positionStrategy = strategy; From 694d3d64628450eeeca21e0409fb2bd0d8b1bdd5 Mon Sep 17 00:00:00 2001 From: Andrew Seguin Date: Fri, 16 Dec 2016 14:52:42 -0800 Subject: [PATCH 08/22] add test for scroll dispatcher --- .../position/connected-position-strategy.ts | 5 +- .../overlay/position/connected-position.ts | 3 +- .../overlay/scroll/scroll-dispatcher.spec.ts | 116 ++++++++++++------ src/lib/tooltip/tooltip.spec.ts | 2 +- 4 files changed, 85 insertions(+), 41 deletions(-) diff --git a/src/lib/core/overlay/position/connected-position-strategy.ts b/src/lib/core/overlay/position/connected-position-strategy.ts index 0abd5ccd836c..11f8a3765bb7 100644 --- a/src/lib/core/overlay/position/connected-position-strategy.ts +++ b/src/lib/core/overlay/position/connected-position-strategy.ts @@ -34,8 +34,8 @@ export class ConnectedPositionStrategy implements PositionStrategy { /** The offset in pixels for the overlay connection point on the y-axis */ private _offsetY: number = 0; - /** The Scrollable containers that may cause the overlay's connectedTo element to be clipped */ - private scrollables: Scrollable[]; + /** The Scrollable containers used to check scrollable view properties on position change. */ + private scrollables: Scrollable[] = []; /** Whether the we're dealing with an RTL context */ get _isRtl() { @@ -323,6 +323,7 @@ export class ConnectedPositionStrategy implements PositionStrategy { element.style.top = overlayPoint.y + 'px'; } + /** Returns the bounding positions of the provided element with respect to the viewport. */ private _getElementBounds(element: HTMLElement): ElementBoundingPositions { const boundingClientRect = element.getBoundingClientRect(); return { diff --git a/src/lib/core/overlay/position/connected-position.ts b/src/lib/core/overlay/position/connected-position.ts index 0778c45d53cc..7d8d093199ad 100644 --- a/src/lib/core/overlay/position/connected-position.ts +++ b/src/lib/core/overlay/position/connected-position.ts @@ -1,4 +1,5 @@ /** Horizontal dimension of a connection point on the perimeter of the origin or overlay element. */ +import {Optional} from '@angular/core'; export type HorizontalConnectionPos = 'start' | 'center' | 'end'; /** Vertical dimension of a connection point on the perimeter of the origin or overlay element. */ @@ -42,5 +43,5 @@ export class ScrollableViewProperties { /** The change event emitted by the strategy when a fallback position is used. */ export class ConnectedOverlayPositionChange { constructor(public connectionPair: ConnectionPositionPair, - public scrollableViewProperties: ScrollableViewProperties) {} + @Optional() public scrollableViewProperties: ScrollableViewProperties) {} } diff --git a/src/lib/core/overlay/scroll/scroll-dispatcher.spec.ts b/src/lib/core/overlay/scroll/scroll-dispatcher.spec.ts index 8975198ab9ce..8b0ad84a2c94 100644 --- a/src/lib/core/overlay/scroll/scroll-dispatcher.spec.ts +++ b/src/lib/core/overlay/scroll/scroll-dispatcher.spec.ts @@ -1,12 +1,10 @@ import {inject, TestBed, async, ComponentFixture} from '@angular/core/testing'; -import {NgModule, Component, ViewChild, ElementRef} from '@angular/core'; +import {NgModule, Component, ViewChild, ElementRef, QueryList, ViewChildren} from '@angular/core'; import {ScrollDispatcher} from './scroll-dispatcher'; import {OverlayModule} from '../overlay-directives'; import {Scrollable} from './scrollable'; describe('Scroll Dispatcher', () => { - let scroll: ScrollDispatcher; - let fixture: ComponentFixture; beforeEach(async(() => { TestBed.configureTestingModule({ @@ -16,45 +14,71 @@ describe('Scroll Dispatcher', () => { TestBed.compileComponents(); })); - beforeEach(inject([ScrollDispatcher], (s: ScrollDispatcher) => { - scroll = s; + describe('Basic usage', () => { + let scroll: ScrollDispatcher; + let fixture: ComponentFixture; - fixture = TestBed.createComponent(ScrollingComponent); - fixture.detectChanges(); - })); + beforeEach(inject([ScrollDispatcher], (s: ScrollDispatcher) => { + scroll = s; - it('should be registered with the scrollable directive with the scroll service', () => { - const componentScrollable = fixture.componentInstance.scrollable; - expect(scroll.scrollableReferences.has(componentScrollable)).toBe(true); - }); + fixture = TestBed.createComponent(ScrollingComponent); + fixture.detectChanges(); + })); + + it('should be registered with the scrollable directive with the scroll service', () => { + const componentScrollable = fixture.componentInstance.scrollable; + expect(scroll.scrollableReferences.has(componentScrollable)).toBe(true); + }); + + it('should have the scrollable directive deregistered when the component is destroyed', () => { + const componentScrollable = fixture.componentInstance.scrollable; + expect(scroll.scrollableReferences.has(componentScrollable)).toBe(true); - it('should have the scrollable directive deregistered when the component is destroyed', () => { - const componentScrollable = fixture.componentInstance.scrollable; - expect(scroll.scrollableReferences.has(componentScrollable)).toBe(true); + fixture.destroy(); + expect(scroll.scrollableReferences.has(componentScrollable)).toBe(false); + }); + + it('should notify through the directive and service that a scroll event occurred', () => { + let hasDirectiveScrollNotified = false; + // Listen for notifications from scroll directive + let scrollable = fixture.componentInstance.scrollable; + scrollable.elementScrolled().subscribe(() => { hasDirectiveScrollNotified = true; }); + + // Listen for notifications from scroll service + let hasServiceScrollNotified = false; + scroll.scrolled().subscribe(() => { hasServiceScrollNotified = true; }); - fixture.destroy(); - expect(scroll.scrollableReferences.has(componentScrollable)).toBe(false); + // Emit a scroll event from the scrolling element in our component. + // This event should be picked up by the scrollable directive and notify. + // The notification should be picked up by the service. + const scrollEvent = document.createEvent('UIEvents'); + scrollEvent.initUIEvent('scroll', true, true, window, 0); + fixture.componentInstance.scrollingElement.nativeElement.dispatchEvent(scrollEvent); + + expect(hasDirectiveScrollNotified).toBe(true); + expect(hasServiceScrollNotified).toBe(true); + }); }); - it('should notify through the directive and service that a scroll event occurred', () => { - let hasDirectiveScrollNotified = false; - // Listen for notifications from scroll directive - let scrollable = fixture.componentInstance.scrollable; - scrollable.elementScrolled().subscribe(() => { hasDirectiveScrollNotified = true; }); - - // Listen for notifications from scroll service - let hasServiceScrollNotified = false; - scroll.scrolled().subscribe(() => { hasServiceScrollNotified = true; }); - - // Emit a scroll event from the scrolling element in our component. - // This event should be picked up by the scrollable directive and notify. - // The notification should be picked up by the service. - const scrollEvent = document.createEvent('UIEvents'); - scrollEvent.initUIEvent('scroll', true, true, window, 0); - fixture.componentInstance.scrollingElement.nativeElement.dispatchEvent(scrollEvent); - - expect(hasDirectiveScrollNotified).toBe(true); - expect(hasServiceScrollNotified).toBe(true); + fdescribe('Nested scrollables', () => { + let scroll: ScrollDispatcher; + let fixture: ComponentFixture; + + beforeEach(inject([ScrollDispatcher], (s: ScrollDispatcher) => { + scroll = s; + + fixture = TestBed.createComponent(NestedScrollingComponent); + fixture.detectChanges(); + })); + + it('should be able to identify the containing scrollables of an element', () => { + const interestingElement = fixture.componentInstance.interestingElement; + const scrollContainers = scroll.getScrollContainers(interestingElement); + const scrollableElementIds = + scrollContainers.map(scrollable => scrollable.getElementRef().nativeElement.id); + + expect(scrollableElementIds).toEqual(['scrollable-1', 'scrollable-1a']); + }); }); }); @@ -68,7 +92,25 @@ class ScrollingComponent { @ViewChild('scrollingElement') scrollingElement: ElementRef; } -const TEST_COMPONENTS = [ScrollingComponent]; + +/** Component containing nested scrollables. */ +@Component({ + template: ` +
+
+
+
+
+
+
+ ` +}) +class NestedScrollingComponent { + @ViewChild('interestingElement') interestingElement: ElementRef; + @ViewChildren(Scrollable) scrollables: QueryList; +} + +const TEST_COMPONENTS = [ScrollingComponent, NestedScrollingComponent]; @NgModule({ imports: [OverlayModule], providers: [ScrollDispatcher], diff --git a/src/lib/tooltip/tooltip.spec.ts b/src/lib/tooltip/tooltip.spec.ts index b48092a644b9..572f6a726977 100644 --- a/src/lib/tooltip/tooltip.spec.ts +++ b/src/lib/tooltip/tooltip.spec.ts @@ -300,7 +300,7 @@ describe('MdTooltip', () => { }); - fdescribe('scrollable usage', () => { + describe('scrollable usage', () => { let fixture: ComponentFixture; let buttonDebugElement: DebugElement; let buttonElement: HTMLButtonElement; From 56f3497c03b436e6a3d30172eabf99e4720cca20 Mon Sep 17 00:00:00 2001 From: Andrew Seguin Date: Fri, 16 Dec 2016 15:17:08 -0800 Subject: [PATCH 09/22] add comment about the ScrollableViewProperties --- .../position/connected-position-strategy.ts | 11 ++++---- .../overlay/position/connected-position.ts | 27 +++++++++++++++++-- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/src/lib/core/overlay/position/connected-position-strategy.ts b/src/lib/core/overlay/position/connected-position-strategy.ts index 11f8a3765bb7..6861629187d7 100644 --- a/src/lib/core/overlay/position/connected-position-strategy.ts +++ b/src/lib/core/overlay/position/connected-position-strategy.ts @@ -126,8 +126,9 @@ export class ConnectedPositionStrategy implements PositionStrategy { } /** - * Sets the list of scrolling or resizing containers that host the connectedTo element so that - * on reposition we can evaluate if it has been clipped when repositioned. + * Sets the list of Scrollable containers that host the origin element so that + * on reposition we can evaluate if it or the overlay has been clipped or outside view. Every + * Scrollable must be an ancestor element of the strategy's origin element. */ withScrollableContainers(scrollables: Scrollable[]) { this.scrollables = scrollables; @@ -271,15 +272,15 @@ export class ConnectedPositionStrategy implements PositionStrategy { * or completely outside the view of any of the strategy's scrollables. */ private getScrollableViewProperties(overlay: HTMLElement): ScrollableViewProperties { - const triggerBounds = this._getElementBounds(this._connectedTo.nativeElement); + const originBounds = this._getElementBounds(this._origin); const overlayBounds = this._getElementBounds(overlay); const scrollContainerBounds = this.scrollables.map((scrollable: Scrollable) => { return this._getElementBounds(scrollable.getElementRef().nativeElement); }); return { - isTriggerClipped: this.isElementClipped(triggerBounds, scrollContainerBounds), - isTriggerOutsideView: this.isElementOutsideView(triggerBounds, scrollContainerBounds), + isOriginClipped: this.isElementClipped(originBounds, scrollContainerBounds), + isOriginOutsideView: this.isElementOutsideView(originBounds, scrollContainerBounds), isOverlayClipped: this.isElementClipped(overlayBounds, scrollContainerBounds), isOverlayOutsideView: this.isElementOutsideView(overlayBounds, scrollContainerBounds), }; diff --git a/src/lib/core/overlay/position/connected-position.ts b/src/lib/core/overlay/position/connected-position.ts index 7d8d093199ad..a995851bcf89 100644 --- a/src/lib/core/overlay/position/connected-position.ts +++ b/src/lib/core/overlay/position/connected-position.ts @@ -33,9 +33,32 @@ export class ConnectionPositionPair { } } +/** + * Set of properties regarding the position of the origin and overlay relative to the viewport + * with respect to the containing Scrollable elements. + * + * The overlay and origin are clipped if any part of their bounding client rectangle exceeds the + * bounds of any one of the strategy's Scrollable's bounding client rectangle. + * + * The overlay and origin are outside view if there is no overlap between their bounding client + * rectangle and any one of the strategy's Scrollable's bounding client rectangle. + * + * ----------- ----------- + * | outside | | clipped | + * | view | -------------------------- + * | | | | | | + * ---------- | ----------- | + * -------------------------- | | + * | | | Scrollable | + * | | | | + * | | -------------------------- + * | Scrollable | + * | | + * -------------------------- + */ export class ScrollableViewProperties { - isTriggerClipped: boolean; - isTriggerOutsideView: boolean; + isOriginClipped: boolean; + isOriginOutsideView: boolean; isOverlayClipped: boolean; isOverlayOutsideView: boolean; } From 70f3dfefe1255e8e3e4845c409e1612f1b1b13ac Mon Sep 17 00:00:00 2001 From: Andrew Seguin Date: Fri, 16 Dec 2016 15:54:34 -0800 Subject: [PATCH 10/22] revert tooltip demo --- src/demo-app/tooltip/tooltip-demo.html | 41 +++++--------------------- src/demo-app/tooltip/tooltip-demo.scss | 13 +------- src/demo-app/tooltip/tooltip-demo.ts | 2 +- 3 files changed, 10 insertions(+), 46 deletions(-) diff --git a/src/demo-app/tooltip/tooltip-demo.html b/src/demo-app/tooltip/tooltip-demo.html index f56767ee6aaa..fea0a8fcac19 100644 --- a/src/demo-app/tooltip/tooltip-demo.html +++ b/src/demo-app/tooltip/tooltip-demo.html @@ -1,14 +1,14 @@ -
+

Tooltip Demo

@@ -40,7 +40,7 @@

Tooltip Demo

Mouse over to - -
- -
- Scroll to see tooltip button. - -
-
- Scroll to see tooltip button. -
\ No newline at end of file diff --git a/src/demo-app/tooltip/tooltip-demo.scss b/src/demo-app/tooltip/tooltip-demo.scss index ea5bf54f70ef..bf0f004cab1d 100644 --- a/src/demo-app/tooltip/tooltip-demo.scss +++ b/src/demo-app/tooltip/tooltip-demo.scss @@ -5,15 +5,4 @@ md-radio-button { display: block; } -} - -.scrolling-container { - margin: 300px; - height: 300px; - width: 300px; - overflow: auto; - - button { - margin: 500px; - } -} +} \ No newline at end of file diff --git a/src/demo-app/tooltip/tooltip-demo.ts b/src/demo-app/tooltip/tooltip-demo.ts index 56046e7156c7..61a9f23e0e74 100644 --- a/src/demo-app/tooltip/tooltip-demo.ts +++ b/src/demo-app/tooltip/tooltip-demo.ts @@ -13,4 +13,4 @@ export class TooltipDemo { message: string = 'Here is the tooltip'; showDelay = 0; hideDelay = 0; -} +} \ No newline at end of file From 5d700ffde6b30150275ec40b582f9b0676bd14da Mon Sep 17 00:00:00 2001 From: Andrew Seguin Date: Fri, 16 Dec 2016 15:55:44 -0800 Subject: [PATCH 11/22] add extra lines to tooltip demo files --- src/demo-app/tooltip/tooltip-demo.html | 2 +- src/demo-app/tooltip/tooltip-demo.scss | 2 +- src/demo-app/tooltip/tooltip-demo.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/demo-app/tooltip/tooltip-demo.html b/src/demo-app/tooltip/tooltip-demo.html index fea0a8fcac19..5266f2eaeb3c 100644 --- a/src/demo-app/tooltip/tooltip-demo.html +++ b/src/demo-app/tooltip/tooltip-demo.html @@ -49,4 +49,4 @@

Tooltip Demo

-
\ No newline at end of file +
diff --git a/src/demo-app/tooltip/tooltip-demo.scss b/src/demo-app/tooltip/tooltip-demo.scss index bf0f004cab1d..39c9602a9e72 100644 --- a/src/demo-app/tooltip/tooltip-demo.scss +++ b/src/demo-app/tooltip/tooltip-demo.scss @@ -5,4 +5,4 @@ md-radio-button { display: block; } -} \ No newline at end of file +} diff --git a/src/demo-app/tooltip/tooltip-demo.ts b/src/demo-app/tooltip/tooltip-demo.ts index 61a9f23e0e74..56046e7156c7 100644 --- a/src/demo-app/tooltip/tooltip-demo.ts +++ b/src/demo-app/tooltip/tooltip-demo.ts @@ -13,4 +13,4 @@ export class TooltipDemo { message: string = 'Here is the tooltip'; showDelay = 0; hideDelay = 0; -} \ No newline at end of file +} From b473919b1953e69676ec99ff414a55aa1a9bd030 Mon Sep 17 00:00:00 2001 From: Andrew Seguin Date: Tue, 20 Dec 2016 10:15:48 -0800 Subject: [PATCH 12/22] add connected position strategy tests --- src/demo-app/tooltip/tooltip-demo.html | 4 +- .../connected-position-strategy.spec.ts | 780 ++++++++++-------- .../overlay/scroll/scroll-dispatcher.spec.ts | 2 +- src/lib/core/overlay/scroll/scrollable.ts | 6 - 4 files changed, 456 insertions(+), 336 deletions(-) diff --git a/src/demo-app/tooltip/tooltip-demo.html b/src/demo-app/tooltip/tooltip-demo.html index 5266f2eaeb3c..2c0770c601c9 100644 --- a/src/demo-app/tooltip/tooltip-demo.html +++ b/src/demo-app/tooltip/tooltip-demo.html @@ -1,8 +1,8 @@

Tooltip Demo

-

- + Scroll down while tooltip is open to see it hide automatically

diff --git a/src/demo-app/tooltip/tooltip-demo.ts b/src/demo-app/tooltip/tooltip-demo.ts index 56046e7156c7..a18c5605a24d 100644 --- a/src/demo-app/tooltip/tooltip-demo.ts +++ b/src/demo-app/tooltip/tooltip-demo.ts @@ -12,5 +12,5 @@ export class TooltipDemo { position: TooltipPosition = 'below'; message: string = 'Here is the tooltip'; showDelay = 0; - hideDelay = 0; + hideDelay = 1000; } diff --git a/src/lib/sidenav/sidenav.ts b/src/lib/sidenav/sidenav.ts index 3bd9391dd71c..89852ac84dd4 100644 --- a/src/lib/sidenav/sidenav.ts +++ b/src/lib/sidenav/sidenav.ts @@ -22,19 +22,7 @@ import {FocusTrap} from '../core/a11y/focus-trap'; import {ESCAPE} from '../core/keyboard/keycodes'; import {OverlayModule} from '../core/overlay/overlay-directives'; import {InteractivityChecker} from '../core/a11y/interactivity-checker'; -<<<<<<< ebeb579c60bb44de259764cd9d226ea92252671e -<<<<<<< 38f2302c080b3941ee7be5289c9a47a0d1a3bcef import {ScrollDispatcher} from '../core/overlay/scroll/scroll-dispatcher'; -======= -import {MdLiveAnnouncer} from '../core/a11y/live-announcer'; -<<<<<<< 7f62f0a674630555a66a123367e701089f68e338 -import {ScrollDispatcher} from '../core/scroll/scroll-dispatcher'; ->>>>>>> review response -======= -======= ->>>>>>> make lint happy -import {ScrollDispatcher} from '../core/overlay/scroll/scroll-dispatcher'; ->>>>>>> move scroll to overlay /** Exception thrown when two MdSidenav are matching the same side. */ From 6fce9414aa91fc9ad3fe9ff02437aa44c7e82462 Mon Sep 17 00:00:00 2001 From: Andrew Seguin Date: Tue, 20 Dec 2016 15:37:01 -0800 Subject: [PATCH 15/22] make FF and IE happy in tests --- .../position/connected-position-strategy.spec.ts | 14 +++++++++----- test/karma.config.ts | 2 +- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/lib/core/overlay/position/connected-position-strategy.spec.ts b/src/lib/core/overlay/position/connected-position-strategy.spec.ts index 3eb553a2bfff..cff8cbd28a91 100644 --- a/src/lib/core/overlay/position/connected-position-strategy.spec.ts +++ b/src/lib/core/overlay/position/connected-position-strategy.spec.ts @@ -430,7 +430,7 @@ describe('ConnectedPositionStrategy', () => { } }); - describe('onPositionChange with scrollable view properties', () => { + fdescribe('onPositionChange with scrollable view properties', () => { let overlayElement: HTMLElement; let strategy: ConnectedPositionStrategy; @@ -447,9 +447,7 @@ describe('ConnectedPositionStrategy', () => { overlayContainerElement.appendChild(overlayElement); // Set up the origin - let originElement = createPositionedBlockElement(); - originElement.style.top = '0'; - originElement.style.left = '0'; + let originElement = createBlockElement(); originElement.style.margin = '0 1000px 1000px 0'; // Added so that the container scrolls // Create a scrollable container and put the origin inside @@ -534,10 +532,16 @@ describe('ConnectedPositionStrategy', () => { /** Creates an absolutely positioned, display: block element with a default size. */ function createPositionedBlockElement() { - let element = document.createElement('div'); + let element = createBlockElement(); element.style.position = 'absolute'; element.style.top = '0'; element.style.left = '0'; + return element; +} + +/** Creates a block element with a default size. */ +function createBlockElement() { + let element = document.createElement('div'); element.style.width = `${DEFAULT_WIDTH}px`; element.style.height = `${DEFAULT_HEIGHT}px`; element.style.backgroundColor = 'rebeccapurple'; diff --git a/test/karma.config.ts b/test/karma.config.ts index e5fb72704ed0..0854147973cb 100644 --- a/test/karma.config.ts +++ b/test/karma.config.ts @@ -84,7 +84,7 @@ export function config(config) { browserDisconnectTimeout: 20000, browserNoActivityTimeout: 240000, captureTimeout: 120000, - browsers: ['Chrome_1024x768'], + browsers: ['Firefox'], singleRun: false }); From b0fa57239a4557ba169439357d7a17700ef688b1 Mon Sep 17 00:00:00 2001 From: Andrew Seguin Date: Tue, 20 Dec 2016 15:38:08 -0800 Subject: [PATCH 16/22] change test back to Chrome --- test/karma.config.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/karma.config.ts b/test/karma.config.ts index 0854147973cb..e5fb72704ed0 100644 --- a/test/karma.config.ts +++ b/test/karma.config.ts @@ -84,7 +84,7 @@ export function config(config) { browserDisconnectTimeout: 20000, browserNoActivityTimeout: 240000, captureTimeout: 120000, - browsers: ['Firefox'], + browsers: ['Chrome_1024x768'], singleRun: false }); From 67a98fbfc5e241402f4e4037c5e4476b7313bd95 Mon Sep 17 00:00:00 2001 From: Andrew Seguin Date: Tue, 20 Dec 2016 15:42:34 -0800 Subject: [PATCH 17/22] remove fdescribe --- .../core/overlay/position/connected-position-strategy.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/core/overlay/position/connected-position-strategy.spec.ts b/src/lib/core/overlay/position/connected-position-strategy.spec.ts index cff8cbd28a91..f00af334acc6 100644 --- a/src/lib/core/overlay/position/connected-position-strategy.spec.ts +++ b/src/lib/core/overlay/position/connected-position-strategy.spec.ts @@ -430,7 +430,7 @@ describe('ConnectedPositionStrategy', () => { } }); - fdescribe('onPositionChange with scrollable view properties', () => { + describe('onPositionChange with scrollable view properties', () => { let overlayElement: HTMLElement; let strategy: ConnectedPositionStrategy; From 6a740a32efcab48955bd202f770b232e63231f3b Mon Sep 17 00:00:00 2001 From: Andrew Seguin Date: Wed, 21 Dec 2016 12:54:51 -0800 Subject: [PATCH 18/22] resolve comments --- src/lib/core/overlay/position/connected-position-strategy.ts | 5 +++++ src/lib/tooltip/tooltip.ts | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/lib/core/overlay/position/connected-position-strategy.ts b/src/lib/core/overlay/position/connected-position-strategy.ts index 6861629187d7..a6cac4e28e36 100644 --- a/src/lib/core/overlay/position/connected-position-strategy.ts +++ b/src/lib/core/overlay/position/connected-position-strategy.ts @@ -11,6 +11,11 @@ import {Subject} from 'rxjs/Subject'; import {Observable} from 'rxjs/Observable'; import {Scrollable} from '../scroll/scrollable'; +/** + * Container to hold the bounding positions of a particular element with respect to the viewport, + * where top and bottom are the y-axis coordinates of the bounding rectangle and left and right are + * the x-axis coordinates. + */ export type ElementBoundingPositions = { top: number; right: number; diff --git a/src/lib/tooltip/tooltip.ts b/src/lib/tooltip/tooltip.ts index 65000b686053..e703e7db4b42 100644 --- a/src/lib/tooltip/tooltip.ts +++ b/src/lib/tooltip/tooltip.ts @@ -185,7 +185,7 @@ export class MdTooltip implements OnInit, OnDestroy { // close the tooltip. let strategy = this._overlay.position().connectedTo(this._elementRef, origin, position); strategy.withScrollableContainers(this._scrollDispatcher.getScrollContainers(this._elementRef)); - strategy.onPositionChange.subscribe((change: ConnectedOverlayPositionChange) => { + strategy.onPositionChange.subscribe(change => { if (change.scrollableViewProperties.isOverlayClipped) { this.hide(0); } From aada2525b88d00b661dd6757a1d4794dfea38107 Mon Sep 17 00:00:00 2001 From: Andrew Seguin Date: Wed, 21 Dec 2016 13:16:11 -0800 Subject: [PATCH 19/22] remove usage of scrollables until throttling is set up --- src/demo-app/tooltip/tooltip-demo.html | 5 +- .../connected-position-strategy.spec.ts | 4 +- src/lib/sidenav/sidenav-container.html | 2 +- src/lib/sidenav/sidenav.ts | 3 +- src/lib/tooltip/tooltip.spec.ts | 75 ++----------------- src/lib/tooltip/tooltip.ts | 31 +------- 6 files changed, 17 insertions(+), 103 deletions(-) diff --git a/src/demo-app/tooltip/tooltip-demo.html b/src/demo-app/tooltip/tooltip-demo.html index 18bed245620a..5266f2eaeb3c 100644 --- a/src/demo-app/tooltip/tooltip-demo.html +++ b/src/demo-app/tooltip/tooltip-demo.html @@ -1,8 +1,8 @@

Tooltip Demo

-

- - Scroll down while tooltip is open to see it hide automatically

diff --git a/src/lib/core/overlay/position/connected-position-strategy.spec.ts b/src/lib/core/overlay/position/connected-position-strategy.spec.ts index f00af334acc6..c50217cd982e 100644 --- a/src/lib/core/overlay/position/connected-position-strategy.spec.ts +++ b/src/lib/core/overlay/position/connected-position-strategy.spec.ts @@ -432,6 +432,7 @@ describe('ConnectedPositionStrategy', () => { describe('onPositionChange with scrollable view properties', () => { let overlayElement: HTMLElement; + let overlayContainerElement: HTMLElement; let strategy: ConnectedPositionStrategy; let scrollable: HTMLDivElement; @@ -441,7 +442,7 @@ describe('ConnectedPositionStrategy', () => { beforeEach(() => { // Set up the overlay - let overlayContainerElement = createFixedElement(); + overlayContainerElement = createFixedElement(); overlayElement = createPositionedBlockElement(); document.body.appendChild(overlayContainerElement); overlayContainerElement.appendChild(overlayElement); @@ -471,6 +472,7 @@ describe('ConnectedPositionStrategy', () => { afterEach(() => { onPositionChangeSubscription.unsubscribe(); document.body.removeChild(scrollable); + document.body.removeChild(overlayContainerElement); }); it('should not have origin or overlay clipped or out of view without scroll', () => { diff --git a/src/lib/sidenav/sidenav-container.html b/src/lib/sidenav/sidenav-container.html index c916ef399af4..652ebf250c62 100644 --- a/src/lib/sidenav/sidenav-container.html +++ b/src/lib/sidenav/sidenav-container.html @@ -3,6 +3,6 @@ -

+
diff --git a/src/lib/sidenav/sidenav.ts b/src/lib/sidenav/sidenav.ts index 89852ac84dd4..f12e3c3c0f74 100644 --- a/src/lib/sidenav/sidenav.ts +++ b/src/lib/sidenav/sidenav.ts @@ -22,7 +22,6 @@ import {FocusTrap} from '../core/a11y/focus-trap'; import {ESCAPE} from '../core/keyboard/keycodes'; import {OverlayModule} from '../core/overlay/overlay-directives'; import {InteractivityChecker} from '../core/a11y/interactivity-checker'; -import {ScrollDispatcher} from '../core/overlay/scroll/scroll-dispatcher'; /** Exception thrown when two MdSidenav are matching the same side. */ @@ -516,7 +515,7 @@ export class MdSidenavModule { static forRoot(): ModuleWithProviders { return { ngModule: MdSidenavModule, - providers: [InteractivityChecker, ScrollDispatcher] + providers: [InteractivityChecker] }; } } diff --git a/src/lib/tooltip/tooltip.spec.ts b/src/lib/tooltip/tooltip.spec.ts index 572f6a726977..70abb527b47c 100644 --- a/src/lib/tooltip/tooltip.spec.ts +++ b/src/lib/tooltip/tooltip.spec.ts @@ -1,18 +1,17 @@ import { - async, - ComponentFixture, - TestBed, - tick, - fakeAsync, - flushMicrotasks + async, + ComponentFixture, + TestBed, + tick, + fakeAsync, + flushMicrotasks } from '@angular/core/testing'; -import {Component, DebugElement, AnimationTransitionEvent, ViewChild} from '@angular/core'; +import {Component, DebugElement, AnimationTransitionEvent} from '@angular/core'; import {By} from '@angular/platform-browser'; import {TooltipPosition, MdTooltip, MdTooltipModule} from './tooltip'; import {OverlayContainer} from '../core'; import {Dir, LayoutDirection} from '../core/rtl/dir'; import {OverlayModule} from '../core/overlay/overlay-directives'; -import {Scrollable} from '../core/overlay/scroll/scrollable'; const initialTooltipMessage = 'initial tooltip message'; @@ -23,7 +22,7 @@ describe('MdTooltip', () => { beforeEach(async(() => { TestBed.configureTestingModule({ imports: [MdTooltipModule.forRoot(), OverlayModule], - declarations: [BasicTooltipDemo, ScrollableTooltipDemo], + declarations: [BasicTooltipDemo], providers: [ {provide: OverlayContainer, useFactory: () => { overlayContainerElement = document.createElement('div'); @@ -298,34 +297,6 @@ describe('MdTooltip', () => { }).toThrowError('Tooltip position "everywhere" is invalid.'); }); }); - - - describe('scrollable usage', () => { - let fixture: ComponentFixture; - let buttonDebugElement: DebugElement; - let buttonElement: HTMLButtonElement; - let tooltipDirective: MdTooltip; - - beforeEach(() => { - fixture = TestBed.createComponent(ScrollableTooltipDemo); - fixture.detectChanges(); - buttonDebugElement = fixture.debugElement.query(By.css('button')); - buttonElement = buttonDebugElement.nativeElement; - tooltipDirective = buttonDebugElement.injector.get(MdTooltip); - }); - - it('should hide tooltip if clipped after changing positions', fakeAsync(() => { - expect(tooltipDirective._tooltipInstance).toBeUndefined(); - - tooltipDirective.show(); - tick(0); // Tick for the show delay (default is 0) - expect(tooltipDirective._isTooltipVisible()).toBe(true); - - fixture.componentInstance.scrollDown(); - tick(); - expect(tooltipDirective._isTooltipVisible()).toBe(false); - })); - }); }); @Component({ @@ -343,33 +314,3 @@ class BasicTooltipDemo { showButton: boolean = true; } -@Component({ - selector: 'app', - template: ` -
- -
` -}) -class ScrollableTooltipDemo { - position: string = 'below'; - message: string = initialTooltipMessage; - showButton: boolean = true; - - @ViewChild(Scrollable) scrollingContainer: Scrollable; - - scrollDown() { - const scrollingContainerEl = this.scrollingContainer.getElementRef().nativeElement; - scrollingContainerEl.scrollTop = 50; - - // Emit a scroll event from the scrolling element in our component. - // This event should be picked up by the scrollable directive and notify. - // The notification should be picked up by the service. - const scrollEvent = document.createEvent('UIEvents'); - scrollEvent.initUIEvent('scroll', true, true, window, 0); - scrollingContainerEl.dispatchEvent(scrollEvent); - } -} diff --git a/src/lib/tooltip/tooltip.ts b/src/lib/tooltip/tooltip.ts index e703e7db4b42..9307170949c9 100644 --- a/src/lib/tooltip/tooltip.ts +++ b/src/lib/tooltip/tooltip.ts @@ -31,9 +31,7 @@ import {MdTooltipInvalidPositionError} from './tooltip-errors'; import {Observable} from 'rxjs/Observable'; import {Subject} from 'rxjs/Subject'; import {Dir} from '../core/rtl/dir'; -import {ScrollDispatcher} from '../core/overlay/scroll/scroll-dispatcher'; import {OVERLAY_PROVIDERS} from '../core/overlay/overlay'; -import {ConnectedOverlayPositionChange} from '../core/overlay/position/connected-position'; import 'rxjs/add/operator/first'; export type TooltipPosition = 'left' | 'right' | 'above' | 'below' | 'before' | 'after'; @@ -57,7 +55,7 @@ export const TOUCHEND_HIDE_DELAY = 1500; }, exportAs: 'mdTooltip', }) -export class MdTooltip implements OnInit, OnDestroy { +export class MdTooltip implements OnDestroy { _overlayRef: OverlayRef; _tooltipInstance: TooltipComponent; @@ -106,22 +104,11 @@ export class MdTooltip implements OnInit, OnDestroy { set _deprecatedMessage(v: string) { this.message = v; } constructor(private _overlay: Overlay, - private _scrollDispatcher: ScrollDispatcher, private _elementRef: ElementRef, private _viewContainerRef: ViewContainerRef, private _ngZone: NgZone, @Optional() private _dir: Dir) { } - ngOnInit() { - // When a scroll on the page occurs, update the position in case this tooltip needs - // to be repositioned. - this._scrollDispatcher.scrolled().subscribe(() => { - if (this._overlayRef) { - this._overlayRef.updatePosition(); - } - }); - } - /** * Dispose the tooltip when destroyed. */ @@ -179,18 +166,7 @@ export class MdTooltip implements OnInit, OnDestroy { private _createOverlay(): void { let origin = this._getOrigin(); let position = this._getOverlayPosition(); - - // Create connected position strategy that listens for scroll events to reposition. - // After position changes occur and the overlay is clipped by a parent scrollable then - // close the tooltip. let strategy = this._overlay.position().connectedTo(this._elementRef, origin, position); - strategy.withScrollableContainers(this._scrollDispatcher.getScrollContainers(this._elementRef)); - strategy.onPositionChange.subscribe(change => { - if (change.scrollableViewProperties.isOverlayClipped) { - this.hide(0); - } - }); - let config = new OverlayState(); config.positionStrategy = strategy; @@ -412,10 +388,7 @@ export class MdTooltipModule { static forRoot(): ModuleWithProviders { return { ngModule: MdTooltipModule, - providers: [ - OVERLAY_PROVIDERS, - ScrollDispatcher - ] + providers: [OVERLAY_PROVIDERS] }; } } From ac2f572a8ff146f4af3ca87983760166b973441f Mon Sep 17 00:00:00 2001 From: Andrew Seguin Date: Wed, 21 Dec 2016 13:24:57 -0800 Subject: [PATCH 20/22] remove sha --- src/lib/core/overlay/position/connected-position-strategy.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/lib/core/overlay/position/connected-position-strategy.ts b/src/lib/core/overlay/position/connected-position-strategy.ts index a6cac4e28e36..e5b1059faf51 100644 --- a/src/lib/core/overlay/position/connected-position-strategy.ts +++ b/src/lib/core/overlay/position/connected-position-strategy.ts @@ -140,7 +140,6 @@ export class ConnectedPositionStrategy implements PositionStrategy { } /** - <<<<<<< b07c918b15021de7b9b5af2928d994203174cbe7 * Adds a new preferred fallback position. * @param originPos * @param overlayPos From ce03106b5b7b002d9ee6c4d8cb81c68242815b2f Mon Sep 17 00:00:00 2001 From: Andrew Seguin Date: Wed, 21 Dec 2016 15:53:24 -0800 Subject: [PATCH 21/22] remove combineLatest --- src/lib/core/overlay/scroll/scroll-dispatcher.ts | 1 - tools/gulp/tasks/components.ts | 1 - 2 files changed, 2 deletions(-) diff --git a/src/lib/core/overlay/scroll/scroll-dispatcher.ts b/src/lib/core/overlay/scroll/scroll-dispatcher.ts index b9b339de2559..d9f2c25d860f 100644 --- a/src/lib/core/overlay/scroll/scroll-dispatcher.ts +++ b/src/lib/core/overlay/scroll/scroll-dispatcher.ts @@ -4,7 +4,6 @@ import {Subject} from 'rxjs/Subject'; import {Observable} from 'rxjs/Observable'; import {Subscription} from 'rxjs/Subscription'; import 'rxjs/add/observable/fromEvent'; -import 'rxjs/add/observable/combineLatest'; /** diff --git a/tools/gulp/tasks/components.ts b/tools/gulp/tasks/components.ts index b4dd9f6d5118..66ea3e82ca1a 100644 --- a/tools/gulp/tasks/components.ts +++ b/tools/gulp/tasks/components.ts @@ -69,7 +69,6 @@ task(':build:components:rollup', () => { // Rxjs dependencies 'rxjs/Subject': 'Rx', - 'rxjs/add/observable/combineLatest': 'Rx.Observable.prototype', 'rxjs/add/observable/fromEvent': 'Rx.Observable', 'rxjs/add/observable/forkJoin': 'Rx.Observable', 'rxjs/add/observable/of': 'Rx.Observable', From a343d78b7d4f2fbbc809e73493dcd266579ed8cf Mon Sep 17 00:00:00 2001 From: Andrew Seguin Date: Wed, 21 Dec 2016 16:34:44 -0800 Subject: [PATCH 22/22] remove unused OnInit import --- src/lib/tooltip/tooltip.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/lib/tooltip/tooltip.ts b/src/lib/tooltip/tooltip.ts index 9307170949c9..e4c1849452d8 100644 --- a/src/lib/tooltip/tooltip.ts +++ b/src/lib/tooltip/tooltip.ts @@ -14,8 +14,7 @@ import { AnimationTransitionEvent, NgZone, Optional, - OnDestroy, - OnInit + OnDestroy } from '@angular/core'; import { Overlay,