Skip to content

feat: add exportAs logic for isActive on routerLinkActive directive#940

Merged
sis0k0 merged 2 commits intoNativeScript:masterfrom
sean-perkins:master
Aug 19, 2017
Merged

feat: add exportAs logic for isActive on routerLinkActive directive#940
sis0k0 merged 2 commits intoNativeScript:masterfrom
sean-perkins:master

Conversation

@sean-perkins
Copy link
Copy Markdown

Included example usage in the ng-sample for router-outlet-test.ts.

Implements the exportAs feature for the nsRouterLinkActive directive. Taken from Angular's repository, this allows you to dynamically access the isActive state of the directive to do additional logic outside of just binding classes.

Also includes optimizations to the update cycle, to prevent unnecessary DOM manipulations as called out here: https://github.com/angular/angular/blob/master/packages/router/src/directives/router_link_active.ts#L124

@ghost ghost added the ♥ community PR label Aug 13, 2017
@sis0k0
Copy link
Copy Markdown
Contributor

sis0k0 commented Aug 15, 2017

run ci


private hasActiveLinks(): boolean {
return this.links.some(this.isLinkActive(this.router)) ||
this.links.some(this.isLinkActive(this.router));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we don't need that line.

@NativeScript NativeScript deleted a comment from ns-bot Aug 17, 2017
@NativeScript NativeScript deleted a comment from ns-bot Aug 17, 2017
@NativeScript NativeScript deleted a comment from ns-bot Aug 17, 2017
@NativeScript NativeScript deleted a comment from ns-bot Aug 17, 2017
@NativeScript NativeScript deleted a comment from ns-bot Aug 17, 2017
@sean-perkins
Copy link
Copy Markdown
Author

I agree the statement did seem unneeded. I wonder why it existed in the Angular repository. Updated nonetheless.

@sis0k0 sis0k0 changed the title Implements #928 - exportAs logic for isActive on routerLinkActive directive feat: add exportAs logic for isActive on routerLinkActive directive Aug 19, 2017
@ghost ghost assigned sis0k0 Aug 19, 2017
@ghost ghost added in progress and removed ♥ community PR labels Aug 19, 2017
Copy link
Copy Markdown
Contributor

@sis0k0 sis0k0 left a comment

Choose a reason for hiding this comment

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

Looks great!

@sis0k0 sis0k0 merged commit 147d35a into NativeScript:master Aug 19, 2017
@ghost ghost removed the in progress label Aug 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants