Skip to content

Commit

Permalink
fix(docs-infra): exclude heading anchor icon text from ToC tooltips (a…
Browse files Browse the repository at this point in the history
…ngular#32418)

The Table of Contents (ToC) is auto-generated based on the content of
heading elements on the page. At the same time, anchor links are
auto-generated and added to each heading element. Note that the Material
Icons used for the anchor icon make use of ligatures, which means that
the icons are specified by using their textual name as text content of
the icon element. As a result, the name of the icon is included in the
parent element's `textContent`.

Previously, the `TocService` used to strip off these anchor elements
when generating the content of ToC items, but not when generating the
content of their tooltips. Thus, tooltips for ToC items would
confusingly include a `link` suffix (`link` is the textual name of the
icon used in heading anchor links).

This commit fixes this by deriving the tooltip content from the
transformed text content (which already has anchor links stripped off),
instead of from the original heading content.

PR Close angular#32418
  • Loading branch information
gkalpak authored and matsko committed Sep 5, 2019
1 parent 094538c commit 007282d
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 27 deletions.
3 changes: 1 addition & 2 deletions aio/src/app/shared/toc.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -299,12 +299,11 @@ describe('TocService', () => {
beforeEach(() => {
docId = 'fizz/buzz/';

// An almost-actual <h2> ... with extra whitespace
callGenToc(`
<h2 id="setup-to-develop-locally">
Setup to <a href="moo">develop</a> <i>locally</i>.
<a class="header-link" href="tutorial/toh-pt1#setup-to-develop-locally" aria-hidden="true">
<span class="icon icon-link"></span>
<span class="icon">icon-link</span>
</a>
</h2>
`, docId);
Expand Down
68 changes: 43 additions & 25 deletions aio/src/app/shared/toc.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,16 @@ export class TocService {

const headings = this.findTocHeadings(docElement);
const idMap = new Map<string, number>();
const tocList = headings.map(heading => ({
content: this.extractHeadingSafeHtml(heading),
href: `${docId}#${this.getId(heading, idMap)}`,
level: heading.tagName.toLowerCase(),
title: (heading.textContent || '').trim(),
}));
const tocList = headings.map(heading => {
const {title, content} = this.extractHeadingSafeHtml(heading);

return {
level: heading.tagName.toLowerCase(),
href: `${docId}#${this.getId(heading, idMap)}`,
title,
content,
};
});

this.tocList.next(tocList);

Expand All @@ -52,29 +56,35 @@ export class TocService {
this.tocList.next([]);
}

// This bad boy exists only to strip off the anchor link attached to a heading
// Transform the HTML content to be safe to use in the ToC:
// - Strip off the auto-generated heading anchor links).
// - Strip off any anchor links (but keep their content)
// - Mark the HTML as trusted to be used with `[innerHTML]`.
private extractHeadingSafeHtml(heading: HTMLHeadingElement) {
const div: HTMLDivElement = this.document.createElement('div');
div.innerHTML = heading.innerHTML;
const anchorLinks = Array.from(div.querySelectorAll('a'));
for (const anchorLink of anchorLinks) {
if (!anchorLink.classList.contains('header-link')) {
// this is an anchor that contains actual content that we want to keep
// move the contents of the anchor into its parent
const parent = anchorLink.parentNode!;
while (anchorLink.childNodes.length) {
parent.insertBefore(anchorLink.childNodes[0], anchorLink);
}
}
// now remove the anchor
if (anchorLink.parentNode !== null) {
// We cannot use ChildNode.remove() because of IE11
anchorLink.parentNode.removeChild(anchorLink);

// Remove any `.header-link` elements (along with their content).
div.querySelectorAll('.header-link').forEach(removeNode);

// Remove any remaining `a` elements (but keep their content).
div.querySelectorAll('a').forEach(anchorLink => {
// We want to keep the content of this anchor, so move it into its parent.
const parent = anchorLink.parentNode!;
while (anchorLink.childNodes.length) {
parent.insertBefore(anchorLink.childNodes[0], anchorLink);
}
}
// security: the document element which provides this heading content
// is always authored by the documentation team and is considered to be safe
return this.domSanitizer.bypassSecurityTrustHtml(div.innerHTML.trim());

// Now, remove the anchor.
removeNode(anchorLink);
});

return {
// Security: The document element which provides this heading content is always authored by
// the documentation team and is considered to be safe.
content: this.domSanitizer.bypassSecurityTrustHtml(div.innerHTML.trim()),
title: (div.textContent || '').trim(),
};
}

private findTocHeadings(docElement: Element): HTMLHeadingElement[] {
Expand Down Expand Up @@ -115,3 +125,11 @@ export class TocService {
}
}
}

// Helpers
function removeNode(node: Node): void {
if (node.parentNode !== null) {
// We cannot use `Node.remove()` because of IE11.
node.parentNode.removeChild(node);
}
}

0 comments on commit 007282d

Please sign in to comment.