-
Notifications
You must be signed in to change notification settings - Fork 43
Fixes issues with placeholder state and url; adds error property. #44
Changes from 10 commits
ebac79f
ef71fef
3e4fb02
57c6888
4c5e605
b27af65
0f9633e
c87d9fe
0b1acb9
11f680e
e49694e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -79,12 +79,21 @@ | |
position: relative; | ||
} | ||
|
||
#sizedImgDiv { | ||
display: none; | ||
@apply(--layout-fit); | ||
} | ||
|
||
#img { | ||
display: block; | ||
width: var(--iron-image-width, auto); | ||
height: var(--iron-image-height, auto); | ||
} | ||
|
||
:host([sizing]) #sizedImgDiv { | ||
display: block; | ||
} | ||
|
||
:host([sizing]) #img { | ||
display: none; | ||
} | ||
|
@@ -105,9 +114,15 @@ | |
|
||
<template> | ||
|
||
<img id="img" role="none" hidden$="[[_computeImageVisibility(sizing)]]"> | ||
<div id="placeholder" hidden$="[[_computePlaceholderVisibility(fade,loaded,preload)]]" class$="[[_computePlaceholderClassName(fade,loaded,preload)]]"></div> | ||
<content></content> | ||
<div id="sizedImgDiv" | ||
role="img" | ||
hidden$="[[_computeImgDivHidden(sizing)]]" | ||
aria-hidden$="[[_computeImgDivARIAHidden(alt)]]" | ||
aria-label$="[[_computeImgDivARIALabel(alt, src)]]"></div> | ||
<img id="img" alt$="[[alt]]" hidden$="[[_computeImgHidden(sizing)]]"> | ||
<div id="placeholder" | ||
hidden$="[[_computePlaceholderHidden(preload, fade, loading, loaded)]]" | ||
class$="[[_computePlaceholderClassName(preload, fade, loading, loaded)]]"></div> | ||
|
||
</template> | ||
|
||
|
@@ -125,14 +140,23 @@ | |
value: '' | ||
}, | ||
|
||
/** | ||
* A short text alternative for the image. | ||
*/ | ||
alt: { | ||
type: String, | ||
value: null | ||
}, | ||
|
||
/** | ||
* When true, the image is prevented from loading and any placeholder is | ||
* shown. This may be useful when a binding to the src property is known to | ||
* be invalid, to prevent 404 requests. | ||
*/ | ||
preventLoad: { | ||
type: Boolean, | ||
value: false | ||
value: false, | ||
observer: '_preventLoadChanged' | ||
}, | ||
|
||
/** | ||
|
@@ -143,7 +167,8 @@ | |
*/ | ||
sizing: { | ||
type: String, | ||
value: null | ||
value: null, | ||
reflectToAttribute: true | ||
}, | ||
|
||
/** | ||
|
@@ -170,7 +195,8 @@ | |
*/ | ||
placeholder: { | ||
type: String, | ||
value: null | ||
value: null, | ||
observer: '_placeholderChanged' | ||
}, | ||
|
||
/** | ||
|
@@ -187,6 +213,7 @@ | |
*/ | ||
loaded: { | ||
notify: true, | ||
readOnly: true, | ||
type: Boolean, | ||
value: false | ||
}, | ||
|
@@ -197,6 +224,17 @@ | |
*/ | ||
loading: { | ||
notify: true, | ||
readOnly: true, | ||
type: Boolean, | ||
value: false | ||
}, | ||
|
||
/** | ||
* Read-only value that indicates that the last set `src` failed to load. | ||
*/ | ||
error: { | ||
notify: true, | ||
readOnly: true, | ||
type: Boolean, | ||
value: false | ||
}, | ||
|
@@ -224,70 +262,91 @@ | |
type: Number, | ||
value: null | ||
}, | ||
|
||
_placeholderBackgroundUrl: { | ||
type: String, | ||
computed: '_computePlaceholderBackgroundUrl(preload,placeholder)', | ||
observer: '_placeholderBackgroundUrlChanged' | ||
}, | ||
|
||
requiresPreload: { | ||
type: Boolean, | ||
computed: '_computeRequiresPreload(preload,loaded)' | ||
}, | ||
|
||
canLoad: { | ||
type: Boolean, | ||
computed: '_computeCanLoad(preventLoad, src)' | ||
} | ||
}, | ||
|
||
observers: [ | ||
'_transformChanged(sizing, position)', | ||
'_loadBehaviorChanged(canLoad, preload, loaded)', | ||
'_loadStateChanged(src, preload, loaded)', | ||
'_transformChanged(sizing, position)' | ||
], | ||
|
||
ready: function() { | ||
if (!this.hasAttribute('role')) { | ||
this.setAttribute('role', 'img'); | ||
var img = this.$.img; | ||
|
||
img.onload = function() { | ||
if (this.$.img.src !== this._resolveSrc(this.src)) return; | ||
|
||
this._setLoading(false); | ||
this._setLoaded(true); | ||
this._setError(false); | ||
}.bind(this); | ||
|
||
img.onerror = function() { | ||
if (this.$.img.src !== this._resolveSrc(this.src)) return; | ||
|
||
this._reset(); | ||
|
||
this._setLoading(false); | ||
this._setLoaded(false); | ||
this._setError(true); | ||
}.bind(this); | ||
|
||
this._resolvedSrc = ''; | ||
}, | ||
|
||
_load: function(src) { | ||
if (src) { | ||
this.$.img.src = src; | ||
} else { | ||
this.$.img.removeAttribute('src'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this superior to assigning some kind of empty value, like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A combination of a couple of things: setting There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Funnnn.. SGTM |
||
} | ||
this.$.sizedImgDiv.style.backgroundImage = src ? 'url("' + src + '")' : ''; | ||
|
||
this._setLoading(true); | ||
this._setLoaded(false); | ||
this._setError(false); | ||
}, | ||
|
||
_computeImageVisibility: function() { | ||
return !!this.sizing; | ||
_reset: function() { | ||
this.$.img.removeAttribute('src'); | ||
this.$.sizedImgDiv.style.backgroundImage = ''; | ||
|
||
this._setLoading(false); | ||
this._setLoaded(false); | ||
this._setError(false); | ||
}, | ||
|
||
_computePlaceholderVisibility: function() { | ||
return !this.preload || (this.loaded && !this.fade); | ||
_computePlaceholderHidden: function() { | ||
return !this.preload || (!this.fade && !this.loading && this.loaded); | ||
}, | ||
|
||
_computePlaceholderClassName: function() { | ||
if (!this.preload) { | ||
return ''; | ||
} | ||
return (this.preload && this.fade && !this.loading && this.loaded) ? 'faded-out' : ''; | ||
}, | ||
|
||
if (this.loaded && this.fade) { | ||
return 'faded-out'; | ||
} | ||
_computeImgDivHidden: function() { | ||
return !this.sizing; | ||
}, | ||
|
||
return ''; | ||
_computeImgDivARIAHidden: function() { | ||
return this.alt === '' ? 'true' : undefined; | ||
}, | ||
|
||
_computePlaceholderBackgroundUrl: function() { | ||
if (this.preload && this.placeholder) { | ||
return 'url(' + this.placeholder + ')'; | ||
_computeImgDivARIALabel: function() { | ||
if (this.alt !== null) { | ||
return this.alt; | ||
} | ||
|
||
return null; | ||
}, | ||
// Polymer.ResolveUrl.resolveUrl will resolve '' relative to a URL x to | ||
// that URL x, but '' is the default for src. | ||
if (this.src === '') { | ||
return ''; | ||
} | ||
|
||
_computeRequiresPreload: function() { | ||
return this.preload && !this.loaded; | ||
var pathComponents = (new URL(this._resolveSrc(this.src))).pathname.split("/"); | ||
return pathComponents[pathComponents.length - 1]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fancy 💎 |
||
}, | ||
|
||
_computeCanLoad: function() { | ||
return Boolean(!this.preventLoad && this.src); | ||
_computeImgHidden: function() { | ||
return !!this.sizing; | ||
}, | ||
|
||
_widthChanged: function() { | ||
|
@@ -298,64 +357,48 @@ | |
this.style.height = isNaN(this.height) ? this.height : this.height + 'px'; | ||
}, | ||
|
||
_preventLoadChanged: function() { | ||
if (this.preventLoad || this.loaded) return; | ||
|
||
this._reset(); | ||
this._load(this.src); | ||
}, | ||
|
||
_srcChanged: function(newSrc, oldSrc) { | ||
if (newSrc !== oldSrc) { | ||
this.loaded = false; | ||
var newResolvedSrc = this._resolveSrc(newSrc); | ||
if (newResolvedSrc === this._resolvedSrc) return; | ||
this._resolvedSrc = newResolvedSrc; | ||
|
||
this._reset(); | ||
if (!this.preventLoad) { | ||
this._load(newSrc); | ||
} | ||
}, | ||
|
||
_placeholderBackgroundUrlChanged: function() { | ||
_placeholderChanged: function() { | ||
this.$.placeholder.style.backgroundImage = | ||
this._placeholderBackgroundUrl; | ||
this.placeholder ? 'url("' + this.placeholder + '")' : ''; | ||
}, | ||
|
||
_transformChanged: function() { | ||
var sizedImgDivStyle = this.$.sizedImgDiv.style; | ||
var placeholderStyle = this.$.placeholder.style; | ||
|
||
this.style.backgroundSize = | ||
placeholderStyle.backgroundSize = this.sizing; | ||
sizedImgDivStyle.backgroundSize = | ||
placeholderStyle.backgroundSize = | ||
this.sizing; | ||
|
||
this.style.backgroundPosition = | ||
placeholderStyle.backgroundPosition = | ||
sizedImgDivStyle.backgroundPosition = | ||
placeholderStyle.backgroundPosition = | ||
this.sizing ? this.position : ''; | ||
|
||
this.style.backgroundRepeat = | ||
placeholderStyle.backgroundRepeat = | ||
sizedImgDivStyle.backgroundRepeat = | ||
placeholderStyle.backgroundRepeat = | ||
this.sizing ? 'no-repeat' : ''; | ||
}, | ||
|
||
_loadBehaviorChanged: function() { | ||
var img; | ||
|
||
if (!this.canLoad) { | ||
return; | ||
} | ||
|
||
if (this.requiresPreload) { | ||
img = new Image(); | ||
img.src = this.src; | ||
|
||
this.loading = true; | ||
|
||
img.onload = function() { | ||
this.loading = false; | ||
this.loaded = true; | ||
}.bind(this); | ||
} else { | ||
this.loaded = true; | ||
} | ||
}, | ||
|
||
_loadStateChanged: function() { | ||
if (this.requiresPreload) { | ||
return; | ||
} | ||
|
||
if (this.sizing) { | ||
this.style.backgroundImage = this.src ? 'url(' + this.src + ')': ''; | ||
} else { | ||
this.$.img.src = this.src || ''; | ||
} | ||
_resolveSrc: function(testSrc) { | ||
return Polymer.ResolveUrl.resolveUrl(testSrc, this.ownerDocument.baseURI); | ||
} | ||
}); | ||
</script> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes me very nervous, so thanks for pointing it out............... 😷
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's ship this, but be ready to back it out if someone complains.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, if we consider undocumented properties as having undefined behavior, this PR is technically backwards compatible.. :P
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Always look on the bright side ;)