-
Notifications
You must be signed in to change notification settings - Fork 875
docs(guide): add dart version of hierarchical dependency injection #683
Conversation
@Output() EventEmitter canceled = new EventEmitter(); | ||
@Output() EventEmitter saved = new EventEmitter(); | ||
|
||
RestoreService<Hero> _restoreService; |
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 one can be final
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.
Also private. Good catch +1
Edit: nope it must be public.
2238a5c
to
3d7f88e
Compare
- angular2: | ||
platform_directives: | ||
- 'package:angular2/common.dart#CORE_DIRECTIVES' | ||
- 'package:angular2/common.dart#FORM_DIRECTIVES' |
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.
wouldn't juts #COMMON_DIRECTIVES
cover both?
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.
@yjbanov what would you recommend?
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.
CORE_DIRECTIVES
only include:
const List<Type> CORE_DIRECTIVES = const [
NgClass,
NgFor,
NgIf,
NgStyle,
NgSwitch,
NgSwitchWhen,
NgSwitchDefault
];
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.
I'd recommend including the minimum necessary. It does not look like you need the form directives for this example app.
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.
hero_editor_component.dart
needs an NgModel, if I don't include FORM_DIRECTIVES It won't work.
Btw If I only include directives: const [NgModel]
in that component it also does't work, only works with FORM_DIRECTIVES. Is this a bug or work as intended?
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.
Either a bug or possibly you forgot an import?
3d7f88e
to
8eaac0e
Compare
I think made all the corrections, I end up using: (Thanks to zoechi)
Do you want me to change this for a directive FORM_DIRECTIVES in the |
@andresaraujo I think it's better to keep everything in platform_directives. I don't know whether it's better to use COMMON_DIRECTIVES or CORE_DIRECTIVES & FORM_DIRECTIVES. @yjbanov? |
This looks good to me, as far as I can tell. @sanfordredlich can you do a technical review? |
@andresaraujo: You don't need to squash commits when you upload. I'll take care of that when I merge, and it'd be nice to be able to more easily see the diffs between versions. |
LGTM. Trivial: you might want to match case on the power:
On Tue, Jan 12, 2016 at 2:21 PM Kathy Walrath notifications@github.com
|
LGTM! I'll merge this. |
Adds Dart version of hierarchical dependency injection guide.
@kwalrath Could you take a look?
Thanks.