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
♻️ amp-viqeo-player: placeholder and refactoring #19885
♻️ amp-viqeo-player: placeholder and refactoring #19885
Conversation
kinoshnik2070
commented
Dec 14, 2018
•
edited
edited
- create placeholder
- refactoring
- create placeholder - remove autoplay attribute and add noautoplay - refactoring
This is considered a breaking change due to |
@kimhogeling per @honeybadgerdontcare comment and per AMP's autoplay policy, your video player should use AMP's builtin autoplay feature which is based on the opt-in |
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.
@kinoshnik2070 Couple of small requests, we can merge after addressing them. Thank you
placeholder.setAttribute('layout', 'fill'); | ||
placeholder.setAttribute('placeholder', ''); | ||
placeholder.setAttribute('referrerpolicy', 'origin'); | ||
placeholder.setAttribute('alt', 'Loading video'); |
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 is overriding alt again, I will fix it and merge, no action needed from you.
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.
thanks
@kinoshnik2070 merged. Thanks! |