adapter and ng-show
emabiz opened this issue · 20 comments
Hi,
I need to hide some elements displayed using ui.scroll.
I'm using version 1.6.3.
If you modify the basic adapter demo/adapter/adapter.html like this:
If you replace ng-show with ng-if, you should get an exception like this:
TypeError: Cannot read property 'toLowerCase' of undefined
at new Padding (http://127.0.0.1:8000/dist/ui-scroll.js:1088:28)
at JQLite.createPaddingElements (http://127.0.0.1:8000/dist/ui-scroll.js:883:20)
at http://127.0.0.1:8000/dist/ui-scroll.js:1274:16
at publicLinkFn (https://ajax.googleapis.com/ajax/libs/angularjs/1.3.14/angular.js:6956:29)
at boundTranscludeFn (https://ajax.googleapis.com/ajax/libs/angularjs/1.3.14/angular.js:7096:16)
at controllersBoundTransclude (https://ajax.googleapis.com/ajax/libs/angularjs/1.3.14/angular.js:7756:18)
at link (http://127.0.0.1:8000/dist/ui-scroll.js:1273:5)
at invokeLinkFn (https://ajax.googleapis.com/ajax/libs/angularjs/1.3.14/angular.js:8219:9)
at nodeLinkFn (https://ajax.googleapis.com/ajax/libs/angularjs/1.3.14/angular.js:7729:11)
at compositeLinkFn (https://ajax.googleapis.com/ajax/libs/angularjs/1.3.14/angular.js:7078:13)
maybe there is a better way to hide some elements and at the same time use the adapter.
Can you help me on this, please?
Thank you for your job,
Emanuele
@emabiz First of all, thanks for the issue. I made a little update on the directive sources, so the next time (the next release I mean) you'll get the meaningful error: 'ui-scroll directive requires an Element node for templating the view'.
May I ask you, what is your use case? The situation when you put ng-if="false" on the template during/before ui-scroll initialization is not normal, it surely leads to problems. I can think a bit more and give your detailed explanation, but it would be better if we would search a solution for your exact use case instead.
Actually, ran into this problem too, and to be honest I just assumed I was doing something wrong and hacked around it (since I didn't see this problem on any of the demos, just in my project).
The short version is I added an extra check inside the Padding constructor to fall back to using <div></div> if it would have otherwise failed. It's a late hour for me right now, but tomorrow sometime if you haven't already started making headway on this I'll see if I can replicate on a plunker for you.
Hi,
my real scenario is to display a long list of contacts, that can be added and removed dinamically. So I'm using the adapter.
Some contacts could be shown or not following some rules.
Sorry in my initial post the pasted html has been broken by github. Is was:
<div ui-scroll-viewport class="viewport viewport-height-fixed" id="viewport-adapter1">
<div class="item" ui-scroll="item in datasource"
adapter="firstListAdapter"
ng-show="false"
is-loading="isLoadingOnScope"
buffer-size='5'>{{item.content}}</div>
</div>
In the example above I put simply ng-show="false" for every item, but in my code I have a function.
Now I'm changing my code filtering the list that has to be displayed before to pass it to the datasource/adapter.
But before using uiscroll I have a simple ng-repeat directive plus ng-show and was working.
Replacing ng-repeat with ui-scroll caused ng-show stop working.
I have also another question. In the controller how can I do to know when the adapter is created?
It is not created immediately. I can be sure that the adapter exists only after the $scope.datasource.get callback has been triggered once
Thank you,
Emanuele
Ok, here is a plunker: https://plnkr.co/edit/sIbgmzBkw5c2puWRQkBk
I saw this myself a few days ago when I first started using ui-scroll and hacked around it, but later realized I was in fact doing things wrong and took my ng-if off the ui-scroll element, which made my hack unnecessary. I could definitely see most people giving up however rather than digging in the way I did.
I'm not sure what the correct approach to solving this is. There seem to be 2 (possibly unrelated) issues, which can be verified in the plunker:
ng-showandng-hidedon't work on theui-scrollelement (they don't add the.ng-hideclass, so all elements continue to be visible)ng-ifon theui-scrollelement causes the whole thing to crash and burn with the error mentioned above (Cannot read property 'toLowerCase' of undefined)
The use case for those things is that someone for whatever reason may have a datasource which provides elements, but which they then want to filter later so not all are displayed. Since there's no built-in way to filter data in ui-scroll the way one can with ng-repeat's | filter:<something> syntax, I would imagine ng-show and ng-if is something a lot of people are likely to try (even if there is perhaps a better way from ui-scroll's perspective).
It may be possible to fix things so that those 3 directives to function as intended, but my gut instinct is that's going to be a lot of effort, and likely not worth it. So instead, a solution would probably involve add to the README to explain that ng-show, ng-hide, and ng-if are forbidden on the ui-scroll element, and that the proper way to filter ui-scroll elements is inside the datasource.
Ideally, we would also put some checks in the code to throw informative errors if those directives are detected on the ui-scroll element.
As a further aid to this use case, it might be possible to add a new attribute binding called filter or something similar, which receives a function by which elements from datasource.get() can be filtered from display. But that's probably pretty substantial work as well, so the above (documentation and informative errors) is likely the best short-term fix, IMO.
@emabiz As for how to detect that the adapter is in place, you can always use if ($scope.datasource.adapter) {} to look for it directly. If necessary, you could queue up the things you need to do on the adapter, and then set up a $interval timer to check for it and perform the actions once it exists.
Regarding ng-hide/ng-show. That directives put on the ui-scroll element may not work due to internal mechanism of visualization optimization (look here and here) – the ui-scroll engine hides elements after inserting and before data binding and then shows them again after the data has been bound. And ng-hide class is being used for that. Maybe it's a good idea to replace it with for example ui-scroll-ng-hide class... that should allow ng-hide/ng-show directives.
But the main problem here is that the fetching mechanism works with real heights, and if all items fetched in a previous turn are hidden, then the next fetch will be trigger immediately right after the data is bound because the viewport won't be filled. This will cuase unlimited fetching issue.
Ho guys,
Thank you for your previous anwers.
So, if I have for example to add a search component that allows to filter elements on the list, how do I have to proceed? Prefilter elements and manually update the adapter? What do you suggest?
@emabiz, keep in mind you should listen to @dhilt over me, as I've only recently started working with ui-scroll.
But generally yes, you are correct. It sounds like currently, ui-scroll is not really built to handle elements that will change height, and that includes elements which will change height from hidden / 0px to some other height or vice versa.
If you need to dynamically filter elements within ui-scroll, the correct place to do that is before/during datasource.get(). If the filter criteria changes, you either need to use adapter. topVisible and adapter. bottomVisible (or one of their variants) along with adapter.applyUpdates/adapter.prepend/adapter.append to enforce changes in the currently-displayed elements.
Or alternatively, you could use adapter. topVisible or adapter. topVisibleScope to determine the index of the top visible item, and simply call adapter.reload(topVisibleIndex) to refresh the ui-scroll data via new calls to datasource.get() so you can handle all the filtering there. I think this approach would yield simpler/cleaner code, but could cause re-rendering of some existing ui-scroll items that didn't really need to be re-rendered, and so is a bit wasteful on performance. If that's OK for your app, I'd be inclined to suggest the adapter.reload(topVisibleIndex) approach.
@dhilt, I see what you are saying. So it seems like the answer at least for now is probably just to add / further refine the error messages, and add documentation? If you think that's a good idea, I may try to work on this unless you want to handle it quicker (I'd probably get the PR up sometime in the next week).
I might also make PR adding a couple example pages which implement list filters using those 2 approaches I just described. It feels like a common enough use case (dynamically filtering a list on client-side) that it would be useful to show the recommended way to approach it.
The ui-scroll certainly could handle items with different heights and here is unpublished demo on the subject. Also, here is the logic to consider the heights dynamic updates (not sure if it's tested well).
Then, here is the code to watch for items which are inserted being invisible. The scroller updates yourself when invisible item becomes visible. And the demo: scroll 20-40 pages down, then go to the top. And when items retrieving is in progress, turn off the viewport visibility. This will cause fetching process stop (and I was not right when I spoke about unlimited fetching issue, this and this conditions will be false due to effectiveHeight = 0). But then you would turn the viewport visibility on, which will make items visible and which will trigger further fetching.
So I can't agree with you on these 2 points.
The problem with ng-if is that Angular starts ui-scroll directive processing before ng-if destroys it (I believe this is so due to priority 1000 which is higher then 600). But ng-if happens before Paddings templating, which causes toLowerCase undefined error (I changed the error in the branch). You see, it's not the same as ng-show issue I mentioned earlier.
PS I will answer to your last comment a bit later, thanks for your suggestions and help!
@emabiz
I agree with datasourse solution for search logic. The datasource is a proxy between scroller and remote data source. If no search is presented, the proxy do nothing but calls the remote with index-count params and passes the result to the scroller via success callback. If search is presented, the datasource needs to call the remote with index-count-search params and again pass the result to the scroller. And yes, I would recommend to reload the scroller each time you change the search. If you want client-side-only filtering, then Adapter.applyUpdates is your friend.
@zacronos Sorry, I was involved into a long story with #184 PR and it sucked my energy out. A new example(s) would be very helpful! Try also this demo, it does not work properly in master, but in a branch it's ok. Maybe it would be a good idea to provide Server factories conglomerate like it is done for test-datasources... I think it should displace the data generator logic for most part of current fake demo-datasources. One simple Server factory (just to satisfy index-count calls), one from branch's adapterSync demo (to provide append/prepend/remove functionality) and one for your new sample (for index-count-search calls).
Regarding documentation, just open the PR and let's discuss it in details.
@emabiz I introduced .ng-ui-scroll-hide class instead of .ng-hide for view optimization purposes, so the initial issue with ng-show directive should go away. The code is in branch and this will be included in the next release. Regarding ng-if, I put some explanation here. As a workaround the ng-if directive could be used within the ui-scroll template in a way like
<div ui-scroll="item in datasource">
<span ng-if="shouldHide(item)">
{{item.text}}
</span>
</div>
And I agree, that we may update the documentation...
Hi,
I tried the ng-show fix of the branch with the adapter demo, as before.
Now if I put ng-show="false" it works, nothing is shown.
But if I use this:
<div ui-scroll-viewport class="viewport viewport-height-fixed" id="viewport-adapter1">
<div class="item" ui-scroll="item in datasource"
adapter="firstListAdapter"
ng-show="isVisible()"
is-loading="isLoadingOnScope"
buffer-size='5'>{{item.content}}</div>
</div>
$scope.isVisible=function(){
var res=Math.floor((Math.random() * 10) + 1)>5;
return res;
}
I get this exception:
angular.js:11607 Error: [$rootScope:infdig] 10 $digest() iterations reached. Aborting!
Watchers fired in the last 5 iterations: [[{"msg":"isVisible()","newVal":true,"oldVal":false},{"msg":"isVisible()","newVal":false,"oldVal":true},{"msg":"isVisible()","newVal":false,"oldVal":true}],[{"msg":"isVisible()","newVal":false,"oldVal":true},{"msg":"isVisible()","newVal":true,"oldVal":false}],[{"msg":"isVisible()","newVal":true,"oldVal":false},{"msg":"isVisible()","newVal":true,"oldVal":false}],[{"msg":"isVisible()","newVal":true,"oldVal":false}],[{"msg":"isVisible()","newVal":false,"oldVal":true},{"msg":"isVisible()","newVal":false,"oldVal":true},{"msg":"isVisible()","newVal":true,"oldVal":false}]]
http://errors.angularjs.org/1.3.14/$rootScope/infdig?p0=10&p1=%5B%5B%7B%22msg%22%3A%22isVisible()%22%2C%22newVal%22%3Atrue%2C%22oldVal%22%3Afalse%7D%2C%7B%22msg%22%3A%22isVisible()%22%2C%22newVal%22%3Afalse%2C%22oldVal%22%3Atrue%7D%2C%7B%22msg%22%3A%22isVisible()%22%2C%22newVal%22%3Afalse%2C%22oldVal%22%3Atrue%7D%5D%2C%5B%7B%22msg%22%3A%22isVisible()%22%2C%22newVal%22%3Afalse%2C%22oldVal%22%3Atrue%7D%2C%7B%22msg%22%3A%22isVisible()%22%2C%22newVal%22%3Atrue%2C%22oldVal%22%3Afalse%7D%5D%2C%5B%7B%22msg%22%3A%22isVisible()%22%2C%22newVal%22%3Atrue%2C%22oldVal%22%3Afalse%7D%2C%7B%22msg%22%3A%22isVisible()%22%2C%22newVal%22%3Atrue%2C%22oldVal%22%3Afalse%7D%5D%2C%5B%7B%22msg%22%3A%22isVisible()%22%2C%22newVal%22%3Atrue%2C%22oldVal%22%3Afalse%7D%5D%2C%5B%7B%22msg%22%3A%22isVisible()%22%2C%22newVal%22%3Afalse%2C%22oldVal%22%3Atrue%7D%2C%7B%22msg%22%3A%22isVisible()%22%2C%22newVal%22%3Afalse%2C%22oldVal%22%3Atrue%7D%2C%7B%22msg%22%3A%22isVisible()%22%2C%22newVal%22%3Atrue%2C%22oldVal%22%3Afalse%7D%5D%5D
at angular.js:63
at Scope.$digest (angular.js:14281)
at processUpdates (ui-scroll.js:347)
at adjustBufferAfterFetch (ui-scroll.js:375)
at ui-scroll.js:416
at adapter.js:20
at angular.js:16223
at completeOutstandingRequest (angular.js:4905)
at angular.js:5285
is it allowed, or I'm doing something wrong?
@emabiz This issue does not relate to ng-show and even to ui-scroll. Try
<div ng-repeat="item in [1,2,3,4,5]">{{isVisible()}}</div>
and you'll get the same error. This is how the AngularJS dirty checking works. It scans the scope for changes. As all watched entities are in a single loop (digest cycle), any value change of any variable forces to reassign values of other watched variables in DOM. The isVisible() call is that watched entity which forces the {{isVisible()}} interpolation string to be rebuilt. That digest cycle has only 10 levels in the deep and on each turn your isVisible() returns random value, so he can't stop.
As a workaround you may pass to your controller's isVisible method an index variable
<div ui-scroll="item in datasource" ng-show="isVisible($index)">{{item.content}}</div>
and store the results on scope to make it constant during digest cycle (until the index is not changed)
let visibilityList = [];
$scope.isVisible = (index) => {
const found = visibilityList.find(v => v.index === index);
if(found) {
return found.value;
}
const value = Math.floor((Math.random() * 10) + 1) > 5;
visibilityList.push({
index: index,
value: value
})
return value;
};
Ah sorry for that,
I just wrote the first thing that came to my mind in order to change the visibility of items.
Then for me the fix works and the issue can be closed.
I wait for a release that contains it.
Thank you very much!
:)
Closed due to v1.7.0-rc.4 containing these fixes has been released.