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

Add support for header videos #1078

Closed
westonruter opened this issue Apr 17, 2018 · 4 comments · Fixed by #1074
Closed

Add support for header videos #1078

westonruter opened this issue Apr 17, 2018 · 4 comments · Fixed by #1074
Labels
Milestone

Comments

@westonruter
Copy link
Member

westonruter commented Apr 17, 2018

In Twenty Seventeen there is a header video feature: https://make.wordpress.org/core/2016/11/26/video-headers-in-4-7/

image

This is not working in Twenty Seventeen when the amp theme support is added. It is just shown as blank (as with header image in the theme):

image

@kienstra
Copy link
Contributor

kienstra commented May 16, 2018

Request To Work On Issue

Hi @DavidCramer,
Would you mind if I worked on this?

@DavidCramer
Copy link
Contributor

@kienstra - go for it!

@kienstra
Copy link
Contributor

kienstra commented Jun 5, 2018

Steps To Test

Hi @csossi,
Could you please test this?

  1. Visit this staging site. Your login details will be the same as other AMP staging sites like this, but feel free to Slack me if they don't work.
  2. It currently has a YouTube URL as the header video, but there's a header image on mobile displays
  3. Ensure that display well, without obvious styling issues
  4. In the Customizer, click "Select video" to choose a different video:

header-video

5. Ensure that the video displays well on the front-end, and that it doesn't display on mobile devices

@kienstra kienstra assigned csossi and unassigned DavidCramer and kienstra Jun 5, 2018
@csossi
Copy link

csossi commented Jun 7, 2018

verified in QA

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 a pull request may close this issue.

6 participants