-
Notifications
You must be signed in to change notification settings - Fork 395
Add doc-viewer component #17
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -2,3 +2,4 @@ | |||
<img src="../../../assets/img/components/{{component.src}}.svg" [alt]="component.name" /> | |||
{{component.name}} | |||
</div> | |||
<doc-viewer documentUrl="/assets/documents/README.html"></doc-viewer> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I think we usually use document-url
and use @Input('document-url')
in typeScript file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's what I had originally, but @jelbourn suggested I change it to this
error => { | ||
console.log(error); | ||
this._elementRef.nativeElement.innerText = | ||
`Failed to load document: ${url}. Error: ${error}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very nit: indent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@jelbourn @tinayuangao is src/app/shared going to be the official home of our components? |
@mmalerba |
tests working (though I wasn't able to tests for the |
LGTM, just needs rebase |
No description provided.