From 7f657ecd6554461277452a41e13e8b7d0c41dec2 Mon Sep 17 00:00:00 2001 From: Will Howell Date: Fri, 3 Nov 2017 14:42:59 -0400 Subject: [PATCH 1/9] Have link no longer maintain its own 'active' state. Handle that in the toc. --- .../table-of-contents/table-of-contents.html | 2 +- .../table-of-contents.spec.ts | 1 - .../table-of-contents/table-of-contents.ts | 18 +++++++----------- 3 files changed, 8 insertions(+), 13 deletions(-) diff --git a/src/app/shared/table-of-contents/table-of-contents.html b/src/app/shared/table-of-contents/table-of-contents.html index 2b24b07ea..b61e76299 100644 --- a/src/app/shared/table-of-contents/table-of-contents.html +++ b/src/app/shared/table-of-contents/table-of-contents.html @@ -4,7 +4,7 @@ + [class.docs-active]="_activeLinkIndex === i"> {{link.name}} diff --git a/src/app/shared/table-of-contents/table-of-contents.spec.ts b/src/app/shared/table-of-contents/table-of-contents.spec.ts index 40b4de43e..720abfff1 100644 --- a/src/app/shared/table-of-contents/table-of-contents.spec.ts +++ b/src/app/shared/table-of-contents/table-of-contents.spec.ts @@ -45,7 +45,6 @@ describe('TableOfContents', () => { id: 'test', name: 'test', top: 0, - active: false } ]; diff --git a/src/app/shared/table-of-contents/table-of-contents.ts b/src/app/shared/table-of-contents/table-of-contents.ts index 9735aeed2..0f90532c1 100644 --- a/src/app/shared/table-of-contents/table-of-contents.ts +++ b/src/app/shared/table-of-contents/table-of-contents.ts @@ -14,9 +14,6 @@ interface Link { /* header type h3/h4 */ type: string; - /* If the anchor is in view of the page */ - active: boolean; - /* name of the anchor */ name: string; @@ -27,7 +24,7 @@ interface Link { @Component({ selector: 'table-of-contents', styleUrls: ['./table-of-contents.scss'], - templateUrl: './table-of-contents.html' + templateUrl: './table-of-contents.html', }) export class TableOfContents implements OnInit { @@ -35,6 +32,7 @@ export class TableOfContents implements OnInit { @Input() container: string; @Input() headerSelectors = '.docs-markdown-h3,.docs-markdown-h4'; + _activeLinkIndex: number; _rootUrl: string; private _scrollContainer: any; private _destroyed = new Subject(); @@ -103,7 +101,7 @@ export class TableOfContents implements OnInit { } private createLinks(): Link[] { - const links = []; + const links: Link[] = []; const headers = Array.from(this._document.querySelectorAll(this.headerSelectors)) as HTMLElement[]; @@ -114,10 +112,9 @@ export class TableOfContents implements OnInit { const {top} = header.getBoundingClientRect(); links.push({ name, + top, type: header.tagName.toLowerCase(), - top: top, - id: header.id, - active: false + id: header.id }); } } @@ -126,9 +123,8 @@ export class TableOfContents implements OnInit { } private onScroll(): void { - for (let i = 0; i < this.links.length; i++) { - this.links[i].active = this.isLinkActive(this.links[i], this.links[i + 1]); - } + this._activeLinkIndex = this.links + .findIndex((link, i) => this.isLinkActive(link, this.links[i + 1])); } private isLinkActive(currentLink: any, nextLink: any): boolean { From 7941daaef27cb9d80818e1c8ea801adaade820e8 Mon Sep 17 00:00:00 2001 From: Will Howell Date: Fri, 3 Nov 2017 14:49:28 -0400 Subject: [PATCH 2/9] Refactor a little logic for cleaner readability --- .../table-of-contents/table-of-contents.ts | 52 +++++++++---------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/src/app/shared/table-of-contents/table-of-contents.ts b/src/app/shared/table-of-contents/table-of-contents.ts index 0f90532c1..36dba4787 100644 --- a/src/app/shared/table-of-contents/table-of-contents.ts +++ b/src/app/shared/table-of-contents/table-of-contents.ts @@ -90,36 +90,22 @@ export class TableOfContents implements OnInit { } } - /** Gets the scroll offset of the scroll container */ - private getScrollOffset(): number { - const {top} = this._element.nativeElement.getBoundingClientRect(); - if (typeof this._scrollContainer.scrollTop !== 'undefined') { - return this._scrollContainer.scrollTop + top; - } else if (typeof this._scrollContainer.pageYOffset !== 'undefined') { - return this._scrollContainer.pageYOffset + top; - } - } - + /** Gets links generated from header selectors. */ private createLinks(): Link[] { - const links: Link[] = []; const headers = Array.from(this._document.querySelectorAll(this.headerSelectors)) as HTMLElement[]; - if (headers.length) { - for (const header of headers) { - // remove the 'link' icon name from the inner text - const name = header.innerText.trim().replace(/^link/, ''); - const {top} = header.getBoundingClientRect(); - links.push({ - name, - top, - type: header.tagName.toLowerCase(), - id: header.id - }); - } - } - - return links; + return headers.map(header => { + // remove the 'link' icon name from the inner text + const name = header.innerText.trim().replace(/^link/, ''); + const {top} = header.getBoundingClientRect(); + return { + name, + top, + type: header.tagName.toLowerCase(), + id: header.id + }; + }); } private onScroll(): void { @@ -134,4 +120,18 @@ export class TableOfContents implements OnInit { return scrollOffset >= currentLink.top && !(nextLink && nextLink.top < scrollOffset); } + /** Gets the scroll offset of the scroll container */ + private getScrollOffset(): number { + const {top} = this._element.nativeElement.getBoundingClientRect(); + if (this._scrollContainer.scrollTop != null) { + return this._scrollContainer.scrollTop + top; + } + + if (this._scrollContainer.pageYOffset != null) { + return this._scrollContainer.pageYOffset + top; + } + + return 0; + } + } From bfe48dc9ddf28e1ce23c5a72d19c8ab123e713e5 Mon Sep 17 00:00:00 2001 From: Will Howell Date: Fri, 3 Nov 2017 14:52:43 -0400 Subject: [PATCH 3/9] Combine duplicated logic --- src/app/shared/table-of-contents/table-of-contents.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/app/shared/table-of-contents/table-of-contents.ts b/src/app/shared/table-of-contents/table-of-contents.ts index 36dba4787..90dc269d8 100644 --- a/src/app/shared/table-of-contents/table-of-contents.ts +++ b/src/app/shared/table-of-contents/table-of-contents.ts @@ -55,11 +55,7 @@ export class TableOfContents implements OnInit { this._route.fragment.takeUntil(this._destroyed).subscribe(fragment => { this._urlFragment = fragment; - - const target = document.getElementById(this._urlFragment); - if (target) { - target.scrollIntoView(); - } + this.scrollFragmentIntoView(); }); } @@ -83,7 +79,11 @@ export class TableOfContents implements OnInit { updateScrollPosition(): void { this.links = this.createLinks(); + this.scrollFragmentIntoView(); + } + /** Find the target from the url fragment and scroll it into view. */ + private scrollFragmentIntoView(): void { const target = document.getElementById(this._urlFragment); if (target) { target.scrollIntoView(); From b36c688d171d0ad563f65e2b10ff6a5af9e2252f Mon Sep 17 00:00:00 2001 From: Will Howell Date: Fri, 3 Nov 2017 14:53:30 -0400 Subject: [PATCH 4/9] Rename method to be more descriptive --- src/app/shared/table-of-contents/table-of-contents.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/shared/table-of-contents/table-of-contents.ts b/src/app/shared/table-of-contents/table-of-contents.ts index 90dc269d8..ebfc9591f 100644 --- a/src/app/shared/table-of-contents/table-of-contents.ts +++ b/src/app/shared/table-of-contents/table-of-contents.ts @@ -69,7 +69,7 @@ export class TableOfContents implements OnInit { Observable.fromEvent(this._scrollContainer, 'scroll') .takeUntil(this._destroyed) .debounceTime(10) - .subscribe(() => this.onScroll()); + .subscribe(() => this.setActiveLink()); }); } @@ -108,7 +108,7 @@ export class TableOfContents implements OnInit { }); } - private onScroll(): void { + private setActiveLink(): void { this._activeLinkIndex = this.links .findIndex((link, i) => this.isLinkActive(link, this.links[i + 1])); } From 0cbecaf853dbdb4e11872f7f13b5e5d0f754c2f4 Mon Sep 17 00:00:00 2001 From: Will Howell Date: Fri, 3 Nov 2017 16:43:02 -0400 Subject: [PATCH 5/9] Use scroll dispatcher to respond to scroll events instead of manually subscribing to events on container - Remove links as an input - Calculate container from scrollable event instead of using input --- .../pages/component-viewer/component-api.html | 4 +- .../component-viewer/component-overview.html | 2 +- .../table-of-contents/table-of-contents.html | 4 +- .../table-of-contents.module.ts | 3 +- .../table-of-contents.spec.ts | 2 +- .../table-of-contents/table-of-contents.ts | 75 +++++++++++-------- 6 files changed, 49 insertions(+), 41 deletions(-) diff --git a/src/app/pages/component-viewer/component-api.html b/src/app/pages/component-viewer/component-api.html index 3de0bac87..b9ed3522e 100644 --- a/src/app/pages/component-viewer/component-api.html +++ b/src/app/pages/component-viewer/component-api.html @@ -5,6 +5,4 @@ documentUrl="/assets/documents/api/{{componentViewer.componentDocItem.packageName}}-{{componentViewer.componentDocItem.id}}.html" class="docs-component-view-text-content docs-component-api" (contentLoaded)="toc.updateScrollPosition()"> - + diff --git a/src/app/pages/component-viewer/component-overview.html b/src/app/pages/component-viewer/component-overview.html index e56dcb1b5..38fa41d60 100644 --- a/src/app/pages/component-viewer/component-overview.html +++ b/src/app/pages/component-viewer/component-overview.html @@ -6,4 +6,4 @@ class="docs-component-view-text-content docs-component-overview" (contentLoaded)="toc.updateScrollPosition()"> - + diff --git a/src/app/shared/table-of-contents/table-of-contents.html b/src/app/shared/table-of-contents/table-of-contents.html index b61e76299..2420723d3 100644 --- a/src/app/shared/table-of-contents/table-of-contents.html +++ b/src/app/shared/table-of-contents/table-of-contents.html @@ -1,8 +1,8 @@ -
+
Contents