WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 19 months 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 2 years ago.
video_icon_alignment.diff (536 bytes) - added by mmaumio 2 years 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 2 years ago.
Before Icon Alignment Fix
after-icon-alignment-fix.png (196.7 KB) - added by mmaumio 2 years ago.
After the fix
44664.3.patch (1.2 KB) - added by laurelfulford 20 months ago.
Copy fix editor styles.
twenty-fourteen-editor-screenshot-before.png (156.2 KB) - added by laurelfulford 20 months ago.
Twenty Fourteen - video in classic editor before.
twenty-fourteen-editor-screenshot-after.png (156.3 KB) - added by laurelfulford 20 months ago.
Twenty Fourteen - video in classic editor after.

Download all attachments as: .zip

Change History (23)

@chriseverson
2 years ago

#1 @chriseverson
2 years 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
2 years 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
2 years ago

Before Icon Alignment Fix

@mmaumio
2 years ago

After the fix

#2 @mmaumio
2 years 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.


2 years ago

#4 @pento
22 months ago

  • Milestone changed from 4.9.9 to 5.0.1

#5 @celloexpressions
22 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.


21 months ago

#7 @SergeyBiryukov
21 months ago

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

#8 @pento
20 months ago

  • Milestone changed from 5.0.1 to 5.0.2

@laurelfulford
20 months ago

Copy fix editor styles.

@laurelfulford
20 months ago

Twenty Fourteen - video in classic editor before.

@laurelfulford
20 months ago

Twenty Fourteen - video in classic editor after.

#9 follow-ups: @laurelfulford
20 months 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
20 months ago

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

Thanks for taking care of it :)

@laurelfulford

#11 @pento
20 months ago

  • Milestone changed from 5.0.2 to 5.0.3

#12 in reply to: ↑ 9 @netweb
20 months 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
20 months ago

#43509 was marked as a duplicate.

#14 @laurelfulford
20 months 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
19 months ago

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

Reopening for merging to the 5.0 branch.

Last edited 19 months ago by SergeyBiryukov (previous) (diff)

#16 @SergeyBiryukov
19 months 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.