diff --git a/aio/src/app/shared/location.service.spec.ts b/aio/src/app/shared/location.service.spec.ts index cabdb466bc..4d6d8ba616 100644 --- a/aio/src/app/shared/location.service.spec.ts +++ b/aio/src/app/shared/location.service.spec.ts @@ -6,12 +6,14 @@ import { Subject } from 'rxjs'; import { GaService } from 'app/shared/ga.service'; import { SwUpdatesService } from 'app/sw-updates/sw-updates.service'; import { LocationService } from './location.service'; +import { ScrollService } from './scroll.service'; describe('LocationService', () => { let injector: ReflectiveInjector; let location: MockLocationStrategy; let service: LocationService; let swUpdates: MockSwUpdatesService; + let scrollService: MockScrollService; beforeEach(() => { injector = ReflectiveInjector.resolveAndCreate([ @@ -20,12 +22,14 @@ describe('LocationService', () => { { provide: GaService, useClass: TestGaService }, { provide: LocationStrategy, useClass: MockLocationStrategy }, { provide: PlatformLocation, useClass: MockPlatformLocation }, - { provide: SwUpdatesService, useClass: MockSwUpdatesService } + { provide: SwUpdatesService, useClass: MockSwUpdatesService }, + { provide: ScrollService, useClass: MockScrollService } ]); location = injector.get(LocationStrategy); service = injector.get(LocationService); swUpdates = injector.get(SwUpdatesService); + scrollService = injector.get(ScrollService); }); describe('currentUrl', () => { @@ -289,11 +293,14 @@ describe('LocationService', () => { expect(goExternalSpy).toHaveBeenCalledWith(externalUrl); }); - it('should do a "full page navigation" if a ServiceWorker update has been activated', () => { + it('should do a "full page navigation" and remove the stored scroll position when navigating to ' + + 'internal URLs only if a ServiceWorker update has been activated', () => { const goExternalSpy = spyOn(service, 'goExternal'); + const removeStoredScrollPositionSpy = spyOn(scrollService, 'removeStoredScrollPosition'); // Internal URL - No ServiceWorker update service.go('some-internal-url'); + expect(removeStoredScrollPositionSpy).not.toHaveBeenCalled(); expect(goExternalSpy).not.toHaveBeenCalled(); expect(location.path(true)).toEqual('some-internal-url'); @@ -301,7 +308,25 @@ describe('LocationService', () => { swUpdates.updateActivated.next('foo'); service.go('other-internal-url'); expect(goExternalSpy).toHaveBeenCalledWith('other-internal-url'); - expect(location.path(true)).toEqual('some-internal-url'); + expect(removeStoredScrollPositionSpy).toHaveBeenCalled(); + }); + + it('should not remove the stored scroll position when navigating to external URLs', () => { + const removeStoredScrollPositionSpy = spyOn(scrollService, 'removeStoredScrollPosition'); + const goExternalSpy = spyOn(service, 'goExternal'); + const externalUrl = 'http://some/far/away/land'; + const otherExternalUrl = 'http://some/far/far/away/land'; + + // External URL - No ServiceWorker update + service.go(externalUrl); + expect(removeStoredScrollPositionSpy).not.toHaveBeenCalled(); + expect(goExternalSpy).toHaveBeenCalledWith(externalUrl); + + // External URL - ServiceWorker update + swUpdates.updateActivated.next('foo'); + service.go(otherExternalUrl); + expect(removeStoredScrollPositionSpy).not.toHaveBeenCalled(); + expect(goExternalSpy).toHaveBeenCalledWith(otherExternalUrl); }); it('should not update currentUrl for external url that starts with "http"', () => { @@ -607,6 +632,10 @@ class MockSwUpdatesService { updateActivated = new Subject(); } +class MockScrollService { + removeStoredScrollPosition() { } +} + class TestGaService { locationChanged = jasmine.createSpy('locationChanged'); } diff --git a/aio/src/app/shared/location.service.ts b/aio/src/app/shared/location.service.ts index a200774e58..ec13fa8a6e 100644 --- a/aio/src/app/shared/location.service.ts +++ b/aio/src/app/shared/location.service.ts @@ -6,6 +6,7 @@ import { map, tap } from 'rxjs/operators'; import { GaService } from 'app/shared/ga.service'; import { SwUpdatesService } from 'app/sw-updates/sw-updates.service'; +import { ScrollService } from './scroll.service'; @Injectable() export class LocationService { @@ -25,6 +26,7 @@ export class LocationService { constructor( private gaService: GaService, private location: Location, + private scrollService: ScrollService, private platformLocation: PlatformLocation, swUpdates: SwUpdatesService) { @@ -41,9 +43,13 @@ export class LocationService { go(url: string|null|undefined) { if (!url) { return; } url = this.stripSlashes(url); - if (/^http/.test(url) || this.swUpdateActivated) { + if (/^http/.test(url)) { // Has http protocol so leave the site - // (or do a "full page navigation" if a ServiceWorker update has been activated) + this.goExternal(url); + } else if (this.swUpdateActivated) { + // (Do a "full page navigation" if a ServiceWorker update has been activated) + // We need to remove stored Position in order to be sure to scroll to the Top position + this.scrollService.removeStoredScrollPosition(); this.goExternal(url); } else { this.location.go(url);