amphtml
amphtml copied to clipboard
new 3q-player feature - a listener for 'resize' event
This PR adds a new feature to 3q-player, a listener for 'resize' event. It allows to resize content containers, which is needed for playlists, and it was inspired by already existing AMP method, amp-list. /cc @alanorozco
Hey @erwinmombay! These files were changed:
build-system/babel-plugins/OWNERS
build-system/eslint-rules/OWNERS
Hey @estherkim! These files were changed:
build-system/tasks/e2e/index.js
Hey @alanorozco! These files were changed:
build-system/tasks/storybook/index.js
Hey @danielrozenberg! These files were changed:
build-system/tasks/visual-diff/index.js
build-system/tasks/visual-diff/package-lock.json
build-system/tasks/visual-diff/package.json
Hey @ampproject/wg-amp4email! These files were changed:
extensions/amp-accordion/validator-amp-accordion.protoascii
extensions/amp-fit-text/validator-amp-fit-text.protoascii
extensions/amp-lightbox/validator-amp-lightbox.protoascii
extensions/amp-selector/validator-amp-selector.protoascii
extensions/amp-sidebar/validator-amp-sidebar.protoascii
Hey @ampproject/wg-caching! These files were changed:
extensions/amp-accordion/validator-amp-accordion.protoascii
extensions/amp-ad-exit/validator-amp-ad-exit.protoascii
extensions/amp-animation/validator-amp-animation.protoascii
extensions/amp-app-banner/validator-amp-app-banner.protoascii
extensions/amp-audio/validator-amp-audio.protoascii
extensions/amp-brightcove/validator-amp-brightcove.protoascii
extensions/amp-consent/validator-amp-consent.protoascii
extensions/amp-dailymotion/validator-amp-dailymotion.protoascii
extensions/amp-facebook-comments/validator-amp-facebook-comments.protoascii
extensions/amp-facebook-like/validator-amp-facebook-like.protoascii
extensions/amp-facebook-page/validator-amp-facebook-page.protoascii
extensions/amp-facebook/validator-amp-facebook.protoascii
+81 more
Hey @smart-adserver! These files were changed:
extensions/amp-ad-network-smartadserver-impl/0.1/amp-ad-network-smartadserver-impl.js
extensions/amp-ad-network-smartadserver-impl/0.1/test/test-amp-ad-network-smartadserver-impl.js
extensions/amp-ad-network-smartadserver-impl/amp-ad-network-smartadserver-impl-internal.md
Hey @gmajoulet! These files were changed:
extensions/amp-story-360/0.1/amp-story-360.css
extensions/amp-story-360/0.1/amp-story-360.js
extensions/amp-story-interactive/0.1/amp-story-interactive-abstract.js
extensions/amp-story-interactive/0.1/amp-story-interactive-slider.js
extensions/amp-story-interactive/0.1/test/test-amp-story-interactive-slider.js
extensions/amp-story-interactive/0.1/test/validator-amp-story-interactive-valid.html
extensions/amp-story-interactive/0.1/test/validator-amp-story-interactive-valid.out
extensions/amp-story-interactive/validator-amp-story-interactive.protoascii
extensions/amp-story-page-attachment/0.1/amp-story-draggable-drawer.js
extensions/amp-story-shopping/0.1/amp-story-shopping-tag.css
extensions/amp-story-shopping/0.1/amp-story-shopping.md
extensions/amp-story-shopping/0.1/test/test-amp-story-shopping-attachment.js
+29 more
Hey @processprocess, @t0mg! These files were changed:
extensions/amp-story-360/0.1/amp-story-360.css
extensions/amp-story-360/0.1/amp-story-360.js
Hey @mszylkowski! These files were changed:
extensions/amp-story-interactive/0.1/amp-story-interactive-abstract.js
extensions/amp-story-interactive/0.1/amp-story-interactive-slider.js
extensions/amp-story-interactive/0.1/test/test-amp-story-interactive-slider.js
extensions/amp-story-interactive/0.1/test/validator-amp-story-interactive-valid.html
extensions/amp-story-interactive/0.1/test/validator-amp-story-interactive-valid.out
extensions/amp-story-interactive/validator-amp-story-interactive.protoascii
extensions/amp-story-page-attachment/0.1/amp-story-draggable-drawer.js
Hey @newmuis! These files were changed:
extensions/amp-story-page-attachment/0.1/amp-story-draggable-drawer.js
extensions/amp-story-subscriptions/0.1/amp-story-subscriptions.css
extensions/amp-story-subscriptions/0.1/amp-story-subscriptions.js
extensions/amp-story-subscriptions/0.1/amp-story-subscriptions.md
extensions/amp-story-subscriptions/0.1/test/test-amp-story-subscriptions.js
extensions/amp-story-subscriptions/0.1/test/validator-amp-story-subscriptions.html
extensions/amp-story-subscriptions/0.1/test/validator-amp-story-subscriptions.out
extensions/amp-story-subscriptions/validator-amp-story-subscriptions.protoascii
extensions/amp-story/1.0/amp-story-desktop-one-panel.css
extensions/amp-story/1.0/amp-story-page.js
extensions/amp-story/1.0/amp-story-store-service.js
extensions/amp-story/1.0/amp-story-system-layer.css
+18 more
Hey @jpettitt! These files were changed:
extensions/amp-subscriptions-google/0.1/amp-subscriptions-google.js
extensions/amp-subscriptions-google/0.1/test/test-amp-subscriptions-google.js
extensions/amp-subscriptions-google/OWNERS
extensions/amp-subscriptions/OWNERS
extensions/amp-subscriptions/validator-amp-subscriptions.protoascii
third_party/subscriptions-project/OWNERS
third_party/subscriptions-project/config.js
third_party/subscriptions-project/swg-button.css
third_party/subscriptions-project/swg-gaa.js
third_party/subscriptions-project/swg.js
I've just signed the CLA
Best regards
@andreasschrottenbaum Unsure why the CLA check is still failing. It's likely that @aywar2000 needs to sign as well.
I've just signed the CLA
Best regards
Hey Alan, thanks for your time and reviews. I added the catch block.
Still getting formatting issues https://app.circleci.com/pipelines/github/ampproject/amphtml/22925/workflows/f0712bcc-0c5d-4127-add5-f59e63dd695e/jobs/457826
Dear Alan, thanks again for your time, I fixed the formatting issues. @danielrozenberg @rebeccanthomas, could you please review it, so we can merge it this week? Thanks!
Dear Alan, thanks again for your time, I fixed the formatting issues. @danielrozenberg @rebeccanthomas, could you please review it, so we can merge it this week? Thanks!
It looks like Rebecca and I were auto-added to this PR for an earlier revision (probably one with a large merge commit?) and it doesn't seem like any of the files that either of us are responsible for are being changed here. Alan's review should be enough here :)
It looks like the validator file was removed, so approval from "WG: Caching" is no longer needed.
Hey @danielrozenberg , hey @Gregable ; thanks for your time and for looking into the matter. It looks like I did not built AMP completely, which is why I had formatting errors on reviews. @alanorozco, I apologize. Will update the pull request tomorrow, so we can merge it.
@aywar2000 Apologies, CI is failing for unrelated reasons. Push an empty commit to re-run:
git commit --allow-empty -m "empty commit to trigger ci"
Hey @alanorozco, thanks for getting back. I've just pushed an empty commit.
Dear @alanorozco, I've just commited an unrelated fix to my amphtml repository that really should be merged as soon as possible. Could you please look into it?
I've just signed the CLA
Best regards
I've just signed the CLA
Best regards
I've just signed the CLA
Best regards
I've just signed the CLA
Best regards
Hello @danielrozenberg, can we please check the owners approvals and CLA license? Thanks for your time:)
It looks like you've added a lot of commits to this branch by a user that's not associated with a GitHub account (I assume that's you, just with commits signed using a different email address). My recommendation is to squash your branch* into a single commit, and making sure you use the same email address that you use on GitHub to sign these commits
*note that we use main
as the name our main branch, not master
It looks like you've added a lot of commits to this branch by a user that's not associated with a GitHub account (I assume that's you, just with commits signed using a different email address). My recommendation is to squash your branch* into a single commit, and making sure you use the same email address that you use on GitHub to sign these commits
*note that we use
main
as the name our main branch, notmaster
Thanks for the reply, will try your recommendation.
Hey @danielrozenberg, can you please help out? This is stalled here for ages, tried the slack channel, tried Stack Overflow, tried to write your colleagues from AMP, asked my senior colleagues in the company and this thing is not moving anywhere. What should I do, please?
It looks like you're changing package.json and package-lock.json unrelatedly to this PR, if you revert these files it would make it so that you won't need my approval. Perhaps @alanorozco can help with getting approval from the caching team for the other file
Hey @alanorozco @danielrozenberg squashed all the commits as advised, which resulted in about 315 files changed, and some of it were validator- files. I had to change one validator file to get the PR to pass CircleCi check. As i pulled changes from AMP branch, it caused changes to package- files, including .json. If I try to revert now, there are merge conflicts for 315 files. If I try to solve merge conflicts in the reverted commit, I have to do the npm install to run tests. I can't run tests because I have to have package.json updated to run npm install, and if I update the package.json so I can run npm install, I can't revert the commit and therefore the problems persist.
Would it make sense just to move away from all this mess and to open a new PR request?
Perhaps opening a new PR will make things easier. Make sure you work on a branch with another name (not main
) so it'll be easy to merge changes from this repository and your own fork