Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add amp-audio support #65

Merged
merged 5 commits into from
Sep 11, 2015
Merged

Add amp-audio support #65

merged 5 commits into from
Sep 11, 2015

Conversation

rgthree
Copy link
Contributor

@rgthree rgthree commented Sep 11, 2015

No description provided.

@cramforce
Copy link
Member

Nice! Would you mind moving this into extensions/
This should be a really easy conversion. Sample commit 3863293

I think this is sufficiently rare to be good for opt-in.

cramforce
cramforce reviewed Sep 11, 2015
View reviewed changes
src/amp-audio.js
* browser implementations.
* @return {Dimensions}
*/
function getBrowserAudioDefaultDimensions() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool

@rgthree
Move audio to /extensions
cce518a 
Also, use 'loadstart' event for AUDIO/VIDEO media in 'loadPromise'
@rgthree
Merge pull request #2 from rgthree/audio
9d1826c 
Move audio to /extensions
@rgthree
Copy link
Contributor Author

rgthree commented Sep 11, 2015

Cool.

  • Moved amp-audio to /extensions.
  • Added https check for both audio or nested source elements.
  • Return loadPromise(audio) in AmpAudio#layoutCallback.
  • Added loadstart event for AUDIO/VIDEO elements within loadPromise to resolve once a media element has started loading. (Or, canplaythrough would be the audio/video load equivalent).

@cramforce
Copy link
Member

LGTM

cramforce added a commit that referenced this pull request Sep 11, 2015
@cramforce
Merge pull request #65 from rgthree/master
90ab32d 
Add amp-audio support
@cramforce cramforce merged commit 90ab32d into ampproject:master Sep 11, 2015
@erwinmombay erwinmombay mentioned this pull request Sep 11, 2015
Closed
@uditalias uditalias mentioned this pull request Nov 4, 2022
Open
@Elisabeth1983 Elisabeth1983 mentioned this pull request Nov 4, 2022
Open
@Masterci84 Masterci84 mentioned this pull request Nov 5, 2022
Open
@muratkalenderoglu muratkalenderoglu mentioned this pull request Nov 5, 2022
Open
@shmoop207 shmoop207 mentioned this pull request Nov 15, 2022
Open
@Elisabeth1983 Elisabeth1983 mentioned this pull request Nov 15, 2022
Open
@Masterci84 Masterci84 mentioned this pull request Nov 16, 2022
Open
@muratkalenderoglu muratkalenderoglu mentioned this pull request Nov 16, 2022
Open
@shmoop207 shmoop207 mentioned this pull request Nov 28, 2022
Open
@Elisabeth1983 Elisabeth1983 mentioned this pull request Nov 28, 2022
Open
@Masterci84 Masterci84 mentioned this pull request Nov 28, 2022
Open
@muratkalenderoglu muratkalenderoglu mentioned this pull request Nov 29, 2022
Open
@Elisabeth1983 Elisabeth1983 mentioned this pull request Dec 25, 2022
Open
@shmoop207 shmoop207 mentioned this pull request Dec 25, 2022
Open
@Elisabeth1983 Elisabeth1983 mentioned this pull request Dec 25, 2022
Open
@Masterci84 Masterci84 mentioned this pull request Dec 25, 2022
Open
@muratkalenderoglu muratkalenderoglu mentioned this pull request Dec 26, 2022
Open
@Elisabeth1983 Elisabeth1983 mentioned this pull request Oct 14, 2023
Open
@Elisabeth1983 Elisabeth1983 mentioned this pull request May 7, 2024
Open
This was referenced May 7, 2024
Open
Open
This was referenced May 9, 2024
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rgthree @cramforce @erwinmombay