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

lazySizes.autoSizer.checkElems() should trigger reassessment of bg image implemented by object-fit #648

Closed
Hlsgs opened this issue May 1, 2019 · 4 comments

Comments

@Hlsgs
Copy link

Hlsgs commented May 1, 2019

I am using LS with parent-fit, object-fit and respimg. Some interesting "issues" arose when cloning an element that contains an image already handled by the plugins:

If cloning and appending said element somewhere where it has different dimensions via CSS, the auto-sizes functionality does not do it's job out of the box. Since LS auto-detects elements added to the DOM, I wondered what exactly in the cloned nodes was stopping that, so I removed the lazyloaded class and readded lazyload. Modern browsers started working as expected, but IE11 din not(It's stuck with the background src it had before cloning).

var source = $(this);
var content = source.children().not('.lazysizes-display-clone').clone();
content.find('img').removeClass('lazyloaded').addClass('lazyload');
stage.find('#team-info-wrapper').empty().append( content );

My next idea was adding lazySizes.autoSizer.checkElems() after appending the element, and, again, modern browsers cool, IE bad.

I then did a quick check without object-fit, and everything was fine in IE, so I guess it's with object-fit that the logic stops.

I'm thinking that both readding the lazyload class and calling lazySizes.autoSizer.checkElems() should, in turn, trigger a reassessment of the bg image implemented by the object-fit.

I hope this all made sense!

Cheers and much kudos for this excellent stuff. It's become an essential part of my toolkit!

@aFarkas
Copy link
Owner

aFarkas commented May 1, 2019

Could you write a simplified testcase over at jsfiddle, jsbin or whatever to showcase your problem?

@Hlsgs
Copy link
Author

Hlsgs commented May 1, 2019

Ofcourse:
https://jsfiddle.net/Hlsgs/exb60ghj/46/
https://jsbin.com/zelejeb/edit?html,css,js,output

The problme is, though, that in both cases, ls.object-fit doesn't seem to run at all in IE.

@aFarkas
Copy link
Owner

aFarkas commented May 1, 2019

I just made some changes that should fix your issue. But I'm not really happy with the changes and not 100% sure about the robustness (Need to think about this).

I must admit that this part is better handled by the object-fit-images polyfill of @bfred-it. At the same time I had once a bug report about some kind of race condition where the full polyfill had some issues showing the right image and I don't really understand why the project is archived and @bfred-it isn't supporting it anymore?

@Hlsgs
Copy link
Author

Hlsgs commented May 1, 2019

I just tested and it seems to work fine. Both calling lazySizes.autoSizer.checkElems() and readding the lazyload class work.

That beeing said, I can see what you're saying about rubustness, as I've noticed some inconsistencies in that, whether one filters out lazysizes-display-clone when cloning or not, the resulting markup for the new clone is diferent from the original, contains the original img srcset and the data-object-fit-polyfilled attribute.

"Original" clone:

<img width="170" height="170" class="grid-image cover-adjust lazysizes-display-clone lazyloaded" style='background-position: center; background-image: url("https://via.placeholder.com/320x320.jpg"); background-repeat: no-repeat; background-size: cover;' alt="" src="data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==" sizes="(max-width: 620px) calc(50vw - 18px), (max-width: 1279px) 282px, 291px" data-sizes="auto" data-aspectratio="1" data-srcset="" srcset="data:image/svg+xml;utf8,%3Csvg%20xmlns%3D%27http%3A%2F%2Fwww.w3.org%2F2000%2Fsvg%27%20viewBox%3D%270%200%20170%20170%27%2F%3E" data-position="" data-visibility="">

New clone:

<img width="170" height="170" class="grid-image cover-adjust ls-is-cached lazysizes-display-clone lazyloaded" style='background-position: center; background-image: url("https://via.placeholder.com/480x480.jpg"); background-repeat: no-repeat; background-size: cover;' alt="" src="data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==" sizes="291px" data-sizes="auto" data-aspectratio="1" data-srcset="" srcset="https://via.placeholder.com/170x170.jpg 170w, https://via.placeholder.com/150x150.jpg 150w, https://via.placeholder.com/360x360.jpg 360w, https://via.placeholder.com/280x280.jpg 280w, https://via.placeholder.com/250x250.jpg 250w, https://via.placeholder.com/595x596.jpg 595w, https://via.placeholder.com/600x600.jpg 600w, https://via.placeholder.com/480x480.jpg 480w, https://via.placeholder.com/768x769.jpg 768w, https://via.placeholder.com/1080x1080.jpg 1080w, https://via.placeholder.com/320x320.jpg 320w, https://via.placeholder.com/125x125.jpg 125w, https://via.placeholder.com/1200x1200.jpg 1200w" data-src="" data-position="" data-visibility="" data-object-fit-polyfilled="">

To be honest, I'm not at all interested in adding another library. I've been using LS for some time, but only recently understood and totally got into the sizes=auto functionality(I used to write sizes attributes like this:(max-aspect-ratio: 16/9) and (max-width: 1339px) calc( calc(100vh - 93px) * (16 / 9) ), (max-aspect-ratio: 16/9) and (min-width: 1340px) calc( calc(100vh - 144px) * (16 / 9) ), 100vw :) ) and so decided to go all in with your stuff. Previously, I had been using a (way) more barbaric approach for poor old IE. And Edge, as this also covers video, which I still use it for.

Have a look, if you're interested(video plugin, maybe? :)) ):

var supportsCSS = !!((window.CSS && window.CSS.supports) || window.supportsCSS || false);
var no_of_support = !supportsCSS || !CSS.supports( "(object-fit: cover)" ) || CSS.supports( "(-ms-ime-align: auto)" ); // (-ms-ime-align: auto) Edge because video

function o_fit( elements ) {

  elements.each(function(index, el) {

    var el_w_orig = $(this).attr("width") || 16;
    var el_h_orig = $(this).attr("height") || 9;

    if ( $(this).prop("tagName").toLowerCase() == "img" && supportsCSS && CSS.supports( "(object-fit: cover)" ) && CSS.supports( "(-ms-ime-align: auto)" ) ) return true; //skip for images on Edge

    var container_w = $(this).parent().innerWidth();
    var container_h = $(this).parent().innerHeight();
      
    var scale_w = container_w / el_w_orig;
    var scale_h = container_h / el_h_orig;

    if ( $(this).hasClass('contain') ) {
      var scale = scale_w > scale_h ? scale_h : scale_w;
    } else {
      var scale = scale_w > scale_h ? scale_w : scale_h;
    }

    $(this).width(scale * el_w_orig);
    $(this).height(scale * el_h_orig);

    $(this).addClass('no-object-fit');

  });

}

if ( no_of_support ) o_fit( $('.cover, .contain') );
.of-container {
 position: relative;
 overflow: hidden;
}
.cover,
.contain {
  position: absolute;
  top: 0;
  left: 0;
  width: 100%;
  height: 100%;
  object-fit: cover;
  &.no-object-fit {
    top: 50%;
    left: 50%;
    transform: translate(-50%, -50%);
    max-width: none;
  }
}
.contain {
  object-fit: contain;
}

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