diff --git a/aio/src/app/app.component.spec.ts b/aio/src/app/app.component.spec.ts
index 144de9ab68..c58d53dcf2 100644
--- a/aio/src/app/app.component.spec.ts
+++ b/aio/src/app/app.component.spec.ts
@@ -265,19 +265,49 @@ describe('AppComponent', () => {
});
});
- describe('autoScrolling', () => {
- it('should AutoScrollService.scroll when the url changes', () => {
- const scrollService: AutoScrollService = fixture.debugElement.injector.get(AutoScrollService);
- spyOn(scrollService, 'scroll');
- locationService.go('some/url#fragment');
- expect(scrollService.scroll).toHaveBeenCalledWith();
+ describe('autoScrolling with AutoScrollService', () => {
+ let scrollService: AutoScrollService;
+ let scrollSpy: jasmine.Spy;
+
+ beforeEach(() => {
+ scrollService = fixture.debugElement.injector.get(AutoScrollService);
+ scrollSpy = spyOn(scrollService, 'scroll');
});
- it('should be called when a document has been rendered', () => {
- const scrollService: AutoScrollService = fixture.debugElement.injector.get(AutoScrollService);
- spyOn(scrollService, 'scroll');
+ it('should not scroll immediately when the docId (path) changes', () => {
+ locationService.go('guide/pipes');
+ // deliberately not calling `fixture.detectChanges` because don't want `onDocRendered`
+ expect(scrollSpy).not.toHaveBeenCalled();
+ });
+
+ it('should scroll when just the hash changes (# alone)', () => {
+ locationService.go('guide/pipes');
+ locationService.go('guide/pipes#somewhere');
+ expect(scrollSpy).toHaveBeenCalled();
+ });
+
+ it('should scroll when just the hash changes (/#)', () => {
+ locationService.go('guide/pipes');
+ locationService.go('guide/pipes/#somewhere');
+ expect(scrollSpy).toHaveBeenCalled();
+ });
+
+ it('should scroll again when nav to the same hash twice in succession', () => {
+ locationService.go('guide/pipes');
+ locationService.go('guide/pipes#somewhere');
+ locationService.go('guide/pipes#somewhere');
+ expect(scrollSpy.calls.count()).toBe(2);
+ });
+
+ it('should scroll when call onDocRendered directly', () => {
component.onDocRendered();
- expect(scrollService.scroll).toHaveBeenCalledWith();
+ expect(scrollSpy).toHaveBeenCalled();
+ });
+
+ it('should scroll (via onDocRendered) when finish navigating to a new doc', () => {
+ locationService.go('guide/pipes');
+ fixture.detectChanges(); // triggers the event that calls onDocRendered
+ expect(scrollSpy).toHaveBeenCalled();
});
});
@@ -464,7 +494,7 @@ class TestHttp {
const match = /content\/docs\/(.+)\.json/.exec(url);
const id = match[1];
const title = id.split('/').pop().replace(/^([a-z])/, (_, letter) => letter.toUpperCase());
- const contents = `
${title} Doc
`;
+ const contents = `${title} Doc
Some heading
`;
data = { id, title, contents };
if (id === 'no-title') {
data.title = '';
diff --git a/aio/src/app/app.component.ts b/aio/src/app/app.component.ts
index 71669c39aa..886177ae28 100644
--- a/aio/src/app/app.component.ts
+++ b/aio/src/app/app.component.ts
@@ -21,6 +21,7 @@ const sideNavView = 'SideNav';
export class AppComponent implements OnInit {
currentNode: CurrentNode;
+ currentPath: string;
dtOn = false;
pageId: string;
currentDocument: DocumentContents;
@@ -76,8 +77,15 @@ export class AppComponent implements OnInit {
this.setPageId(doc.id);
});
- // scroll even if only the hash fragment changed
- this.locationService.currentUrl.subscribe(() => this.autoScroll());
+ this.locationService.currentPath.subscribe(path => {
+ if (this.currentPath && path === this.currentPath) {
+ // scroll only if on same page (most likely a change to the hash)
+ this.autoScroll();
+ } else {
+ // don't scroll; leave that to `onDocRendered`
+ this.currentPath = path;
+ }
+ });
this.navigationService.currentNode.subscribe(currentNode => {
this.currentNode = currentNode;
@@ -100,7 +108,7 @@ export class AppComponent implements OnInit {
this.swUpdateNotifications.enable();
}
- // Scroll to the anchor in the hash fragment.
+ // Scroll to the anchor in the hash fragment or top of doc.
autoScroll() {
this.autoScrollService.scroll();
}
diff --git a/aio/src/app/shared/location.service.ts b/aio/src/app/shared/location.service.ts
index 8e41d54a4e..50c3668996 100644
--- a/aio/src/app/shared/location.service.ts
+++ b/aio/src/app/shared/location.service.ts
@@ -17,8 +17,9 @@ export class LocationService {
.map(url => this.stripSlashes(url))
.do(url => this.gaService.locationChanged(url))
.publishReplay(1);
+
currentPath = this.currentUrl
- .map(url => url.match(/[^?#]*/)[0]); // strip off query and hash
+ .map(url => url.match(/[^?#]*/)[0]); // strip query and hash
constructor(
private gaService: GaService,
diff --git a/aio/src/testing/location.service.ts b/aio/src/testing/location.service.ts
index ae1df87038..ad8bb2d329 100644
--- a/aio/src/testing/location.service.ts
+++ b/aio/src/testing/location.service.ts
@@ -2,7 +2,8 @@ import { BehaviorSubject } from 'rxjs/BehaviorSubject';
export class MockLocationService {
urlSubject = new BehaviorSubject(this.initialUrl);
- currentUrl = this.urlSubject.asObservable();
+ currentUrl = this.urlSubject.asObservable().map(url => this.stripSlashes(url));
+ // strip off query and hash
currentPath = this.currentUrl.map(url => url.match(/[^?#]*/)[0]);
search = jasmine.createSpy('search').and.returnValue({});
setSearch = jasmine.createSpy('setSearch');
@@ -12,5 +13,9 @@ export class MockLocationService {
.and.returnValue(false); // prevent click from causing a browser navigation
constructor(private initialUrl) {}
+
+ private stripSlashes(url: string) {
+ return url.replace(/^\/+/, '').replace(/\/+(\?|#|$)/, '$1');
+ }
}