-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
Add Vine HTML component #1006
Add Vine HTML component #1006
Conversation
import {isLayoutSizeDefined} from '../../../src/layout'; | ||
import {loadPromise} from '../../../src/event-helper'; | ||
|
||
class AmpVine extends AMP.BaseElement { |
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.
add newline between class declaration an and first method.
Looks great! just some minor recommendations and will LGTM. |
@niallkennedy also feel free to add yourself to CONTRIBUTORS |
@erwinmombay outstanding requests should have been met with 4072da7 |
LGTM |
@cramforce should we be updating the validator in this PR too for the |
@erwinmombay I think we aren't there yet. Maybe ping @powdercloud on chat. |
@niallkennedy just one request, do you mind squashing this into 1 commit. thanks! will merge right after |
@cramforce my mistake, its already in the protoascii definition. |
@erwinmombay squashed into eb98861 |
Add an
amp-vine
custom element. Pause video through inactive callback.Includes example file, automated tests, and manual test file.
Intent to implement issue: #1005