WordPress.org

Make WordPress Core

Opened 6 months ago

Closed 10 days ago

#44664 closed defect (bug) (fixed)

Twenty Fourteen: Video play icon alignment

Reported by: celloexpressions Owned by: SergeyBiryukov
Milestone: 5.0.3 Priority: normal
Severity: normal Version: 4.9
Component: Bundled Theme Keywords: good-first-bug has-patch has-screenshots needs-testing commit fixed-major
Focuses: ui Cc:

Description

Twenty Fourteen provides custom styling for the MediaElement.js players. It appears that the updates to MediaElement.js in a recent release caused the play icon (which is a Genericons font icon) in embedded video players to become substantially misaligned. This could be seen in the editor or in a video widget with a locally-uploaded video.

This seems to be fixable by adding position: relative to .hentry .mejs-overlay-button, .widget .mejs-overlay-button { in the Twenty Fourteen stylesheet. This allows the icon to be absolutely positioned relative to the button container, as this worked previously.

If someone has time to verify this issue, document screenshots, and turn this into a patch, this should be a relatively straightforward fix.

Attachments (7)

44664.diff (192.1 KB) - added by chriseverson 6 months ago.
video_icon_alignment.diff (536 bytes) - added by mmaumio 6 months ago.
Removed the margin from the .mejs-overlay-button for both hentry and widget as it was misaligning the icon from being centered vertically and horizontally. Changed the icon position to relative instead of absolute which the issue creator advised as well.
before-icon-alignment-fix.png (205.7 KB) - added by mmaumio 6 months ago.
Before Icon Alignment Fix
after-icon-alignment-fix.png (196.7 KB) - added by mmaumio 6 months ago.
After the fix
44664.3.patch (1.2 KB) - added by laurelfulford 5 weeks ago.
Copy fix editor styles.
twenty-fourteen-editor-screenshot-before.png (156.2 KB) - added by laurelfulford 5 weeks ago.
Twenty Fourteen - video in classic editor before.
twenty-fourteen-editor-screenshot-after.png (156.3 KB) - added by laurelfulford 5 weeks ago.
Twenty Fourteen - video in classic editor after.

Download all attachments as: .zip

Change History (23)

@chriseverson
6 months ago

#1 @chriseverson
6 months ago

I was able to verify this as an issue. The above patch has the suggested fix, which solves things on the frontend, but will still need updates for the editor. Hoping to have those inbound shortly.

@mmaumio
6 months ago

Removed the margin from the .mejs-overlay-button for both hentry and widget as it was misaligning the icon from being centered vertically and horizontally. Changed the icon position to relative instead of absolute which the issue creator advised as well.

@mmaumio
6 months ago

Before Icon Alignment Fix

@mmaumio
6 months ago

After the fix

#2 @mmaumio
6 months ago

  • Keywords has-patch has-screenshots added; needs-patch needs-screenshots removed

This ticket was mentioned in Slack in #core-themes by mmaumio. View the logs.


6 months ago

#4 @pento
3 months ago

  • Milestone changed from 4.9.9 to 5.0.1

#5 @celloexpressions
3 months ago

Twenty Fourteen is already being updated in 5.0 for Gutenberg, and will likely be touching this code anyway. We need to be better about grouping bundled theme updates - @laurelfulford do you think you can take care of this very small adjustment with the other 5.0 changes?

This ticket was mentioned in Slack in #core by sergey. View the logs.


7 weeks ago

#7 @SergeyBiryukov
7 weeks ago

  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#8 @pento
5 weeks ago

  • Milestone changed from 5.0.1 to 5.0.2

@laurelfulford
5 weeks ago

Copy fix editor styles.

@laurelfulford
5 weeks ago

Twenty Fourteen - video in classic editor before.

@laurelfulford
5 weeks ago

Twenty Fourteen - video in classic editor after.

#9 follow-ups: @laurelfulford
5 weeks ago

  • Keywords needs-testing added

Thanks for reporting this issue, @celloexpressions, and for the patches, @chriseverson and @mmaumio! I'm sorry I couldn't get to this earlier in the 5.0 release.

Reviewing the patches, 44664.diff looks like it is of a different file (package-lock.json) but video_icon_alignment.diff fixes the issue for me on the front-end.

44664.3.patch is basically a copy-paste of that fix into the editor styles, so it's fixed there as well. The new block-based editor doesn't currently have MediaElement.js enqueued into it, so you can only see this issue when using the classic editor. However, these styles are also pulled into the block-based editor, so if MediaElement.js is added down the road, it should fix it there too.

#10 in reply to: ↑ 9 @mmaumio
5 weeks ago

It's great to hear the issue has been fixed.

Thanks for taking care of it :)

@laurelfulford

#11 @pento
5 weeks ago

  • Milestone changed from 5.0.2 to 5.0.3

#12 in reply to: ↑ 9 @netweb
2 weeks ago

  • Keywords commit added

Replying to laurelfulford:

44664.3.patch is basically a copy-paste of that fix into the editor styles, so it's fixed there as well. The new block-based editor doesn't currently have MediaElement.js enqueued into it, so you can only see this issue when using the classic editor. However, these styles are also pulled into the block-based editor, so if MediaElement.js is added down the road, it should fix it there too.

44664.3.patch LGTM

#13 @laurelfulford
2 weeks ago

#43509 was marked as a duplicate.

#14 @laurelfulford
2 weeks ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 44380:

Twenty Fourteen: Correct video play button position.

A change to the MediaElement.js caused Twenty Fourteen's custom video play button to display in the top corner, rather than centered. This update returns it to its correct position.

Props mmaumio, celloexpressions.
Fixes #44664.

#15 @SergeyBiryukov
10 days ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for merging to the 5.0 branch.

Last edited 10 days ago by SergeyBiryukov (previous) (diff)

#16 @SergeyBiryukov
10 days ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 44421:

Twenty Fourteen: Correct video play button position.

A change to the MediaElement.js caused Twenty Fourteen's custom video play button to display in the top corner, rather than centered. This update returns it to its correct position.

Props mmaumio, celloexpressions.
Merges [44380] to the 5.0 branch.
Fixes #44664.

Note: See TracTickets for help on using tickets.