diff --git a/aio/src/app/app.component.spec.ts b/aio/src/app/app.component.spec.ts index e968a88827..276cf91fce 100644 --- a/aio/src/app/app.component.spec.ts +++ b/aio/src/app/app.component.spec.ts @@ -5,10 +5,13 @@ import { AppModule } from './app.module'; import { GaService } from 'app/shared/ga.service'; import { SearchService } from 'app/search/search.service'; import { MockSearchService } from 'testing/search.service'; +import { LocationService } from 'app/shared/location.service'; +import { MockLocationService } from 'testing/location.service'; describe('AppComponent', () => { let component: AppComponent; let fixture: ComponentFixture; + const initialUrl = 'a/b'; beforeEach(async(() => { TestBed.configureTestingModule({ @@ -16,7 +19,8 @@ describe('AppComponent', () => { providers: [ { provide: APP_BASE_HREF, useValue: '/' }, { provide: SearchService, useClass: MockSearchService }, - { provide: GaService, useClass: TestGaService } + { provide: GaService, useClass: TestGaService }, + { provide: LocationService, useFactory: () => new MockLocationService(initialUrl) } ] }); TestBed.compileComponents(); @@ -33,11 +37,10 @@ describe('AppComponent', () => { describe('google analytics', () => { it('should call gaService.locationChanged with initial URL', () => { - const url = window.location.pathname.substr(1); // strip leading '/' const { locationChanged } = TestBed.get(GaService) as TestGaService; expect(locationChanged.calls.count()).toBe(1, 'gaService.locationChanged'); const args = locationChanged.calls.first().args; - expect(args[0]).toBe(url); + expect(args[0]).toBe(initialUrl); }); // Todo: add test to confirm tracking URL when navigate. @@ -70,6 +73,17 @@ describe('AppComponent', () => { expect(searchService.loadIndex).toHaveBeenCalled(); })); }); + + describe('click intercepting', () => { + it('should intercept clicks on anchors and call `location.handleAnchorClick()`', + inject([LocationService], (location: LocationService) => { + const anchorElement: HTMLAnchorElement = document.createElement('a'); + anchorElement.href = 'some/local/url'; + fixture.nativeElement.append(anchorElement); + anchorElement.click(); + expect(location.handleAnchorClick).toHaveBeenCalledWith(anchorElement, 0, false, false); + })); + }); }); class TestGaService { diff --git a/aio/src/app/app.component.ts b/aio/src/app/app.component.ts index f021ed9b3f..dfd1812749 100644 --- a/aio/src/app/app.component.ts +++ b/aio/src/app/app.component.ts @@ -1,4 +1,4 @@ -import { Component, ViewChild, OnInit } from '@angular/core'; +import { Component, HostListener, ViewChild, OnInit } from '@angular/core'; import { Observable } from 'rxjs/Observable'; import { GaService } from 'app/shared/ga.service'; @@ -26,7 +26,7 @@ export class AppComponent implements OnInit { constructor( documentService: DocumentService, gaService: GaService, - locationService: LocationService, + private locationService: LocationService, navigationService: NavigationService, private searchService: SearchService) { @@ -46,4 +46,12 @@ export class AppComponent implements OnInit { onResize(width) { this.isSideBySide = width > this.sideBySideWidth; } + + @HostListener('click', ['$event.target', '$event.button', '$event.ctrlKey', '$event.metaKey']) + onClick(eventTarget: HTMLElement, button: number, ctrlKey: boolean, metaKey: boolean): boolean { + if (eventTarget instanceof HTMLAnchorElement) { + return this.locationService.handleAnchorClick(eventTarget, button, ctrlKey, metaKey); + } + return true; + } } diff --git a/aio/src/app/app.module.ts b/aio/src/app/app.module.ts index c2a360ba27..0f2d94641c 100644 --- a/aio/src/app/app.module.ts +++ b/aio/src/app/app.module.ts @@ -28,7 +28,6 @@ import { SearchService } from 'app/search/search.service'; import { TopMenuComponent } from 'app/layout/top-menu/top-menu.component'; import { NavMenuComponent } from 'app/layout/nav-menu/nav-menu.component'; import { NavItemComponent } from 'app/layout/nav-item/nav-item.component'; -import { LinkDirective } from 'app/shared/link.directive'; import { SearchResultsComponent } from './search/search-results/search-results.component'; import { SearchBoxComponent } from './search/search-box/search-box.component'; @@ -49,7 +48,6 @@ import { SearchBoxComponent } from './search/search-box/search-box.component'; TopMenuComponent, NavMenuComponent, NavItemComponent, - LinkDirective, SearchResultsComponent, SearchBoxComponent, ], diff --git a/aio/src/app/shared/link.directive.spec.ts b/aio/src/app/shared/link.directive.spec.ts deleted file mode 100644 index e81d516d33..0000000000 --- a/aio/src/app/shared/link.directive.spec.ts +++ /dev/null @@ -1,93 +0,0 @@ -import { async, inject, ComponentFixture, TestBed } from '@angular/core/testing'; -import { By } from '@angular/platform-browser'; -import { Component } from '@angular/core'; -import { LocationService } from 'app/shared/location.service'; -import { MockLocationService } from 'testing/location.service'; -import { LinkDirective } from './link.directive'; - -describe('LinkDirective', () => { - - @Component({ - template: 'Test Link' - }) - class TestComponent { - url: string; - } - - beforeEach(async(() => { - TestBed.configureTestingModule({ - declarations: [ - LinkDirective, - TestComponent - ], - providers: [ - { provide: LocationService, useFactory: () => new MockLocationService('initial/url') } - ] - }) - .compileComponents(); - })); - - it('should attach to all anchor elements', () => { - const fixture = TestBed.createComponent(TestComponent); - const directiveElement = fixture.debugElement.query(By.directive(LinkDirective)); - expect(directiveElement.name).toEqual('a'); - }); - - it('should bind a property to the "href" attribute', () => { - const fixture = TestBed.createComponent(TestComponent); - const directiveElement = fixture.debugElement.query(By.directive(LinkDirective)); - - fixture.componentInstance.url = 'test/url'; - fixture.detectChanges(); - expect(directiveElement.properties['href']).toEqual('test/url'); - }); - - it('should set the "target" attribute to "_blank" if the href is absolute, otherwise "_self"', () => { - const fixture = TestBed.createComponent(TestComponent); - const directiveElement = fixture.debugElement.query(By.directive(LinkDirective)); - - fixture.componentInstance.url = 'http://test/url'; - fixture.detectChanges(); - expect(directiveElement.properties['target']).toEqual('_blank'); - - fixture.componentInstance.url = 'https://test/url'; - fixture.detectChanges(); - expect(directiveElement.properties['target']).toEqual('_blank'); - - fixture.componentInstance.url = 'ftp://test/url'; - fixture.detectChanges(); - expect(directiveElement.properties['target']).toEqual('_blank'); - - fixture.componentInstance.url = '//test/url'; - fixture.detectChanges(); - expect(directiveElement.properties['target']).toEqual('_blank'); - - fixture.componentInstance.url = 'test/url'; - fixture.detectChanges(); - expect(directiveElement.properties['target']).toEqual('_self'); - - fixture.componentInstance.url = '/test/url'; - fixture.detectChanges(); - expect(directiveElement.properties['target']).toEqual('_self'); - }); - - it('should intercept clicks for local urls and call `location.go()`', inject([LocationService], (location: LocationService) => { - const fixture = TestBed.createComponent(TestComponent); - const directiveElement = fixture.debugElement.query(By.directive(LinkDirective)); - fixture.componentInstance.url = 'some/local/url'; - fixture.detectChanges(); - location.go = jasmine.createSpy('Location.go'); - directiveElement.triggerEventHandler('click', null); - expect(location.go).toHaveBeenCalledWith('some/local/url'); - })); - - it('should not intercept clicks for absolute urls', inject([LocationService], (location: LocationService) => { - const fixture = TestBed.createComponent(TestComponent); - const directiveElement = fixture.debugElement.query(By.directive(LinkDirective)); - fixture.componentInstance.url = 'https://some/absolute/url'; - fixture.detectChanges(); - location.go = jasmine.createSpy('Location.go'); - directiveElement.triggerEventHandler('click', null); - expect(location.go).not.toHaveBeenCalled(); - })); -}); diff --git a/aio/src/app/shared/link.directive.ts b/aio/src/app/shared/link.directive.ts deleted file mode 100644 index f0b56e410f..0000000000 --- a/aio/src/app/shared/link.directive.ts +++ /dev/null @@ -1,39 +0,0 @@ -import { Directive, HostListener, HostBinding, Input, OnChanges } from '@angular/core'; -import { LocationService } from 'app/shared/location.service'; -@Directive({ - /* tslint:disable-next-line:directive-selector */ - selector: 'a[href]' -}) -export class LinkDirective implements OnChanges { - - // We need both these decorators to ensure that we can access - // the href programmatically, and that it appears as a real - // attribute on the element. - @Input() - @HostBinding() - href: string; - - @HostBinding() - target: string; - - @HostListener('click', ['$event']) - onClick($event) { - if (this.isAbsolute(this.href)) { - return true; - } else { - this.location.go(this.href); - return false; - } - } - - private isAbsolute(url) { - return /^[a-z]+:\/\/|\/\//i.test(url); - } - - constructor(private location: LocationService) { } - - ngOnChanges() { - this.target = this.isAbsolute(this.href) ? '_blank' : '_self'; - } - -} diff --git a/aio/src/app/shared/location.service.spec.ts b/aio/src/app/shared/location.service.spec.ts index 60cf4d6eac..ab0bf26fc4 100644 --- a/aio/src/app/shared/location.service.spec.ts +++ b/aio/src/app/shared/location.service.spec.ts @@ -221,4 +221,124 @@ describe('LocationService', () => { expect(query).toContain('a%26b%2Bc%20d=value'); }); }); + + describe('handleAnchorClick', () => { + let service: LocationService, anchor: HTMLAnchorElement; + beforeEach(() => { + service = injector.get(LocationService); + anchor = document.createElement('a'); + }); + + describe('intercepting', () => { + it('should intercept clicks on anchors for relative local urls', () => { + anchor.href = 'some/local/url'; + spyOn(service, 'go'); + const result = service.handleAnchorClick(anchor, 0, false, false); + expect(service.go).toHaveBeenCalledWith('some/local/url'); + expect(result).toBe(false); + }); + + it('should intercept clicks on anchors for absolute local urls', () => { + anchor.href = '/some/local/url'; + spyOn(service, 'go'); + const result = service.handleAnchorClick(anchor, 0, false, false); + expect(service.go).toHaveBeenCalledWith('some/local/url'); + expect(result).toBe(false); + }); + + it('should intercept clicks on anchors for local urls, with query params', () => { + anchor.href = 'some/local/url?query=xxx&other=yyy'; + spyOn(service, 'go'); + const result = service.handleAnchorClick(anchor, 0, false, false); + expect(service.go).toHaveBeenCalledWith('some/local/url?query=xxx&other=yyy'); + expect(result).toBe(false); + }); + + it('should intercept clicks on anchors for local urls, with hash fragment', () => { + anchor.href = 'some/local/url#somefragment'; + spyOn(service, 'go'); + const result = service.handleAnchorClick(anchor, 0, false, false); + expect(service.go).toHaveBeenCalledWith('some/local/url#somefragment'); + expect(result).toBe(false); + }); + + it('should intercept clicks on anchors for local urls, with query params and hash fragment', () => { + anchor.href = 'some/local/url?query=xxx&other=yyy#somefragment'; + spyOn(service, 'go'); + const result = service.handleAnchorClick(anchor, 0, false, false); + expect(service.go).toHaveBeenCalledWith('some/local/url?query=xxx&other=yyy#somefragment'); + expect(result).toBe(false); + }); + }); + + describe('not intercepting', () => { + it('should not intercept clicks on anchors for external urls', () => { + anchor.href = 'http://other.com/some/local/url?query=xxx&other=yyy#somefragment'; + spyOn(service, 'go'); + let result = service.handleAnchorClick(anchor, 0, false, false); + expect(service.go).not.toHaveBeenCalled(); + expect(result).toBe(true); + + anchor.href = 'some/local/url.pdf'; + anchor.protocol = 'ftp'; + result = service.handleAnchorClick(anchor, 0, false, false); + expect(service.go).not.toHaveBeenCalled(); + expect(result).toBe(true); + }); + + it('should not intercept clicks on anchors if button is not zero', () => { + anchor.href = 'some/local/url'; + spyOn(service, 'go'); + const result = service.handleAnchorClick(anchor, 1, false, false); + expect(service.go).not.toHaveBeenCalled(); + expect(result).toBe(true); + }); + + it('should not intercept clicks on anchors if ctrl key is pressed', () => { + anchor.href = 'some/local/url'; + spyOn(service, 'go'); + const result = service.handleAnchorClick(anchor, 0, true, false); + expect(service.go).not.toHaveBeenCalled(); + expect(result).toBe(true); + }); + + it('should not intercept clicks on anchors if meta key is pressed', () => { + anchor.href = 'some/local/url'; + spyOn(service, 'go'); + const result = service.handleAnchorClick(anchor, 0, false, true); + expect(service.go).not.toHaveBeenCalled(); + expect(result).toBe(true); + }); + + it('should not intercept clicks on links with (non-_self) targets', () => { + anchor.href = 'some/local/url'; + spyOn(service, 'go'); + + anchor.target = '_blank'; + let result = service.handleAnchorClick(anchor, 0, false, false); + expect(service.go).not.toHaveBeenCalled(); + expect(result).toBe(true); + + anchor.target = '_parent'; + result = service.handleAnchorClick(anchor, 0, false, false); + expect(service.go).not.toHaveBeenCalled(); + expect(result).toBe(true); + + anchor.target = '_top'; + result = service.handleAnchorClick(anchor, 0, false, false); + expect(service.go).not.toHaveBeenCalled(); + expect(result).toBe(true); + + anchor.target = 'other-frame'; + result = service.handleAnchorClick(anchor, 0, false, false); + expect(service.go).not.toHaveBeenCalled(); + expect(result).toBe(true); + + anchor.target = '_self'; + result = service.handleAnchorClick(anchor, 0, false, false); + expect(service.go).toHaveBeenCalledWith('some/local/url'); + expect(result).toBe(false); + }); + }); + }); }); diff --git a/aio/src/app/shared/location.service.ts b/aio/src/app/shared/location.service.ts index f2ebb22384..2cf2a21eb3 100644 --- a/aio/src/app/shared/location.service.ts +++ b/aio/src/app/shared/location.service.ts @@ -6,6 +6,7 @@ import { ReplaySubject } from 'rxjs/ReplaySubject'; @Injectable() export class LocationService { + private readonly urlParser = document.createElement('a'); private urlSubject = new ReplaySubject(1); currentUrl = this.urlSubject.asObservable(); @@ -60,4 +61,44 @@ export class LocationService { this.platformLocation.replaceState({}, label, this.platformLocation.pathname + search); } + + /** + * Since we are using `LocationService` to navigate between docs, without the browser + * reloading the page, we must intercept clicks on links. + * If the link is to a document that we will render, then we navigate using `Location.go()` + * and tell the browser not to handle the event. + * + * In most apps you might do this in a `LinkDirective` attached to anchors but in this app + * we have a special situation where the `DocViewerComponent` is displaying semi-static + * content that cannot contain directives. So all the links in that content would not be + * able to use such a `LinkDirective`. Instead we are adding a click handler to the + * `AppComponent`, whose element contains all the of the application and so captures all + * link clicks both inside and outside the `DocViewerComponent`. + */ + handleAnchorClick(anchor: HTMLAnchorElement, button: number, ctrlKey: boolean, metaKey: boolean) { + + // Check for modifier keys, which indicate the user wants to control navigation + if (button !== 0 || ctrlKey || metaKey) { + return true; + } + + // If there is a target and it is not `_self` then we take this + // as a signal that it doesn't want to be intercepted. + // TODO: should we also allow an explicit `_self` target to opt-out? + const anchorTarget = anchor.target; + if (anchorTarget && anchorTarget !== '_self') { + return true; + } + + // check for external link + const { pathname, search, hash } = anchor; + const relativeUrl = pathname + search + hash; + this.urlParser.href = relativeUrl; + if (anchor.href !== this.urlParser.href) { + return true; + } + + this.go(this.stripLeadingSlashes(relativeUrl)); + return false; + } } diff --git a/aio/src/testing/location.service.ts b/aio/src/testing/location.service.ts index 918b79bc1b..49c47b30d5 100644 --- a/aio/src/testing/location.service.ts +++ b/aio/src/testing/location.service.ts @@ -5,6 +5,9 @@ export class MockLocationService { currentUrl = this.urlSubject.asObservable(); search = jasmine.createSpy('search').and.returnValue({}); setSearch = jasmine.createSpy('setSearch'); + go = jasmine.createSpy('Location.go'); + handleAnchorClick = jasmine.createSpy('Location.handleAnchorClick') + .and.returnValue(false); // prevent click from causing a browser navigation constructor(private initialUrl) {} }