honeymoose
/
angular-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

refactor(aio): rename DocumentContents `url` to `id` and tidy up (#16139) 

Dgeni is now providing the `id` for all the documents to be viewed. So we
no longer need to add this to the DocumentContents object.

There are some notable changes in the refactoring:

`DocumentService`:

* The id of the document to render is now obtained from `LocationService.path()`.
* The `getFileNotFoundDoc` and `getErrorDoc` methods have been extracted from
the `fetchDocument` method.

`AppComponent`:

* the `pageId` is now computed in a separate `setPageId` method.

`AppComponen` spec file:

* The `TestHttp` has had the hard coded documents removed and replaced with
a function that will generate docs as needed.
This commit is contained in:
Peter Bacon Darwin 2017-04-20 14:16:36 +01:00 committed by Miško Hevery
parent d0dcabd700
commit 5e6a3ff6a7
6 changed files with 78 additions and 93 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

56
aio/src/app/app.component.spec.ts
View File

@ -207,7 +207,7 @@ describe('AppComponent', () => {
describe('pageId', () => {
it('should set the id of the doc viewer container based on the current url', () => {
it('should set the id of the doc viewer container based on the current doc', () => {
const container = fixture.debugElement.query(By.css('section.sidenav-content'));
locationService.go('guide/pipes');
@ -226,7 +226,7 @@ describe('AppComponent', () => {
expect(container.properties['id']).toEqual('home');
});
it('should not be affected by changes to the query or hash', () => {
it('should not be affected by changes to the query', () => {
const container = fixture.debugElement.query(By.css('section.sidenav-content'));
locationService.go('guide/pipes');
@ -236,11 +236,6 @@ describe('AppComponent', () => {
fixture.detectChanges();
expect(component.pageId).toEqual('guide-other');
expect(container.properties['id']).toEqual('guide-other');
locationService.go('guide/http#anchor-1');
fixture.detectChanges();
expect(component.pageId).toEqual('guide-http');
expect(container.properties['id']).toEqual('guide-http');
});
});
@ -261,7 +256,7 @@ describe('AppComponent', () => {
it('should display a marketing page', () => {
locationService.go('features');
fixture.detectChanges();
expect(docViewer.innerText).toMatch(/Test Doc/i);
expect(docViewer.innerText).toMatch(/Features Doc/i);
});
it('should update the document title', () => {
@ -275,7 +270,7 @@ describe('AppComponent', () => {
it('should update the document title, with a default value if the document has no title', () => {
const titleService = TestBed.get(Title);
spyOn(titleService, 'setTitle');
locationService.go('file-not-found');
locationService.go('no-title');
fixture.detectChanges();
expect(titleService.setTitle).toHaveBeenCalledWith('Angular');
});
@ -431,35 +426,20 @@ class TestHttp {
}
};
apiDoc = {
"title": "API",
"contents": "<h1>API Doc</h1>"
};
pipesDoc = {
"title": "Pipes",
"contents": "<h1>Pipes Doc</h1>"
};
testDoc = {
"title": "Test",
"contents": "<h1>Test Doc</h1>"
};
fileNotFoundDoc = {
"title": "",
"contents": "Page not found"
};
// get = jasmine.createSpy('get').and.callFake((url: string) => { ... });
get(url: string) {
const json =
/navigation.json/.test(url) ? this.navJson :
/api/.test(url) ? this.apiDoc :
/pipes/.test(url) ? this.pipesDoc :
/file-not-found/.test(url) ? this.fileNotFoundDoc :
this.testDoc;
return of({ json: () => json });
let data;
if (/navigation\.json/.test(url)) {
data = this.navJson;
} else {
const match = /content\/docs\/(.+)\.json/.exec(url);
const id = match[1];
const title = id.split('/').pop().replace(/^([a-z])/, (_, letter) => letter.toUpperCase());
const contents = `<h1>${title} Doc</h1>`;
data = { id, title, contents };
if (id === 'no-title') {
data.title = '';
}
}
return of({ json: () => data });
}
}

14
aio/src/app/app.component.ts
View File

@ -72,12 +72,7 @@ export class AppComponent implements OnInit {
this.documentService.currentDocument.subscribe(doc => {
this.currentDocument = doc;
this.setDocumentTitle(doc.title);
this.pageId = this.currentDocument.url.replace('/', '-');
// Special case the home page
if (this.pageId === 'index') {
this.pageId = 'home';
}
this.setPageId(doc.id);
});
// scroll even if only the hash fragment changed
@ -151,11 +146,16 @@ export class AppComponent implements OnInit {
this.sidenav.toggle(value);
}
setDocumentTitle(title) {
setDocumentTitle(title: string) {
if (title.trim()) {
this.titleService.setTitle(`Angular - ${title}`);
} else {
this.titleService.setTitle('Angular');
}
}
setPageId(id: string) {
// Special case the home page
this.pageId = (id === 'index') ? 'home' : id.replace('/', '-');
}
}

5
aio/src/app/documents/document-contents.ts
View File

@ -1,5 +1,8 @@
export interface DocumentContents {
url: string;
/** The unique identifier for this document */
id: string;
/** The string to display in the browser tab when this document is being viewed */
title: string;
/** The HTML to display in the doc viewer */
contents: string;
}

9
aio/src/app/documents/document.service.spec.ts
View File

@ -86,15 +86,14 @@ describe('DocumentService', () => {
connections[0].mockError(new Response(new ResponseOptions({ status: 404, statusText: 'NOT FOUND'})) as any);
expect(connections.length).toEqual(2);
expect(connections[1].request.url).toEqual(CONTENT_URL_PREFIX + 'file-not-found.json');
const fileNotFoundDoc = { title: 'File Not Found' };
const fileNotFoundDoc = { id: 'file-not-found', title: 'Page Not Found', contents: '<h1>Page Not Found</h1>' };
connections[1].mockRespond(createResponse(fileNotFoundDoc));
expect(currentDocument).toEqual(jasmine.objectContaining(fileNotFoundDoc));
expect(currentDocument.url).toEqual('file-not-found');
expect(currentDocument).toEqual(fileNotFoundDoc);
});
it('should emit a hard-coded not-found document if the not-found document is not found on the server', () => {
let currentDocument: DocumentContents;
const notFoundDoc: DocumentContents = { title: 'Not Found', contents: 'Document not found', url: 'file-not-found' };
const notFoundDoc: DocumentContents = { title: 'Not Found', contents: 'Document not found', id: 'file-not-found' };
const nextDoc = { title: 'Next Doc', url: 'new/url' };
const { docService, backend, locationService } = getServices('file-not-found');
const connections = backend.connectionsArray;
@ -178,7 +177,7 @@ describe('DocumentService', () => {
});
it('should map the "folder" locations to the correct document request', () => {
const { docService, backend, locationService} = getServices('guide/');
const { docService, backend, locationService} = getServices('guide');
docService.currentDocument.subscribe();
expect(backend.connectionsArray[0].request.url).toEqual(CONTENT_URL_PREFIX + 'guide.json');

65
aio/src/app/documents/document.service.ts
View File

@ -15,6 +15,7 @@ import { LocationService } from 'app/shared/location.service';
import { Logger } from 'app/shared/logger.service';
const FILE_NOT_FOUND_URL = 'file-not-found';
const FETCHING_ERROR_URL = 'fetching-error';
@Injectable()
export class DocumentService {
@ -28,50 +29,52 @@ export class DocumentService {
private http: Http,
location: LocationService) {
// Whenever the URL changes we try to get the appropriate doc
this.currentDocument = location.currentUrl.switchMap(url => this.getDocument(url));
this.currentDocument = location.currentUrl.switchMap(() => this.getDocument(location.path()));
}
private getDocument(url: string) {
this.logger.log('getting document', url);
url = this.cleanUrl(url);
if ( !this.cache.has(url)) {
this.cache.set(url, this.fetchDocument(url));
const id = url || 'index';
this.logger.log('getting document', id);
if ( !this.cache.has(id)) {
this.cache.set(id, this.fetchDocument(id));
}
return this.cache.get(url);
return this.cache.get(id);
}
private fetchDocument(url: string) {
this.logger.log('fetching document from', url);
private fetchDocument(id: string): Observable<DocumentContents> {
const requestPath = `content/docs/${id}.json`;
this.logger.log('fetching document from', requestPath);
const subject = new AsyncSubject();
this.http
.get(`content/docs/${url}.json`)
// Add the document's url to the DocumentContents provided to the rest of the app
.map(res => Object.assign(res.json(), { url }) as DocumentContents)
.get(requestPath)
.map(response => response.json())
.catch((error: Response) => {
if (error.status === 404) {
if (url !== FILE_NOT_FOUND_URL) {
this.logger.error(`Document file not found at '${url}'`);
// using `getDocument` means that we can fetch the 404 doc contents from the server and cache it
return this.getDocument(FILE_NOT_FOUND_URL);
} else {
return of({ title: 'Not Found', contents: 'Document not found', url: FILE_NOT_FOUND_URL });
}
} else {
this.logger.error('Error fetching document', error);
return Observable.of({ title: 'Error fetching document', contents: 'Sorry we were not able to fetch that document.', url});
}
return error.status === 404 ? this.getFileNotFoundDoc(id) : this.getErrorDoc(error);
})
.subscribe(subject);
return subject.asObservable();
}
private cleanUrl(url: string) {
url = url.match(/[^#?]*/)[0]; // strip off fragment and query
url = url.replace(/\/$/, ''); // strip off trailing slash
if (url === '') {
// deal with root url
url = 'index';
}
return url;
private getFileNotFoundDoc(url: string): Observable<DocumentContents> {
if (url !== FILE_NOT_FOUND_URL) {
this.logger.error(`Document file not found at '${url}'`);
// using `getDocument` means that we can fetch the 404 doc contents from the server and cache it
return this.getDocument(FILE_NOT_FOUND_URL);
} else {
return of({
title: 'Not Found',
contents: 'Document not found',
id: FILE_NOT_FOUND_URL
});
}
}
private getErrorDoc(error: Response): Observable<DocumentContents> {
this.logger.error('Error fetching document', error);
return Observable.of({
title: 'Error fetching document',
contents: 'Sorry we were not able to fetch that document.',
id: FETCHING_ERROR_URL
});
}
}

24
aio/src/app/layout/doc-viewer/doc-viewer.component.spec.ts
View File

@ -122,23 +122,23 @@ describe('DocViewerComponent', () => {
});
it(('should display nothing when set currentDoc has no content'), () => {
component.currentDoc = { title: 'fake title', contents: '', url: 'a/b' };
component.currentDoc = { title: 'fake title', contents: '', id: 'a/b' };
fixture.detectChanges();
expect(docViewerEl.innerHTML).toBe('');
});
it(('should display simple static content doc'), () => {
const contents = '<p>Howdy, doc viewer</p>';
component.currentDoc = { title: 'fake title', contents, url: 'a/b' };
component.currentDoc = { title: 'fake title', contents, id: 'a/b' };
fixture.detectChanges();
expect(docViewerEl.innerHTML).toEqual(contents);
});
it(('should display nothing after reset static content doc'), () => {
const contents = '<p>Howdy, doc viewer</p>';
component.currentDoc = { title: 'fake title', contents, url: 'a/b' };
component.currentDoc = { title: 'fake title', contents, id: 'a/b' };
fixture.detectChanges();
component.currentDoc = { title: 'fake title', contents: '', url: 'a/c' };
component.currentDoc = { title: 'fake title', contents: '', id: 'a/c' };
fixture.detectChanges();
expect(docViewerEl.innerHTML).toEqual('');
});
@ -149,7 +149,7 @@ describe('DocViewerComponent', () => {
<p><aio-foo></aio-foo></p>
<p>Below Foo</p>
`;
component.currentDoc = { title: 'fake title', contents, url: 'a/b' };
component.currentDoc = { title: 'fake title', contents, id: 'a/b' };
fixture.detectChanges();
const fooHtml = docViewerEl.querySelector('aio-foo').innerHTML;
expect(fooHtml).toContain('Foo Component');
@ -165,7 +165,7 @@ describe('DocViewerComponent', () => {
</div>
<p>Below Foo</p>
`;
component.currentDoc = { title: 'fake title', contents, url: 'a/b' };
component.currentDoc = { title: 'fake title', contents, id: 'a/b' };
fixture.detectChanges();
const foos = docViewerEl.querySelectorAll('aio-foo');
expect(foos.length).toBe(2);
@ -177,7 +177,7 @@ describe('DocViewerComponent', () => {
<aio-bar></aio-bar>
<p>Below Bar</p>
`;
component.currentDoc = { title: 'fake title', contents, url: 'a/b' };
component.currentDoc = { title: 'fake title', contents, id: 'a/b' };
fixture.detectChanges();
const barHtml = docViewerEl.querySelector('aio-bar').innerHTML;
expect(barHtml).toContain('Bar Component');
@ -189,7 +189,7 @@ describe('DocViewerComponent', () => {
<aio-bar>###bar content###</aio-bar>
<p>Below Bar</p>
`;
component.currentDoc = { title: 'fake title', contents, url: 'a/b' };
component.currentDoc = { title: 'fake title', contents, id: 'a/b' };
// necessary to trigger projection within ngOnInit
fixture.detectChanges();
@ -207,7 +207,7 @@ describe('DocViewerComponent', () => {
<p><aio-foo></aio-foo></p>
<p>Bottom</p>
`;
component.currentDoc = { title: 'fake title', contents, url: 'a/b' };
component.currentDoc = { title: 'fake title', contents, id: 'a/b' };
// necessary to trigger Bar's projection within ngOnInit
fixture.detectChanges();
@ -230,7 +230,7 @@ describe('DocViewerComponent', () => {
<p><aio-foo></aio-foo><p>
<p>Bottom</p>
`;
component.currentDoc = { title: 'fake title', contents, url: 'a/b' };
component.currentDoc = { title: 'fake title', contents, id: 'a/b' };
// necessary to trigger Bar's projection within ngOnInit
fixture.detectChanges();
@ -254,7 +254,7 @@ describe('DocViewerComponent', () => {
<p><aio-foo></aio-foo></p>
<p>Bottom</p>
`;
component.currentDoc = { title: 'fake title', contents, url: 'a/b' };
component.currentDoc = { title: 'fake title', contents, id: 'a/b' };
// necessary to trigger Bar's projection within ngOnInit
fixture.detectChanges();
@ -282,7 +282,7 @@ describe('DocViewerComponent', () => {
<p><aio-baz>---More baz--</aio-baz></p>
<p>Bottom</p>
`;
component.currentDoc = { title: 'fake title', contents, url: 'a/b' };
component.currentDoc = { title: 'fake title', contents, id: 'a/b' };
// necessary to trigger Bar's projection within ngOnInit
fixture.detectChanges();