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
✨Support <amp-delight-player dock> #20913
Conversation
const placeholder = this.element.ownerDocument.createElement('div'); | ||
const html = htmlFor(this.element); | ||
const placeholder = | ||
html`<div placeholder><amp-img layout=fill></amp-img></div>`; |
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.
@aghassemi Even though this makes it technically get the correct placeholder URL, the src
pattern below appears to be broken :(
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.
@xymw has the Url for poster changed?
@@ -1708,7 +1708,6 @@ export class VideoDocking { | |||
this.getControls_().reset(); | |||
|
|||
[ | |||
element, |
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.
Caught this while manually testing. Same as #20912.
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.
validation looks good
extensions/amp-video-docking/0.1/test/validator-amp-video-docking-amp-delight-player.html
Outdated
Show resolved
Hide resolved
const placeholder = this.element.ownerDocument.createElement('div'); | ||
const html = htmlFor(this.element); | ||
const placeholder = | ||
html`<div placeholder><amp-img layout=fill></amp-img></div>`; |
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.
@xymw has the Url for poster changed?
- cl/234811475 Revision bump for #20913 - cl/234805704 Revision bump for #20911 - cl/234655757 allow render_only_if_payment_method_present for amp-payment-google-button - cl/234162273 Update comment regarding id/name attribute lists - cl/234053286 Update tagspecs using attribute `name` to include blacklist - cl/234038899 Update tagspecs using attribute `id` to include blacklist - cl/234023532 Revision bump for #20772
- cl/234811475 Revision bump for ampproject#20913 - cl/234805704 Revision bump for ampproject#20911 - cl/234655757 allow render_only_if_payment_method_present for amp-payment-google-button - cl/234162273 Update comment regarding id/name attribute lists - cl/234053286 Update tagspecs using attribute `name` to include blacklist - cl/234038899 Update tagspecs using attribute `id` to include blacklist - cl/234023532 Revision bump for ampproject#20772
- cl/234811475 Revision bump for ampproject#20913 - cl/234805704 Revision bump for ampproject#20911 - cl/234655757 allow render_only_if_payment_method_present for amp-payment-google-button - cl/234162273 Update comment regarding id/name attribute lists - cl/234053286 Update tagspecs using attribute `name` to include blacklist - cl/234038899 Update tagspecs using attribute `id` to include blacklist - cl/234023532 Revision bump for ampproject#20772
No description provided.