User Story

As a programmer/themer
I should be able to set the src, title, and fallback text for an AMP video manually. Not all video services use the same URL formatting. (Ex. MPX)

Expected Result

The values I pass into the theme function are rendered into the video template.

Actual Result

These values are hard set in amp_preprocess_amp_video(), overriding any settings passed in via the theme hook and thus ignored.

Proposed Solution

Alter the preprocessor so values are only overridden if they are empty, thus allowing custom values to be passed in via the theme call. Some functionality should be moved to a hook_process() to ensure they run after any custom preprocessors have been called.

CommentFileSizeAuthor
#3 amp-video_theme_overrides-2927191-3.patch2.75 KBKeyboardCowboy

Comments

KeyboardCowboy created an issue. See original summary.

KeyboardCowboy’s picture

KeyboardCowboy
Primary language English
Location Denver, CO, USA
commented
Issue summary: View changes
KeyboardCowboy’s picture

KeyboardCowboy
Primary language English
Location Denver, CO, USA
commented
StatusFileSize
new amp-video_theme_overrides-2927191-3.patch2.75 KB

Adding patch to:

  • Remove theme hook variables for video_attributes (rendered).
  • Add theme hook variables for src, fallback text.
  • Adjust preprocessor to allow SRC to be passed as a variable.
  • Moved fallback text into preprocessor/theme.
  • Moved rendering of video_attributes string into hook_process() callback.
KeyboardCowboy’s picture

KeyboardCowboy
Primary language English
Location Denver, CO, USA
commented
Status: Active » Needs review

quicksketch’s picture

quicksketch
he/him
commented
Status: Needs review » Fixed

This looks great on all counts. Committed to 7.x-1.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.