Skip to content

Adapter is not assigned to $rootScope when compileProvider.debugInfoEnabled is set to false #132

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

Closed
adybionka opened this issue Jan 12, 2017 · 4 comments · Fixed by #144
Closed

Comments

@adybionka
Copy link

I have spent a lot of time to find out, why adapter is not assigned properly to controller scope but instead of it to rootScope.

In my html code I use uiScrollViewport directive, but still adapter was assigned to rootScope.

I analyzed ui-scroll code, and I found that scope is create from directive element:

var candidateScope = candidate.scope();

but my app has set compileProvider.debugInfoEnabled(false) so candidate.scope() returns undefined. This is why adapter has been assigned to rootScope instead to my controller scope.

In my opinion this is bug. ui-scroll should not be dependent of debugInfoEnabled flag. Especially you don't mention about it in the documentation and it is very hard to figure out what is wrong.

@dhilt
Copy link
Member

dhilt commented Jan 13, 2017

@adybionka Looks like we need to review a mechanism of the target scope injection. This may require time. Right now I don't see appropriate workaround. Thanks fo report!

@adybionka
Copy link
Author

I've encountered another problem related with the scope injection mechanism. If I use controller as syntax sometimes adapter is assigned to proper scope, but sometimes not. Everything depends of scopes hierarchy.

Problem is here: candidate = candidate.parent(); - you can't rely on this, because you don't now which parents scopes is proper target scope. If are some scopes between target scope and ui-scroll directive, then is the problem.

You have changed something in 1.5.1. I'm wonder why? In case that in 1.5.0 this mechanism was fine (at least in my app).

@dhilt
Copy link
Member

dhilt commented Jan 18, 2017

@adybionka Do you have a proof? Cause I'm sure we have tests for most cases of nested scope hierachies: https://github.com/angular-ui/ui-scroll/blob/master/test/AssigningSpec.js. Regarding to mechanism changing, I found some discussion #110... but the main question is still here: we can't use .scope method in so-called production mode activated via compileProvider.debugInfoEnabled(false)

@adybionka
Copy link
Author

Ok, the problem is slightly different. I have app like this:

<div ng-controller="MyControllerA as $ctrlA">
  <ul ui-scroll-viewport="">
      <component-a></component-a>  
      <component-b></component-b>     
  <ul>
</div>

HTML of component A:

<div ui-scroll="note in ComponentADatasource" adapter="componentACtrl.adapter">....</div>

HTML of component B:

<div>  
   <div>
        <ul ui-scroll-viewport="">
            <li ui-scroll="note in ComponentBDatasource" adapter="componentBCtrl.adapter">....</li>  
        <ul>
   <div>
<div>

and adapter is properly assigned to componentACtrl but not for componentBCtrl. In scope of component A is componentACtrl.adapter but scope of component B is empty. But componentBCtrl.adapter is in scope of MyControllerA.

Maybe this problem appears when in one ui-scroll is second which has also ui-scroll like in my app?

Anyway this is not important since you need to rewrite this mechanism because of compileProvider.debugInfoEnabled.

I analyzed a little your code, and I see that you don't create isolate scope for uiScrollViewport and uiScroll directive. So it is not possible to simply extract any variable by directive attribute by its scope.

I have an idea how to easily change extracting adapter and rest of variables.

  1. Make isolate scope in uiScrollViewport directive. It will be safer then making isolate scope of uiScroll directive.
  2. Add to controller of uiScrollViewport directive, public methods like setAdapter(adapter), setIsLoading(isLoading) etc.
  3. Extract adapter, isLoading... variables from uiScroll by setting them using public methods of uiScrollViewport controller.
angular.module('ui.scroll', []).directive('uiScrollViewport', function () {
  return {
    restrict: 'A',
    scope: {
      'adapter': '=?'
    },
    controller: ['$log', '$scope', '$element', function (console, scope, element) {
      var self = this;

      self.setAdapter = function(adapter) {
        scope.adapter = adapter;
      };

usage:

<ul ui-scroll-viewport="" adapter="$ctrl.adapter">
    <li ui-scroll="note in datasource" >....</li>  
</ul>

So you will have a very easy and reliable mechanism to extract variables by a directive isolate scope. Of course it is not backward compatible, but you can add this 2nd way of extracting variables, mark old one as deprecated and in the future release delete the old one.

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 a pull request may close this issue.

2 participants