stroke style directive #17
Conversation
alienlike
force-pushed
the
nw/stroke-style-directive
branch
from
04f9b72
to
cc9ff46
Compare
|
Rebased and ready for review. |
alienlike
force-pushed
the
nw/stroke-style-directive
branch
from
cc9ff46
to
ef39a94
Compare
| @@ -1,5 +1,7 @@ | |||
| import { Component, Directive, EventEmitter, Host, OnDestroy, OnChanges, AfterContentInit, Input, Output, ContentChild } from '@angular/core'; | |||
| import { style } from 'openlayers'; | |||
| import { Subscription } from 'rxjs/Subscription'; | |||
| import { Observable } from 'rxjs/Observable'; | |||
There was a problem hiding this comment.
Unused import Observable
| if (this.iconStyleDirective) { | ||
| this.childSubscription$ = this.iconStyleDirective.onChanged.subscribe((): void => this.update()); | ||
| } | ||
| else if (this.strokeStyleDirective) { |
There was a problem hiding this comment.
Can stroke and icon not be used on the same style element?
If they can, this shouldn't be else if but only if
same for the update function
|
The code looks good to me, but wasn't able to test it due to the lack of an example. |
|
Addresses comments above. |
|
LGTM, I would also say that you can keep the relative path as a temporary solution since its more important that everybody can test the example than it being aot compatible, especially since we know that library is aot compatible at the moment. Then when we finished discussing the real solution in #30 we can change it again. |
alienlike
force-pushed
the
nw/stroke-style-directive
branch
from
39236ed
to
6cc7c62
Compare
|
Cool. I will leave the rel path then. |
|
should be addressed in 0.5.0, though in exactly the same fashion. Feel free to reopen if need be. |
No description provided.