Skip to content
This repository was archived by the owner on Feb 22, 2018. It is now read-only.

@Template replaces @Decorator #1003

Closed
wants to merge 2 commits into from
Closed

@Template replaces @Decorator #1003

wants to merge 2 commits into from

Conversation

vicb
Copy link
Contributor

@vicb vicb commented May 5, 2014

replaces #861

TODO:

  • Update the transformers
  • Add some @Template tests in annotation_src_spec

@vicb vicb mentioned this pull request May 5, 2014
Closed
@vicb vicb added cla: yes and removed cla: no labels May 5, 2014
@mhevery
Copy link
Contributor

mhevery commented May 6, 2014

Nice start. I think we should drop target and just use map. The {'.': 'foo'} was an ugly hack which I intended to remove, but never got around to it. I think now would be a good time to do it.

@vicb
Copy link
Contributor Author

vicb commented May 6, 2014

@mhevery Most the issues should be solved.

I still have one pb:

Firefox 28.0.0 (Linux) walking compiler dte.compiler should compile a directive that ignores children FAILED
    Test failed: Caught Expected: ['Ignore', 'Ignore']
      Actual: ['Ignore', 'OneOfTwo', 'TwoOfTwo', 'Ignore', 'OneOfTwo', 'TwoOfTwo']
       Which: was 'OneOfTwo' instead of 'Ignore' at location [1]

I'll look at it tomorrow, any hint appreciated (it's in JS so related to the transformers).

@vicb
style: some code cleanup
b4f5e23 
Dart coding convention
Add explicit types
Some refactoring
@vicb
Copy link
Contributor Author

vicb commented May 7, 2014

The problem was that the code did not accumulate compileChildren and the latest directive would win. Solved by changing compileChildren = annotation.compileChildren; to compileChildren = compilerChildren && annotation.compileChildren; in ElementBinderBuilder.addDirective.

Hopefuly Travis should be happy now.

@vicb
feat(Template): @template replaces @decorator(Directive.TRANSCLUDE_CH…
51c189a 
…ILDREN)

1- Transclusion syntax has changed:

Before:

    @decorator(
        selector: '[ng-if]',
        children: Directive.TRANSCLUDE_CHILDREN,
        map: const {'.': 'mapping'},
        <other kv pairs>)

After:

    @template(
        selector: '[ng-if]',
        map: const {'ng-if' : 'mapping'},
        <other kv pairs>)

2- Directive.children has been removed in facor of
Directive.compileChildren

Before:

    children: Directive.COMPILE_CHILDREN
    children: Directive.IGNORE_CHILDREN
    children: Directive.TRANSCLUDE_CHILDREN

After:

    compileChildren: true
    compileChildren: false
    Use @template (see the previous section)
@vicb vicb changed the title [WIP] @Template replace @Decorator @Template replaces @Decorator May 7, 2014
@vicb
Copy link
Contributor Author

vicb commented May 7, 2014

Travis is happy, should be ready to merge (after review)

@mhevery mhevery added the STAGING label May 7, 2014
vicb added a commit that referenced this pull request May 7, 2014
@vicb @mhevery
feat(Template): @template replaces @decorator(Directive.TRANSCLUDE_CH…
45364e5 
…ILDREN)

1- Transclusion syntax has changed:

Before:

    @decorator(
        selector: '[ng-if]',
        children: Directive.TRANSCLUDE_CHILDREN,
        map: const {'.': 'mapping'},
        <other kv pairs>)

After:

    @template(
        selector: '[ng-if]',
        map: const {'ng-if' : 'mapping'},
        <other kv pairs>)

2- Directive.children has been removed in favor of
Directive.compileChildren

Before:

    children: Directive.COMPILE_CHILDREN
    children: Directive.IGNORE_CHILDREN
    children: Directive.TRANSCLUDE_CHILDREN

After:

    compileChildren: true
    compileChildren: false
    Use @template (see the previous section)

Closes #1003
@mhevery mhevery self-assigned this May 7, 2014
@mhevery
Copy link
Contributor

mhevery commented May 7, 2014

Merged into staging

@jbdeboer
Copy link
Contributor

We are targetting 0.13.0 for this change.

@vicb
Copy link
Contributor Author

vicb commented May 23, 2014

@jbdeboer should I keep this PR rebased on master ? (I think it's easier to do regularly than once before merging). What about staging, should it also be rebased ?

@mvuksano mvuksano assigned vicb and unassigned mhevery and vicb May 26, 2014
@vicb
Copy link
Contributor Author

vicb commented Jul 24, 2014

Waiting for the bind- syntax to be in before rebasing this PR

@vicb
Copy link
Contributor Author

vicb commented Jul 29, 2014

rebased & merged on top of 1269

@vicb vicb closed this Jul 29, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
cla: yes
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vicb @mhevery @jbdeboer