It would be nice if there is an extra class that indicates the provider of the video. I have created Video Embed HTML5 which integrates this module with HTML5 videos.

When I use the colorbox formatter following css is applied

.video-embed-field-responsive-video:after {
  content: '';
  display: block;
  padding-bottom: 56.25%;
}

But for a video-tag this is unwanted behaviour. I would like to do something like this in my own module:

.video-embed-field-responsive-video.html5:after {
  content: none;
}
CommentFileSizeAuthor
#10 interdiff.txt22.87 KBsam152
#10 video_embed_field-provider_class-2870151-10.patch22.98 KBsam152
#5 video_embed_field-provider_class-2870151-5.patch10.07 KBrobin.ingelbrecht
#3 video_embed_field-provider_class-2870151-3.patch2.68 KBrobin.ingelbrecht

Comments

robin.ingelbrecht created an issue. See original summary.

robin.ingelbrecht’s picture

robin.ingelbrecht commented
Issue summary: View changes
robin.ingelbrecht’s picture

robin.ingelbrecht commented
Status: Active » Needs review
StatusFileSize
new video_embed_field-provider_class-2870151-3.patch2.68 KB

This simple patch does the trick.

Status: Needs review » Needs work

The last submitted patch, 3: video_embed_field-provider_class-2870151-3.patch, failed testing.

robin.ingelbrecht’s picture

robin.ingelbrecht commented
StatusFileSize
new video_embed_field-provider_class-2870151-5.patch10.07 KB

Forgot to alter the tests. New patch

robin.ingelbrecht’s picture

robin.ingelbrecht commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 5: video_embed_field-provider_class-2870151-5.patch, failed testing.

sam152’s picture

sam152 commented

I like the idea, but NW for red tests.

robin.ingelbrecht’s picture

robin.ingelbrecht commented

I know. Could tou find the time to have look why tests are failing? For some reason my local dev environment won't run the tests... so I can't debug them

sam152’s picture

sam152 commented
Status: Needs work » Needs review
StatusFileSize
new video_embed_field-provider_class-2870151-10.patch22.98 KB
new interdiff.txt22.87 KB

Updated to fix tests. Adding the class regardless of the responsive setting. What the test fixes make painfully obvious is that this is a slightly backward incompatible change. It adds an extra div situations where it wasn't previously present.

robin.ingelbrecht’s picture

robin.ingelbrecht commented
Status: Needs review » Reviewed & tested by the community

Applied patch and seems to work fine! I think this one is ready to be committed?

sam152’s picture

sam152 commented

I think I'll have to do some more manual testing to make sure the markup change isn't likely to cause issues with existing layouts. Also, not sure why the tests didn't run.

sam152’s picture

sam152 commented
Issue tags: +Needs change record
robin.ingelbrecht’s picture

robin.ingelbrecht commented

Any progress on this issue Sam? Thanks!

sam152’s picture

sam152 commented

Just need to figure out why we haven't got a green test run and create the change record. Should find some time this week.

robin.ingelbrecht’s picture

robin.ingelbrecht commented

Well, I have aded a few patches this week an all the test keep hanging on "Waiting for branch to pass"... maybe a problem with the test servers? Dunno...

sam152’s picture

sam152 commented
Status: Reviewed & tested by the community » Fixed

Sorry for the hold up on this one. Fixed. A change record might be good for this issue, but the decider was the impact of the extra markup on BC is probably fairly minimal given it was for non-responsive videos only and the extra markup actually makes things more consistent when switching between these two settings.

Status: Fixed » Closed (fixed)

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