s-yadav / angulargrid

Pinterest like responsive masonry grid system for angular
MIT License
277 stars 105 forks source link

Sometimes items overlap #75

Closed mavillar closed 8 years ago

mavillar commented 8 years ago

Hi,

Sometimes, in strange circunstantes, a set of items overlap at the same position.

Here I attach a snapshot of the overlap and the <em> tag shown that encapsulate such set of items.

captura de pantalla 2016-06-25 a las 9 48 38 captura de pantalla 2016-06-25 a las 9 48 20

Why the <em> tags encapsulate it? How can I avoid this?

EDIT: I've been checking and the <em> tags come from the description. <em> tags are used to highlight keywords within the description. The curious thing is that all tags are closed but do not understand the behavior of taking a <em> tag and enclose some <li> tags.

marcusmotill commented 8 years ago

Any luck?

s-yadav commented 8 years ago

@mavillar Does your code wrapping li with em tag. In angular grid nothing is written so.

jice-lavocat commented 8 years ago

I encountered the same issue today. @mavillar Did you manage to solve this... How ?

In my case, the problem disapear if I resize the window. It only occurs when the page loads.

s-yadav commented 8 years ago

@tanzaho In mavillar example its overlapping because li are wrapped inside em tag. Angulargrid expects all grid items to be sibling, wrapping a set of items in some other tag may cause this issue.

Is your case same ?

marcusmotill commented 8 years ago

@s-yadav while that may be the case in @tanzaho project I would like to introduce a solution I found to help narrow the problem.

To me I narrowed down that there is a timing issue in the library where it attempts to size itself before the DOM is ready. To fix this I simply added a timeout to refresh the grid after it has been initialized. I imagine this is what the refresh on image load option does but for me I have no images to load in my items.

        $timeout(function() {
            $scope.refreshGrid();
        }, 200);

        $scope.$root.$on('refreshGrid', function(event, data) {
            $scope.refreshGrid();
        });

        $scope.refreshGrid = function() {
            if (angularGridInstance.gallery) {
                angularGridInstance.gallery.refresh();
            }

        };

Unfortunately, this is a "7 minute abs" solution, if you will, and does not work on slower computers unless I bump the timeout duration.

jice-lavocat commented 8 years ago

This is also what I came up with. I plan to code it myself later today. I'll come back to post my own solution. Thanks @marcusmotill

s-yadav commented 8 years ago

@marcusmotill That's strange. DOM should be always ready before link method in a directory is called. Can you show your implementation of angulargrid.

jice-lavocat commented 8 years ago

@s-yadav indeed. @marcusmotill solution doesn't seem to fix the issue for me.

Here is how I'm defining my grid (I need to switch to the cssGrid soon) :

<ul class="dynamic-grid gridMessages"  angular-grid="repost.messages"  ag-grid-width="390" ag-gutter-size="10" ag-id="messagesGrid">
    <li ng-repeat="message in repost.messages" >
      <div  ng-include src="'components/repost/partials/message_view.html'"></div>
    </li>
</ul>

This is what I get when the page loads

screenshot 2016-09-21 a 20 24 38

If I resize the window (any pixel amount is fine), I get this :

screenshot 2016-09-21 a 20 27 03
jice-lavocat commented 8 years ago

It seems to me that it happens only when the last image is not positioned in the first column. I'l try to keep investigating later.

jice-lavocat commented 8 years ago

I'm sorry, forgot what I wrote earlier. If the <li> have the class "grid", then Marcus' solution works.

marcusmotill commented 8 years ago

So to be clear @tanzaho refreshing the grid worked for you only if you had grid class on <li>?

jice-lavocat commented 8 years ago

Correct (seems weird to me, but correct)

s-yadav commented 8 years ago

@tanzaho Ok. I think I got the issue. Looks like real culprit is ng-include. As it loads template asynchronously. Angular grid try to layout before template is loaded and gives overlap item. This is the reason why marcus solution worked. Though it is not reliable as ng-include is asynchronous. @marcusmotill are you doing something similar?

Try listening to event $includeContentLoaded in your controller. https://docs.angularjs.org/api/ng/directive/ngInclude . And when that event is fired refresh the angulargrid.

You can also try embeding message view directly to your grid defination just to check if that works.

Also I will suggest not to use ng-include. Instead create a new directive called MessageView which loads the template. Pass a onLoad callback to your directive which you can call from the link function of MessageView directive.

<ul class="dynamic-grid gridMessages"  angular-grid="repost.messages"  ag-grid-width="390" ag-gutter-size="10" ag-id="messagesGrid">
    <li ng-repeat="message in repost.messages" >
      <MessageView onLoad={refreshAngularGrid}/>
    </li>
</ul>

PS: Also prebundle your templates required for a particular page and load it with the page. Loading all templates async for a page may make your app feels slow.

jice-lavocat commented 8 years ago

Thanks @s-yadav , It seems that the ng-include was indeed the good track to follow for me. I changed it to a directive as you suggested. It works, without the need for the onLoad refresh.

Here is my current code :

angular.module('myApp')
.directive('messageView', function() {
  var myDirective = {
        templateUrl: 'components/repost/partials/message_view.html',
        restrict: 'EA',
        link: function (scope, elem, attrs) {
          scope.items = JSON.parse(attrs.items);
          scope.selected = attrs.selected;

          scope.selectItem = function(item) {
            scope.selected = item;
          };
        }
    };
    return myDirective;
});

and the html generating the grid :

<div class="row dynamic-grid gridMessages"  angular-grid="repost.messages" ag-options="{cssGrid : true}" ag-id="messagesGrid">
      <div ng-repeat="message in repost.messages" class="grid col-lg-4 col-sm-6">
            <div message-View></div>
      </div>
</div>

I have no overlap anymore :)