Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to 1.0.0 breaks inline directive #194

Closed
websirnik opened this issue Jun 5, 2015 · 6 comments · Fixed by #240
Closed

Update to 1.0.0 breaks inline directive #194

websirnik opened this issue Jun 5, 2015 · 6 comments · Fixed by #240
Labels

Comments

@websirnik
Copy link
Contributor

I've updated from 1.0.0-beta-2 to 1.0.0, the following declaration has stopped working for me:

    <div ng-switch-when="some-clause" >
        <div oc-lazy-load="['/vendors/file1.js', '/vendors/file2.js' ]">
            <my-directive url="{[ vm.url ]}"></my-directive>
        </div>
    </div>

I can see that /vendors/file1.js& /vendors/file2.js were loaded, but my-directive never got compiled. fyi my-directive is not inside those files, it was loaded before.

In the docs you mention different syntax, but it produces an error:

<div oc-lazy-load="{['js/testModule.js', 'partials/lazyLoadTemplate.html']}">
  <!-- Use a directive from TestModule -->
  <test-directive></test-directive>
</div>
@ocombe
Copy link
Owner

ocombe commented Jun 8, 2015

I changed some things to the way directives are handled (to fix another bug).
Thanks for reporting this, I'll try to find a way to fix this one without breaking the other one :D

@ocombe ocombe added the bug label Jun 8, 2015
o-3 added a commit to o-3/ocLazyLoad that referenced this issue Jun 17, 2015
Just wrapped raw html string as an element. 
This addresses Issue ocombe#194.
@o-3
Copy link

o-3 commented Jun 17, 2015

For me, 532c7fe fixed the directive compiling issue. Thanks for the great tool:)

@joaovieira
Copy link
Contributor

@snkhrn commit on top of v1.0.5 did not work for me.

Don't know why the $compile step in the directive has changed from the simplified old version:

$ocLazyLoad.load(moduleName).then(function success(moduleConfig) {
  $animate.enter($compile(content)($scope), null, $element);
});

Into:

$ocLazyLoad.load(moduleName).then(function () {
  $animate.enter(content, $element);
  var contents = element.contents();
  angular.forEach(contents, function (content) {
    if (content.nodeType !== 3) {
      // 3 is a text node
      $compile(content)($scope);
    }
  });
});

In this last version, element.contents() will always be empty as you empty it in element.html(''); above. Therefore, the $compile step never happens.

Solution (is there really a need to skip text nodes?):

  1. Compile the entire original content, nicely as it was before:
$animate.enter($compile(content)($scope), $element);
  1. Or use $element instead of element, since the $animate.enter appends the content into $element:
var contents = $element.contents();

I would do a PR with solution 1, but maybe there's a reason for not compiling the content altogether?

For the record, this is my simplified view code:

<div ng-if="flag">
  <div oc-lazy-load="'asyncModule'">
    <ng-include src="'templateSrc'"></ng-include>
  </div>
</div>

Using angular 1.4.6. For some other reason that I didn't want to investigate it works fine without the ng-if.

@ocombe
Copy link
Owner

ocombe commented Sep 24, 2015

This directive proves to be harder to support all the cases than expected :)
Any help would be appreciated on this, but I'd like to keep the compatibility with text nodes

@joaovieira
Copy link
Contributor

Alright, I went back to #168, to find the source for this. Run some quick tests - true that angular doesn't like angular.element('bla bla <div></div>') or $compile('bla bla <div></div>')($scope)...

So the trick is not to store the HTML string as the original content, but the contents element itself. I've created a PR with this. #240

ocombe added a commit that referenced this issue Oct 1, 2015
fix: directive compile original contents after dependency is loaded.
Fixes #168 
Fixes #194
@mstone6769
Copy link

I found some issues using the directive and having ngAnimate turned on in an Angular 1.2.13 app. I believe this is related to the issue above.

http://codepen.io/mstone6769/pen/bBWyGj?editors=1010

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants