Skip to content

🐛Added Mute/Unmute Controls to <amp-ima-video>#18611

Merged
torch2424 merged 3 commits intoampproject:masterfrom
torch2424:ima-video-mute-18531
Oct 9, 2018
Merged

🐛Added Mute/Unmute Controls to <amp-ima-video>#18611
torch2424 merged 3 commits intoampproject:masterfrom
torch2424:ima-video-mute-18531

Conversation

@torch2424
Copy link
Copy Markdown
Contributor

@torch2424 torch2424 commented Oct 8, 2018

closes #18531

This adds mute/unmute controls to <amp-ima-video> 😄 Also, does some updating to the <amp-ima-video> that makes it usable on desktop, and offers more context on ima, and it's example implementation.

Examples

imavideomute

@torch2424
Copy link
Copy Markdown
Contributor Author

Going to look for an IMA team member to give this a review 😄

@zhouyx
Copy link
Copy Markdown
Contributor

zhouyx commented Oct 8, 2018

Should we put the mute button on the left side or right side?

@torch2424
Copy link
Copy Markdown
Contributor Author

IMA team doesn't have bandwidth, and @spacedino approved of the media button on the right side 😄

cc @zhouyx

@zhouyx
Copy link
Copy Markdown
Contributor

zhouyx commented Oct 9, 2018

One more request, can I see the UI in mobile? Otherwise looks awesome! Thank you for digging into the code and let's ship it!

Copy link
Copy Markdown
Contributor

@zhouyx zhouyx left a comment

Choose a reason for hiding this comment

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

LGTM!!

@torch2424
Copy link
Copy Markdown
Contributor Author

Showed Mobile UI in person to @zhouyx

Thanks for the LGTM Merging 😄

@torch2424 torch2424 merged commit 094cb1d into ampproject:master Oct 9, 2018
@torch2424 torch2424 deleted the ima-video-mute-18531 branch October 9, 2018 23:32
noranazmy pushed a commit to noranazmy/amphtml that referenced this pull request Oct 18, 2018
* Fixed the ima video example

* Finished the Mute/UnMute

* Fixed all testing and liniting errors
Enriqe pushed a commit to Enriqe/amphtml that referenced this pull request Nov 28, 2018
* Fixed the ima video example

* Finished the Mute/UnMute

* Fixed all testing and liniting errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

amp-ima-video: Missing Mute/Unmute Controls

4 participants