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

Wrong height, elements overflow #37

Open
plagasul opened this issue Nov 19, 2019 · 7 comments
Open

Wrong height, elements overflow #37

plagasul opened this issue Nov 19, 2019 · 7 comments
Labels
question Further information is requested

Comments

@plagasul
Copy link

plagasul commented Nov 19, 2019

Very nice tool, thank you.

In my case very often the container's height is wrongly calculated and items overflow.

  • A working example of the error
  • If you force-reload, 1 out of 4 times max height is wrongly calculated (I added some console.logs, so look at the console, please)
  • I am starting the grid like this (static:false because the images come from an external service, but with 'static:true' and without 'items', same error)
let magicGrid = new MagicGrid({
	container: '.container',
	animate: false,
	gutter: 0,
	static: false,
	items: 8
//	maxColumns: 4,
// 	useMin: true,
});

magicGrid.listen();

  • This is Chrome 78.0.3904.97 (Official buildl) (64 bits)

Let me know if I can provide other information, or what to try, please.

Thank you

@ademideo
Copy link
Collaborator

Hey @plagasul ,

To properly render images you need to set the height of the image's div to the height of the image itself. If not, magic grid might use the height of that div before the image loads.

That said, I noticed the container does not have a gutter at the bottom. I will fix that in a PR.

@plagasul plagasul closed this as completed Dec 2, 2019
@plagasul
Copy link
Author

plagasul commented Dec 2, 2019

Actually... I believe that solution cannot be applied in my case...

If you check the given example page, the height of the image depends on its width which is 100% of its container's width, wich is 85% of its parent's width which is dynamic (a combination of % and min + max width) and actually interacts with the number of columns as defined by Magic Grid ...so I cannot easily apply the image's height to the image container.

When resizing the window, the error never seems to happen, which would be consistent with your explanation: the image is already loaded.

I find it hard to wrap my head around this one, but if this is caused by MG firing before the image is loaded, can I force it to wait ?

Or, any other suggestion?

Thank you

@plagasul plagasul reopened this Dec 2, 2019
@plagasul
Copy link
Author

plagasul commented Feb 4, 2020

So the problem is that magic grid items are not directly the images, but a div that wraps around the image. And when the image is not loaded the wrapper may be 0 height, and so magic grid fails to calculate.

In my case (which is probably common) it is not possible to set the height apriori, as it depends on the image's height, which depends on the wrapper's width, which is responsive.

Unless I am missing something very simple, the only solution seems to be to intialize magic grid only when the images are already loaded. Perhaps also hiding them until the grid is intialized.

I am unsure if this is a common task, but I cannot seem to find a reliable method to do so.

Any help would be greatly appreciated.

@ptcampbell
Copy link

@plagasul do you know the ratio of the image before it is loaded? In my case I do, which is how I derive the height in advance e.g. get width of column, multiplied by the appropriate ratio e.g. divide by 1.77 if it was a 16:9 image.

@plagasul
Copy link
Author

plagasul commented Feb 9, 2020

Thank you @ptcampbell , I'll give that a go.

By now I am having an interval checking for img.complete on all grid elements. Looks dirty but seems to work, something like:

var checkImages = setInterval(function() {
  if ( document.querySelector('.mosaico-destacados') ) {
    var completed = 0;
    document.querySelectorAll('img.gridImg').forEach(function (el, i) {
      if (el.complete) {
        completed++;
      }
    })
    if (completed == document.querySelectorAll('img.gridImg').length) {
      clearInterval(checkImages);
      // Init magic grid here
    } else {
      ...
    }
  }
}, 500)

@ptcampbell
Copy link

@plagasul — if you can avoid using intervals you should. Introducing time or the notion of a timeline normally causes pain somewhere along the line. For what it's worth, Isotope / Masonry (traditional grid based layout packages) use imagesloaded (a separate package by the same author) to determine whether all of the images have finished loading. It would be more efficient than an interval. :)

@plagasul
Copy link
Author

plagasul commented Feb 9, 2020

@ptcampbell Thank you for sharing your advice, much appreciated. I do actually agree with you. I searched and found imagesloaded before trying to come up with a solution of my won, but the last commit is 2 years ago and there seem to be a few long standing unresolved bugs. I thought it may not be really safe to use it, so I went for the dirty interval solution. In this particular case I believe it is relatively safe, as this is a single element that only loads on homepage. But yeah, yes, you advice stands. Thank you.

@e-oj e-oj added the question Further information is requested label Jan 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants