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.

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

Fix immersive media atom template by using named parameter for mediaWrapper #16157

Merged
merged 1 commit into from Mar 16, 2017

Conversation

gidsg
Copy link
Contributor

@gidsg gidsg commented Mar 16, 2017

What does this change?

Previously we were passing unnamed parameters in the YouTube template. This meant the mediaWrapper wasn't being correctly set I think because the compiler thought it to be a different parameter.

What is the value of this and can you measure success?

Fixes immersive template

Does this affect other platforms - Amp, Apps, etc?

No

Screenshots

Before

screen shot 2017-03-16 at 10 33 37

After

screen shot 2017-03-16 at 10 35 50

Tested in CODE?

No

CC @duarte

@gidsg gidsg changed the title Fix immersive by using named parameter for mediaWrapper Fix immersive media atom template by using named parameter for mediaWrapper Mar 16, 2017
@duarte
Copy link
Contributor

duarte commented Mar 16, 2017

Thanks Gids!

@PRBuilds
Copy link

PR build results:

screenshots
mobile.pngwide.pngtablet.pngdesktop.png

exceptions (0)
thrown-exceptions.js

webpagetest (2)
performanceComparisonSummary.txt

-automated message

@prout-bot
Copy link
Collaborator

Seen on PROD (merged by @gidsg 22 minutes and 38 seconds ago) Please check your changes!

@gu-stav gu-stav deleted the fix-immersive branch June 28, 2017 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants