mlisook / plastic-image

A Polymer 3.0 element which adds extra plasticity to <iron-image> with support for srcset and lazy loading
MIT License
30 stars 6 forks source link

iron-image breaks plastic-image #14

Closed ozasadnyy closed 7 years ago

ozasadnyy commented 7 years ago

I have found the interesting bug. I don't know if it's your component bug or Polymer itself. Fade effect will not work if you put <plastic-image> after <iron-image> like this:

<template>
    <style>
       :host {
        display: block;
      }

      .image {
        width: 400px;
        height: 400px;
        background-color: purple;
      }
    </style>
    <iron-image class="image" src="http://thecatapi.com/api/images/get" preload fade sizing="cover"></iron-image>
    <plastic-image class="image" srcset="http://thecatapi.com/api/images/get" preload fade sizing="cover"></plastic-image>
</template>

The problem I have found that _imgOnLoad is not firing in <iron-image> as a result

this._setLoading(false);
this._setLoaded(true);
this._setError(false);

will never execute. But even if you add listener manually in <plastic-image> and set these values:

assignImgSrc() {
    ...
    if (!this.src || this.src !== this._selectedImgUrl) this.src = this._selectedImgUrl;
    this.preventLoad = false;
    this.$.img.addEventListener('load', e => {
        this.loading = false;
        this.loaded = true;
        this.error = false;
    });
    ...
}

observer _computePlaceholderClassName() will not be fired. As a result <div id="placeholder"> will never get class fade-out.

The quick fix I have found is to set listener manually as I did above and add appropriative class:

assignImgSrc() {
    ...
    if (!this.src || this.src !== this._selectedImgUrl) this.src = this._selectedImgUrl;
    this.preventLoad = false;
    this.$.img.addEventListener('load', e => {
        if (this.preload && this.fade) {
            this.$.placeholder.classList.add('faded-out');
        }
    });
    ...
}

Can anybody explain what is going on? :)

mlisook commented 7 years ago

Thank you for finding this issue.

I confirmed the issue in every variation I could think of. I also confirmed your observation that _imgOnLoad is not firing.

As to why ... I just don't understand, but it does have something to do with the template. If I modify plastic-image by cloning the iron-image template or by adding it in a string form, then plastic-image and iron-image seem to work in the same document.

class PlasticImage extends customElements.get('iron-image') {
            static get is() {
                return 'plastic-image';
            }
            // clone the iron-image template for issue 14 - no idea why this works
            static get template() {
                return customElements.get('iron-image').template.cloneNode(true);
            }

Although, in your thecatapi.com example the browser doesn't request a new image each time it is referenced.

I need to do more research, maybe the polymer slack channel can help me understand what is going on and why cloning the template works.

mlisook commented 7 years ago

I am closing this with the fix in PR #17 ... I've posted a question on polymer slack to understand why it works, but it does seem to. The new version, 1.0.5, is updated in bower.json and tagged. It also encompasses PR #15 merged earlier.