Skip to content
This repository was archived by the owner on Dec 18, 2024. It is now read-only.

feat: Prepare for adding CDK docs #244

Merged
merged 1 commit into from
Oct 9, 2017
Merged

Conversation

josephperrott
Copy link
Member

import {SECTIONS} from '../../shared/documentation-items/documentation-items';

@Injectable()
export class CanActivateComponentSidenav implements CanActivate {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add class description?

constructor(private router: Router) {}

canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot) {
let sectionFound = Object.keys(SECTIONS).find(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add comment explaining what this is doing? E.g. Prevent navigation to a section if...


ngOnInit() {
this._componentPageTitle.title = 'Component Categories';
this.params = Observable.combineLatest(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add comment here?

'components': 'Components',
};
const CDK = 'cdk';
const COMPONENTS = 'components';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worthwhile to have these as constants?

@josephperrott josephperrott force-pushed the cdk branch 2 times, most recently from 1d199fc to 0ebe8a8 Compare September 15, 2017 20:54
@josephperrott josephperrott force-pushed the cdk branch 5 times, most recently from 7ef8532 to 0b6c07a Compare October 9, 2017 17:07
@josephperrott
Copy link
Member Author

@jelbourn This is ready for review once more.

Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jelbourn jelbourn merged commit fd7d5d4 into angular:master Oct 9, 2017
@josephperrott josephperrott mentioned this pull request Oct 10, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants