Skip to content

Commit

Permalink
docs(docs-infra): set meta description per page (angular#51487)
Browse files Browse the repository at this point in the history 
aio currently uses the same `<meta name="Description">` content for every page. It seems like this might be causing a problem with search engine indexing such that different pages are being marked as duplicates of each other. It's unfortunately impossible to know whether this will actually fix the issue without it going live.

PR Close angular#51487
  • Loading branch information
@jelbourn @josephperrott
jelbourn authored and josephperrott committed Aug 24, 2023
1 parent 26ad6d5 commit 6d29ab7
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 33 deletions.
4 changes: 2 additions & 2 deletions aio/content/marketing/index.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ <h1 class="hero-headline no-title no-toc no-anchor" aria-label="Angular - Delive
</h1>

<div class="cta-button-container">
<p class="cta-link">
<div class="cta-link">
<a class="button hero-cta no-print" href="quick-start">Try Angular</a>
</p>
</div>
</div>

</div>
Expand Down
87 changes: 60 additions & 27 deletions aio/src/app/layout/doc-viewer/doc-viewer.component.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import { Logger } from 'app/shared/logger.service';
import { fromOuterHTML } from 'app/shared/security';
import { TocService } from 'app/shared/toc.service';
import {
MockTitle, MockTocService, ObservableWithSubscriptionSpies,
TestDocViewerComponent, TestModule, TestParentComponent, MockElementsLoader
MockTitle, MockTocService, ObservableWithSubscriptionSpies,
TestDocViewerComponent, TestModule, TestParentComponent, MockElementsLoader, MockMeta
} from 'testing/doc-viewer-utils';
import { MockLogger } from 'testing/logger.service';
import { DocViewerComponent, NO_ANIMATIONS } from './doc-viewer.component';
Expand Down Expand Up @@ -110,7 +110,7 @@ describe('DocViewerComponent', () => {
});
});

describe('#prepareTitleAndToc()', () => {
describe('#prepareMetadataAndToc()', () => {
const EMPTY_DOC = '';
const DOC_WITHOUT_H1 = 'Some content';
const DOC_WITH_H1 = '<h1>Features</h1>Some content';
Expand All @@ -124,12 +124,12 @@ describe('DocViewerComponent', () => {
let targetEl: HTMLElement;

const getTocEl = () => targetEl.querySelector('aio-toc');
const doPrepareTitleAndToc = (contents: string, docId = '') => {
const prepareMetadataAndToc = (contents: string, docId = '') => {
targetEl.innerHTML = contents;
return docViewer.prepareTitleAndToc(targetEl, docId);
return docViewer.prepareMetadataAndToc(targetEl, docId);
};
const doAddTitleAndToc = (contents: string, docId = '') => {
const addTitleAndToc = doPrepareTitleAndToc(contents, docId);
const addTitleAndToc = prepareMetadataAndToc(contents, docId);
return addTitleAndToc();
};

Expand All @@ -144,7 +144,7 @@ describe('DocViewerComponent', () => {
afterEach(() => document.body.removeChild(targetEl));

it('should return a function for doing the actual work', () => {
const addTitleAndToc = doPrepareTitleAndToc(DOC_WITH_H1);
const addTitleAndToc = prepareMetadataAndToc(DOC_WITH_H1);

expect(getTocEl()).toBeTruthy();
expect(titleService.setTitle).not.toHaveBeenCalled();
Expand All @@ -158,6 +158,39 @@ describe('DocViewerComponent', () => {
expect(tocService.genToc).toHaveBeenCalledTimes(1);
});

it('should set the meta description to the first paragraph text', () => {
const contentWithParagraphTags = `
<h1>NgIf</h1>
<p>NgIf is a directive</p>
<p>It hides and shows content</p>`;

const meta = TestBed.inject(Meta) as unknown as MockMeta;
const metaElement = document.createElement('meta');
metaElement.setAttribute('name', 'Description');
(meta as any).getTag = jasmine.createSpy('Meta#getTag').and.returnValue(metaElement);

doAddTitleAndToc(contentWithParagraphTags);

expect(metaElement?.getAttribute('content')).toBe('NgIf is a directive');
});

it('should set the meta description when there is an empty paragraph', () => {
const contentWithParagraphTags = `
<h1>NgIf</h1>
<p><a></a></p>
<p>NgIf is a directive</p>
<p>It hides and shows content</p>`;

const meta = TestBed.inject(Meta) as unknown as MockMeta;
const metaElement = document.createElement('meta');
metaElement.setAttribute('name', 'Description');
(meta as any).getTag = jasmine.createSpy('Meta#getTag').and.returnValue(metaElement);

doAddTitleAndToc(contentWithParagraphTags);

expect(metaElement?.getAttribute('content')).toBe('NgIf is a directive');
});

describe('(title)', () => {
it('should set the title if there is an `<h1>` heading', () => {
doAddTitleAndToc(DOC_WITH_H1);
Expand Down Expand Up @@ -216,39 +249,39 @@ describe('DocViewerComponent', () => {
describe('(ToC)', () => {
describe('needed', () => {
it('should add an embedded ToC element if there is an `<h1>` heading', () => {
doPrepareTitleAndToc(DOC_WITH_H1);
prepareMetadataAndToc(DOC_WITH_H1);
const tocEl = getTocEl();

expect(tocEl).toBeTruthy();
expect(tocEl?.classList.contains('embedded')).toBe(true);
});

it('should not add a second ToC element if there a hard coded one in place', () => {
doPrepareTitleAndToc(DOC_WITH_EMBEDDED_TOC);
prepareMetadataAndToc(DOC_WITH_EMBEDDED_TOC);
expect(targetEl.querySelectorAll('aio-toc').length).toEqual(1);
});
});


describe('not needed', () => {
it('should not add a ToC element if there is a `.no-toc` `<h1>` heading', () => {
doPrepareTitleAndToc(DOC_WITH_NO_TOC_H1);
prepareMetadataAndToc(DOC_WITH_NO_TOC_H1);
expect(getTocEl()).toBeFalsy();
});

it('should not add a ToC element if there is no `<h1>` heading', () => {
doPrepareTitleAndToc(DOC_WITHOUT_H1);
prepareMetadataAndToc(DOC_WITHOUT_H1);
expect(getTocEl()).toBeFalsy();

doPrepareTitleAndToc(EMPTY_DOC);
prepareMetadataAndToc(EMPTY_DOC);
expect(getTocEl()).toBeFalsy();
});

it('should remove ToC a hard coded one', () => {
doPrepareTitleAndToc(DOC_WITH_EMBEDDED_TOC_WITHOUT_H1);
prepareMetadataAndToc(DOC_WITH_EMBEDDED_TOC_WITHOUT_H1);
expect(getTocEl()).toBeFalsy();

doPrepareTitleAndToc(DOC_WITH_EMBEDDED_TOC_WITH_NO_TOC_H1);
prepareMetadataAndToc(DOC_WITH_EMBEDDED_TOC_WITH_NO_TOC_H1);
expect(getTocEl()).toBeFalsy();
});
});
Expand Down Expand Up @@ -297,7 +330,7 @@ describe('DocViewerComponent', () => {
});

describe('#render()', () => {
let prepareTitleAndTocSpy: jasmine.Spy;
let prepareMetadataAndTocSpy: jasmine.Spy;
let swapViewsSpy: jasmine.Spy;
let loadElementsSpy: jasmine.Spy;

Expand All @@ -307,7 +340,7 @@ describe('DocViewerComponent', () => {
beforeEach(() => {
const elementsLoader = TestBed.inject(ElementsLoader) as Partial<ElementsLoader> as MockElementsLoader;
loadElementsSpy = elementsLoader.loadContainedCustomElements.and.callFake(() => of(undefined));
prepareTitleAndTocSpy = spyOn(docViewer, 'prepareTitleAndToc');
prepareMetadataAndTocSpy = spyOn(docViewer, 'prepareMetadataAndToc');
swapViewsSpy = spyOn(docViewer, 'swapViews').and.callFake(() => of(undefined));
});

Expand Down Expand Up @@ -341,20 +374,20 @@ describe('DocViewerComponent', () => {
});

it('should prepare the title and ToC (before embedding components)', async () => {
prepareTitleAndTocSpy.and.callFake((targetEl: HTMLElement, docId: string) => {
prepareMetadataAndTocSpy.and.callFake((targetEl: HTMLElement, docId: string) => {
expect(targetEl.innerHTML).toBe('Some content');
expect(docId).toBe('foo');
});

await doRender(htmlEscape('Some content'), 'foo');

expect(prepareTitleAndTocSpy).toHaveBeenCalledTimes(1);
expect(prepareTitleAndTocSpy).toHaveBeenCalledBefore(loadElementsSpy);
expect(prepareMetadataAndTocSpy).toHaveBeenCalledTimes(1);
expect(prepareMetadataAndTocSpy).toHaveBeenCalledBefore(loadElementsSpy);
});

it('should set the title and ToC (after the content has been set)', async () => {
const addTitleAndTocSpy = jasmine.createSpy('addTitleAndToc');
prepareTitleAndTocSpy.and.returnValue(addTitleAndTocSpy);
prepareMetadataAndTocSpy.and.returnValue(addTitleAndTocSpy);

addTitleAndTocSpy.and.callFake(() => expect(docViewerEl.textContent).toBe('Foo content'));
await doRender(htmlEscape('Foo content'));
Expand Down Expand Up @@ -433,7 +466,7 @@ describe('DocViewerComponent', () => {

it('should pass the `addTitleAndToc` callback', async () => {
const addTitleAndTocSpy = jasmine.createSpy('addTitleAndToc');
prepareTitleAndTocSpy.and.returnValue(addTitleAndTocSpy);
prepareMetadataAndTocSpy.and.returnValue(addTitleAndTocSpy);

const el = document.createElement('div');
await doRender(fromOuterHTML(el));
Expand Down Expand Up @@ -465,16 +498,16 @@ describe('DocViewerComponent', () => {
logger = TestBed.inject(Logger) as unknown as MockLogger;
});

it('when `prepareTitleAndTocSpy()` fails', async () => {
const error = Error('Typical `prepareTitleAndToc()` error');
prepareTitleAndTocSpy.and.callFake(() => {
it('when `prepareMetadataAndTocSpy()` fails', async () => {
const error = Error('Typical `prepareMetadataAndToc()` error');
prepareMetadataAndTocSpy.and.callFake(() => {
expect(docViewer.nextViewContainer.innerHTML).not.toBe('');
throw error;
});

await doRender(htmlEscape('Some content'), 'foo');

expect(prepareTitleAndTocSpy).toHaveBeenCalledTimes(1);
expect(prepareMetadataAndTocSpy).toHaveBeenCalledTimes(1);
expect(swapViewsSpy).not.toHaveBeenCalled();
expect(docViewer.nextViewContainer.innerHTML).toBe('');
expect(logger.output.error).toEqual([
Expand All @@ -493,7 +526,7 @@ describe('DocViewerComponent', () => {

await doRender(htmlEscape('Some content'), 'bar');

expect(prepareTitleAndTocSpy).toHaveBeenCalledTimes(1);
expect(prepareMetadataAndTocSpy).toHaveBeenCalledTimes(1);
expect(loadElementsSpy).toHaveBeenCalledTimes(1);
expect(swapViewsSpy).not.toHaveBeenCalled();
expect(docViewer.nextViewContainer.innerHTML).toBe('');
Expand All @@ -512,7 +545,7 @@ describe('DocViewerComponent', () => {

await doRender(htmlEscape('Some content'), 'qux');

expect(prepareTitleAndTocSpy).toHaveBeenCalledTimes(1);
expect(prepareMetadataAndTocSpy).toHaveBeenCalledTimes(1);
expect(swapViewsSpy).toHaveBeenCalledTimes(1);
expect(docViewer.nextViewContainer.innerHTML).toBe('');
expect(logger.output.error).toEqual([
Expand Down
21 changes: 18 additions & 3 deletions aio/src/app/layout/doc-viewer/doc-viewer.component.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,10 @@ export class DocViewerComponent implements OnDestroy {
}

/**
* Prepare for setting the window title and ToC.
* Prepare for setting the page metadata (title + description) and table-of-contents.
* Return a function to actually set them.
*/
protected prepareTitleAndToc(targetElem: HTMLElement, docId: string): () => void {
prepareMetadataAndToc(targetElem: HTMLElement, docId: string): () => void {
const titleEl = targetElem.querySelector('h1');
const needsTitle = !!titleEl && !/no-?title/i.test(titleEl.className);
const needsToc = !!titleEl && !/no-?toc/i.test(titleEl.className);
Expand Down Expand Up @@ -123,6 +123,21 @@ export class DocViewerComponent implements OnDestroy {
}

this.titleService.setTitle(title ? `Angular - ${title}` : 'Angular');

// Set the page description via the existing `<meta name="Description">` tag.
// Updating the page description provides information for internet search indexers.
// Pull the next from the first paragraph element that has text since the docs
// processing pipeline sometimes results in pages where the first paragraph tag
// is empty.
const paragraphs = targetElem.querySelectorAll('p');
const firstParagraph =
Array.from(paragraphs).find(p => p.textContent?.trim());

const firstParagraphText = firstParagraph?.textContent?.trim();
if (firstParagraphText) {
const descriptionMeta = this.metaService.getTag('name="Description"');
descriptionMeta?.setAttribute('content', firstParagraphText);
}
};
}

Expand All @@ -144,7 +159,7 @@ export class DocViewerComponent implements OnDestroy {
this.nextViewContainer.innerHTML = unwrapHtml(doc.contents) as string;
}
}),
tap(() => addTitleAndToc = this.prepareTitleAndToc(this.nextViewContainer, doc.id)),
tap(() => addTitleAndToc = this.prepareMetadataAndToc(this.nextViewContainer, doc.id)),
switchMap(() => this.elementsLoader.loadContainedCustomElements(this.nextViewContainer)),
tap(() => this.docReady.emit()),
switchMap(() => this.swapViews(addTitleAndToc)),
Expand Down
2 changes: 1 addition & 1 deletion aio/src/testing/doc-viewer-utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export class TestDocViewerComponent extends DocViewerComponent {
override nextViewContainer: HTMLElement;

// Only used for type-casting; the actual implementation is irrelevant.
override prepareTitleAndToc(_targetElem: HTMLElement, _docId: string): () => void {
override prepareMetadataAndToc(_targetElem: HTMLElement, _docId: string): () => void {
return null as any;
}

Expand Down

0 comments on commit 6d29ab7

Please sign in to comment.