Videopress Block: Fix video width and lack of alignment classes by aaronrobertshaw · Pull Request #18006 · Automattic/jetpack

aaronrobertshaw · 2020-12-08T09:08:05Z

Fixes Automattic/wp-calypso#46889
Fixes #17925

VideoPress block on WordPress.com doesn't get the alignment CSS classes applied to it on save. This leads to the video appearing to be different widths in editor vs frontend.

Changes proposed in this Pull Request:

Update application of CSS class names it to pick up the alignment CSS classes.
Fix block attributes to prevent block validation errors that were occurring in production
Move setting of classNames attribute out of the render() function to avoid UnsavedChangesWarning error.
Add deprecation for older blocks that were missing alignment classes or didn't get classNames saved due to bug.

Does this pull request change what data or activity we track or use?

No.

Unit test for attribute migrations

Run yarn test-extensions
Confirm extensions/blocks/videopress/test/index.js ran successfully.

Testing instructions - Local:

Using master branch, start up local Jetpack site with sufficient plan to upload videos to videopress
Add a video block to a post and upload a video
Set it's alignment to wide and add a custom CSS class name via sidebar > advanced controls
Save post and view on frontend. (Should be missing alignwide CSS class)
Reload post in the editor, the block's will be invalid due to current bugs.
Checkout this PR and rebuild Jetpack
With dev tools console open, reload the editor and confirm no errors and the block deprecation is successfully applied
Save the post and reload the frontend.
Confirm the block displays with wide alignment

Testing instructions - Sandbox:

Apply this PR branch and test VideoPress block functions correctly locally.
On a sandboxed site with a sufficient plan, add a video to a post, set its alignment to "wide", then save.
Confirm on the frontend that the video's block does not contain the alignwide CSS class.
Apply this PR's associated diff ( D53903-code ) or use a8c-wp-env and sync Jetpack to your sandbox.
Reload the editor for your sandboxed site and confirm the block is still valid.
Change the block's alignment to "full" and save post.
Switch to the frontend, reload and confirm the video block has the alignfull CSS class.

Screenshots

Wide Align - Before	Wide Align - After

Proposed changelog entry for your changes:

Fixes missing alignment classes on VideoPress block.
Fixes VideoPress block validation errors

matticbot · 2020-12-08T09:08:43Z

Caution: This PR has changes that must be merged to WordPress.com
Hello aaronrobertshaw! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer and confirm D53903-code works as expected before merging this PR. Once this PR is merged, please commit the changes to WP.com. Thank you!
This revision will be updated with each commit to this PR

jetpackbot · 2020-12-08T09:09:59Z

Scheduled Jetpack release: February 2, 2021.
Scheduled code freeze: January 25, 2021

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Generated by 🚫 dangerJS against bd70276

glendaviesnz · 2020-12-09T03:29:56Z

I think we might not be able to use useBlockProps as it won't be supported by all the versions of the editor that jetpack supports - looks like it was only renamed and stabalised 2 months ago - WordPress/gutenberg#25642

apeatling

We chatted about this, but I think you could check for the presence of useblockProps and fall back to manual if not present. Or go with manual until Jetpack catches up to this.

roundhill · 2020-12-10T22:09:49Z

Stumbled across this PR, thanks for fixing @aaronrobertshaw 😄

aaronrobertshaw · 2020-12-14T04:37:12Z

This has been updated to check for useBlockProps as well as being rebased.

I've tested this using the latest Gutenberg release and v9.1.1 just prior to when commit WordPress/gutenberg#25642 stabilised the block API involving useBlockProps in v9.2.0

jeherve

Since we're changing the saved markup of our blocks, we'll need to handle block deprecation to avoid block validation errors when editing existing posts. Could you add this?

aaronrobertshaw · 2021-01-12T11:38:33Z

I've rebased this PR and updated it to address the issue where newer versions of Gutenberg caused all the deprecations to be invalid. Tested with WordPress 5.5.3 and 5.6 as well as latest Gutenberg.

The issue with the wp-block-video class being added via the blocks.getSaveContent.extraProps filter was solved using another filter targeting deprecated blocks and adjusting the results so that the original saved content for a deprecated block would be successfully matched.

If anyone has any suggestions as to a better approach, I'd be happy to hear them.

glendaviesnz · 2021-01-14T03:42:21Z

This tests well for me now, and the migrations run correctly for the previous versions.

* Fixes missing alignment CSS classes * Fixes deprecations after Gutenberg introduction of wp-block-video CSS class * Fixes bug introduced in prior patch addressing aspect ratio classes

Improves the clarity of the attribute naming to distinguish what was `classNames` from `className`.

… to attributes

This shouldn't be necessary anymore, we now have that rule set for all the extensions at once.

jeherve

This works well for me now; it should be good to merge!

jeherve · 2021-02-02T11:15:03Z

r220372-wpcom

aaronrobertshaw added Bug When a feature is broken and / or not performing as intended [Feature] VideoPress A feature to help you upload and insert videos on your site. [Status] Needs Team Review Obsolete. Use Needs Review instead. labels Dec 8, 2020

aaronrobertshaw self-assigned this Dec 8, 2020

matticbot added the Touches WP.com Files label Dec 8, 2020

aaronrobertshaw changed the title ~~[WIP] Videopress Block: Fix video width and lack of alignment classes~~ Videopress Block: Fix video width and lack of alignment classes Dec 8, 2020

apeatling requested review from a team and apeatling December 10, 2020 18:40

apeatling suggested changes Dec 10, 2020

View reviewed changes

aaronrobertshaw force-pushed the fix/videopress-width branch from 260217c to f130da7 Compare December 14, 2020 04:24

apeatling self-requested a review December 14, 2020 17:35

apeatling previously approved these changes Dec 14, 2020

View reviewed changes

apeatling added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Team Review Obsolete. Use Needs Review instead. labels Dec 14, 2020

apeatling requested a review from a team December 14, 2020 17:35

jeherve added this to the 9.3 milestone Dec 15, 2020

jeherve requested changes Dec 15, 2020

View reviewed changes

jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Dec 15, 2020

aaronrobertshaw dismissed apeatling’s stale review via 0f99a44 December 18, 2020 06:05

aaronrobertshaw force-pushed the fix/videopress-width branch from f130da7 to 0f99a44 Compare December 18, 2020 06:05

apeatling requested review from apeatling and jeherve December 21, 2020 23:10

apeatling previously approved these changes Dec 21, 2020

View reviewed changes

kraftbj removed the [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. label Dec 21, 2020

aaronrobertshaw force-pushed the fix/videopress-width branch from 922a185 to 880866d Compare January 12, 2021 11:22

aaronrobertshaw requested a review from jeherve January 13, 2021 02:06

aaronrobertshaw added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Jan 13, 2021

glendaviesnz reviewed Jan 13, 2021

View reviewed changes

Comment thread extensions/blocks/videopress/deprecated/v2/index.js Outdated

aaronrobertshaw and others added 7 commits January 15, 2021 13:17

Fix application of videopress block classes

04bd589

* Fixes missing alignment CSS classes * Fixes deprecations after Gutenberg introduction of wp-block-video CSS class * Fixes bug introduced in prior patch addressing aspect ratio classes

Rename classNames to videoPressClassNames

24778d7

Improves the clarity of the attribute naming to distinguish what was `classNames` from `className`.

Hard code the wp-block-video class in edit and save instead of adding…

1dd2a93

… to attributes

Take the align classes out of the videoPressClasses attribute

f441a9d

Consolidate attribute destructuring

fd05e06

Add class filtering to maintain custom CSS classes

43ffe09

Add tests for className attribute migration util

62a7598

jeherve force-pushed the fix/videopress-width branch from 00ee06d to 62a7598 Compare January 15, 2021 12:17

jeherve added 2 commits January 15, 2021 13:55

Move files manually, rebase failed to relocate them properly

be6eb6c

Remove custom linting config

bd70276

This shouldn't be necessary anymore, we now have that rule set for all the extensions at once.

jeherve approved these changes Jan 15, 2021

View reviewed changes

jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Jan 15, 2021

jeherve mentioned this pull request Jan 15, 2021

VideoPress: Fix video upload issue #18327

Closed

aaronrobertshaw merged commit bb1591a into master Jan 18, 2021

aaronrobertshaw deleted the fix/videopress-width branch January 18, 2021 04:44

matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Jan 18, 2021

roundhill mentioned this pull request Jan 19, 2021

VideoPress Block: Add UI for Customizing Player Seekbar Colors #18155

Merged

jeherve added [Status] Has Changelog and removed [Status] Needs Changelog labels Jan 26, 2021

jeherve added a commit that referenced this pull request Jan 26, 2021

Changelog: add #18006

aebed46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Videopress Block: Fix video width and lack of alignment classes#18006

Videopress Block: Fix video width and lack of alignment classes#18006
aaronrobertshaw merged 9 commits into
masterfrom
fix/videopress-width

aaronrobertshaw commented Dec 8, 2020 •

edited by jeherve

Loading

Uh oh!

matticbot commented Dec 8, 2020

Uh oh!

jetpackbot commented Dec 8, 2020 •

edited

Loading

Uh oh!

glendaviesnz commented Dec 9, 2020

Uh oh!

apeatling left a comment

Uh oh!

roundhill commented Dec 10, 2020

Uh oh!

aaronrobertshaw commented Dec 14, 2020

Uh oh!

jeherve left a comment

Uh oh!

aaronrobertshaw commented Jan 12, 2021

Uh oh!

Uh oh!

glendaviesnz commented Jan 14, 2021

Uh oh!

jeherve left a comment

Uh oh!

jeherve commented Feb 2, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

Conversation

aaronrobertshaw commented Dec 8, 2020 • edited by jeherve Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes proposed in this Pull Request:

Does this pull request change what data or activity we track or use?

Unit test for attribute migrations

Testing instructions - Local:

Testing instructions - Sandbox:

Screenshots

Proposed changelog entry for your changes:

Uh oh!

matticbot commented Dec 8, 2020

Uh oh!

jetpackbot commented Dec 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

glendaviesnz commented Dec 9, 2020

Uh oh!

apeatling left a comment

Choose a reason for hiding this comment

Uh oh!

roundhill commented Dec 10, 2020

Uh oh!

aaronrobertshaw commented Dec 14, 2020

Uh oh!

jeherve left a comment

Choose a reason for hiding this comment

Uh oh!

aaronrobertshaw commented Jan 12, 2021

Uh oh!

Uh oh!

glendaviesnz commented Jan 14, 2021

Uh oh!

jeherve left a comment

Choose a reason for hiding this comment

Uh oh!

jeherve commented Feb 2, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

aaronrobertshaw commented Dec 8, 2020 •

edited by jeherve

Loading

jetpackbot commented Dec 8, 2020 •

edited

Loading