-
Notifications
You must be signed in to change notification settings - Fork 395
Disambiguate between material and cdk docs #286
Conversation
] | ||
}; | ||
|
||
for (let category of DOCS[COMPONENTS]) { |
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.
You could probably combine these loops into one w/ something like this:
for (const type in DOCS) {
for (const cat of DOCS[type]) {
for (let doc of cat.items) {
doc.packageName = cat;
}
}
}
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.
The packageName
isn't the category; the categories are things like forms
, layout
, etc.
The type
also isn't the package name, it's the section name ("components" and "cdk").
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.
What I really want to do is generate this entire blob.
this._fragmentSubscription.unsubscribe(); | ||
this._routeSubscription && this._routeSubscription.unsubscribe(); | ||
this._scrollSubscription && this._scrollSubscription.unsubscribe(); | ||
this._fragmentSubscription && this._fragmentSubscription.unsubscribe(); |
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.
This might be a nice time to use takeUntil
so you don't have to manage all the subscriptions
private _onDestroy = new Subject<void>();
constructor() {
this._router.events
.takeUntil(this._onDestroy)
.subscribe();
}
ngOnDestroy() {
this._onDestroy.next();
this._onDestroy.complete();
}
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
this.componentDocItem = docItems.getItemById(params['id']); | ||
this.componentDocItem = docItems.getItemById( | ||
params['id'], | ||
this._route.parent.snapshot.params['section']); |
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.
Maybe?
Observable.combineLatest(this._route.params.pluck('id'), this._route.parent.params.pluck('section'))
.map(([id, section]) => docItems.getItemById(id, section))
.takeUntil(this._onDestroy)
.subscribe(docItem => ...);
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.
Good suggestion; the typings for pluck
are messy, so I did it manually from the tuple type
Observable.combineLatest(_route.params, _route.parent.params)
.map((p: [Params, Params]) => ({id: p[0]['id'], section: p[1]['section']}))
.map(p => docItems.getItemById(p.id, p.section))
.subscribe(d => {...});
I don't have a better idea for marking the package, so open for real review |
Needs rebase |
@andrewseguin rebased |
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
Goes along with angular/components#7675.
I've done some ugly things here- any suggestions on cleaning it up without hard-coding a bunch of repetitive stuff?
I'm also doing something weird with the router because I don't know the real way.