Opened 11 years ago
Closed 10 years ago
#29146 closed defect (bug) (fixed)
Audio/Video in Twenty Thirteen are lacking bottom margin
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 4.1 | Priority: | normal |
Severity: | normal | Version: | 3.6 |
Component: | Bundled Theme | Keywords: | good-first-bug has-patch commit |
Focuses: | ui | Cc: |
Description
This is probably a regression, just want to track with a ticket.
Posts that have audio and video: the comment link is too close to the media.
Attachments (10)
Change History (34)
#3
@
11 years ago
Right, with an embed as the last element that happens to not be wrapped in a paragraph.
#5
@
11 years ago
I was testing the MediaElement upgrade (still hasn't landed) in the past 5 themes, my notes tell me that 2013 was the only problem. I then noticed that the problem persisted without the upgrade.
#6
in reply to:
↑ description
@
11 years ago
I was not able to replicate this issue. Above is a screenshot using a video from WordPress.tv as a standard post and a video post format. I also embedded a video from Youtube with similar results.
#7
@
11 years ago
I could replicate it when using a [video]
or [audio]
shortcode. YouTube oEmbed isn't impacted since it places the video within <p>
tags.
The <div>
tags for the embeds are what's lacking.
#8
@
11 years ago
@mikeyarce - could you update the patch to be relative to root? (e.g. for src/wp-content/themes/twentythirteen/style.css instead of simply style.css)
Makes it easier to test (e.g. grunt patch:29146
would be able to apply the patch). :)
#9
@
11 years ago
- Keywords has-patch added; needs-patch removed
With 29146.1,
[video]
https://cloudup.com/ct7NV7hax96
[audio]
https://cloudup.com/cTzq2pVGgnY
#10
@
10 years ago
Helped a few of my colleagues navigate this ticket - the CSS rules need to be scoped to standard post format when the audio or video shortcode is not wrapped in a <p>
#11
@
10 years ago
Looking at 29146.3.diff:
We currently don't have a Post Format section for standard post formats in style.css
, so we should probably add that and combine the styles. Does the selector have to be that specific? Wouldn't .format-standard .wp-video
suffice?
I also wonder why is this only a bug in Twenty Thirteen, how do other default themes handle that scenario?
This ticket was mentioned in IRC in #wordpress-dev by helen. View the logs.
10 years ago
#13
@
10 years ago
- Milestone changed from 4.0 to Future Release
- Version changed from trunk to 3.6
Let's look at it in 4.1 again.
#14
@
10 years ago
I'm currently researching obenland's question in comment comment:11, and discovered that patch 29146.3 wasn't applying, so I fixed it.
#15
@
10 years ago
In response to comment:11:
I imported theme-unit-test.xml
into the developer install, and checked the default themes as released with 4.0.
The table below shows whether the elements have a bottom margin applied by the style.
twentyten | twentyeleven | twentytwelve | twentythirteen | twentyfourteen | |
---|---|---|---|---|---|
Post Format: Audio | no | no | yes | yes | yes |
Post Format: Video (WordPress.TV) | yes | no | yes | no | - |
Post Format: Video (VideoPress) | - | - | no | no | no |
Post Format: Video (YouTube) | yes | yes | yes | yes | yes |
-
Theme does not support the underlying shortcode, even with Jetpack installed
#17
@
10 years ago
@kraftbj and I will be looking at this ticket in the next week and putting together the final patches for all default themes.
#18
@
10 years ago
Is this a themes issue though, or an issue with the different ways that videos are output?
#19
@
10 years ago
@obenland - Not totally sure. We'll check that out. Either way, aiming to have something to test in time for Beta 1.
@
10 years ago
Fixes bottom margin in Twenty Ten, Twenty Eleven, and Twenty Thirteen for the Video Player, VideoPress, and Audio Player elements in the Standard Post Format. Tested all default themes up to Twenty Fifteen - all other themes either have top margin in the footer .entry-meta or aren't affected.
#20
@
10 years ago
- Keywords needs-testing added
- Milestone changed from Future Release to 4.1
If another person could apply latest patch and test -- and report back, we can get this in.
#21
@
10 years ago
applied 29146.6.diff and appears to be working in TwentyTen, TwentyEleven and TwentyThriteen for both video and audio players. I tested using the theme unit test data for post lists and video and audio post formats.
The above shows how it looks to me. Could you share a screenshot of how it looks on your end?