Dogfalo / materialize

Materialize, a CSS Framework based on Material Design
https://materializecss.com
MIT License
38.87k stars 4.74k forks source link

Feature Request: Make it possible to use a picture element inside the slider #6213

Open CloudPower97 opened 6 years ago

CloudPower97 commented 6 years ago

Hello, I've seen that the current slider implementation just looks for the src property of the img element.

this.$slides.find('img').each((el) => {
     let placeholderBase64 =

'data:image/gif;base64,R0lGODlhAQABAIABAP///wAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==';
     if ($(el).attr('src') !== placeholderBase64) {
          $(el).css('background-image', 'url("' + $(el).attr('src') + '")');
          $(el).attr('src', placeholderBase64);
     }
});

This is NOT ideal, as, for example, one can't use a picture HTML element

to provide versions of an image for different display/device scenarios.

Instead, consider using this:

this.$slides.find('img').each((el) => {
     let placeholderBase64 =

'data:image/gif;base64,R0lGODlhAQABAIABAP///wAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==';
     if ($(el).attr('currentSrc') !== placeholderBase64) {
          $(el).css('background-image', 'url("' + $(el).attr('currentSrc') + '")');
          $(el).attr('currentSrc', placeholderBase64);
     }
});

This little change make it possible to use a picture element, but doesn't expect it to be there, since every img element has a currentSrc property, wheter a picture element is present or not. So this doesn't force you to update any existing HTML code if this change will ever be accepted.

I already have a branch on my local machine, if you want I can create a PR for this issue.

Hopefully we'll be able to use a picture element soon with all of its benefit.

Anyway thanks for your work, I just love materialize.css

Keep up the good work!

khalyomede commented 5 years ago

Also, to support <picture> element on the CSS side, you should consider patching /sass/components/_carousel.scss from (starting from line 46):

.carousel-item {
    visibility: hidden;
    width: $carousel-item-width;
    height: $carousel-item-height;
    position: absolute;
    top: 0;
    left: 0;

    & > img {
      width: 100%;
    }
}

to:

.carousel-item {
    visibility: hidden;
    width: $carousel-item-width;
    height: $carousel-item-height;
    position: absolute;
    top: 0;
    left: 0;

    & > img`,
    & > picture > img {
      width: 100%;
    }
}