Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Blocks: Remove WP <5.5 compatibility code #7928

Closed
wants to merge 4 commits into from

Conversation

spacedmonkey
Copy link
Contributor

Context

Summary

Remove backwards compatiliby workarounds to support WP 5.3

Relevant Technical Choices

To-do

User-facing changes

Testing Instructions

QA

  • This is a non-user-facing change and requires no QA

This PR can be tested by following these steps:

UAT

  • UAT should use the same steps as above.

This PR can be tested by following these steps:

Reviews

Does this PR have a security-related impact?

Does this PR change what data or activity we track or use?

Does this PR have a legal-related impact?

Checklist

  • This PR addresses an existing issue and I have linked this PR to it in ZenHub
  • I have tested this code to the best of my abilities
  • I have verified accessibility to the best of my abilities (docs)
  • I have verified i18n and l10n (translation, right-to-left layout) to the best of my abilities
  • This PR contains automated tests (unit, integration, and/or e2e) to verify the code works as intended (docs)
  • I have added documentation where necessary
  • I have added a matching Type: XYZ label to the PR

Fixes #7674

@spacedmonkey spacedmonkey added Group: WordPress Changes related to WordPress or Gutenberg integration Type: Code Quality Things that need a refactor, rewrite or just some good old developer ❤️ Pod: WP & Infra Group: Blocks Issues related to the provided Gutenberg Blocks labels Jun 11, 2021
@spacedmonkey spacedmonkey requested a review from swissspidy June 11, 2021 12:29
@spacedmonkey spacedmonkey self-assigned this Jun 11, 2021
@google-cla google-cla bot added the cla: yes label Jun 11, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Jun 11, 2021

Size Change: -220 B (0%)

Total Size: 2.13 MB

Filename Size Change
assets/js/web-stories-block.js 570 kB -220 B (0%)
ℹ️ View Unchanged
Filename Size Change
assets/css/carousel-view-rtl.css 701 B 0 B
assets/css/carousel-view.css 701 B 0 B
assets/css/edit-story-rtl.css 277 B 0 B
assets/css/edit-story.css 277 B 0 B
assets/css/stories-dashboard-rtl.css 276 B 0 B
assets/css/stories-dashboard.css 276 B 0 B
assets/css/vendors-edit-story-rtl.css 706 B 0 B
assets/css/vendors-edit-story.css 706 B 0 B
assets/css/web-stories-block-rtl.css 3.21 kB 0 B
assets/css/web-stories-block.css 3.24 kB 0 B
assets/css/web-stories-embed-rtl.css 288 B 0 B
assets/css/web-stories-embed.css 288 B 0 B
assets/css/web-stories-list-styles-rtl.css 2.24 kB 0 B
assets/css/web-stories-list-styles.css 2.25 kB 0 B
assets/css/web-stories-theme-style-twentyeleven-rtl.css 102 B 0 B
assets/css/web-stories-theme-style-twentyeleven.css 102 B 0 B
assets/css/web-stories-theme-style-twentyfifteen-rtl.css 251 B 0 B
assets/css/web-stories-theme-style-twentyfifteen.css 251 B 0 B
assets/css/web-stories-theme-style-twentyfourteen-rtl.css 287 B 0 B
assets/css/web-stories-theme-style-twentyfourteen.css 287 B 0 B
assets/css/web-stories-theme-style-twentyseventeen-rtl.css 288 B 0 B
assets/css/web-stories-theme-style-twentyseventeen.css 288 B 0 B
assets/css/web-stories-theme-style-twentysixteen-rtl.css 224 B 0 B
assets/css/web-stories-theme-style-twentysixteen.css 224 B 0 B
assets/css/web-stories-theme-style-twentyten-rtl.css 143 B 0 B
assets/css/web-stories-theme-style-twentyten.css 143 B 0 B
assets/css/web-stories-theme-style-twentytwelve-rtl.css 256 B 0 B
assets/css/web-stories-theme-style-twentytwelve.css 256 B 0 B
assets/css/web-stories-theme-style-twentytwenty-rtl.css 86 B 0 B
assets/css/web-stories-theme-style-twentytwenty.css 86 B 0 B
assets/css/web-stories-theme-style-twentytwentyone-rtl.css 263 B 0 B
assets/css/web-stories-theme-style-twentytwentyone.css 264 B 0 B
assets/css/web-stories-widget-rtl.css 484 B 0 B
assets/css/web-stories-widget.css 484 B 0 B
assets/js/carousel-view.js 3.72 kB 0 B
assets/js/chunk-fonts-********************.js 45.4 kB 0 B
assets/js/chunk-web-stories-template-0-********************.js 465 B 0 B
assets/js/chunk-web-stories-template-10-********************.js 8.11 kB 0 B
assets/js/chunk-web-stories-template-12-********************.js 423 B 0 B
assets/js/chunk-web-stories-template-16-********************.js 9.73 kB 0 B
assets/js/chunk-web-stories-template-18-********************.js 459 B 0 B
assets/js/chunk-web-stories-template-22-********************.js 8.54 kB 0 B
assets/js/chunk-web-stories-template-24-********************.js 450 B 0 B
assets/js/chunk-web-stories-template-28-********************.js 6.99 kB 0 B
assets/js/chunk-web-stories-template-30-********************.js 461 B 0 B
assets/js/chunk-web-stories-template-34-********************.js 6.06 kB 0 B
assets/js/chunk-web-stories-template-36-********************.js 488 B 0 B
assets/js/chunk-web-stories-template-4-********************.js 7.86 kB 0 B
assets/js/chunk-web-stories-template-40-********************.js 7.84 kB 0 B
assets/js/chunk-web-stories-template-42-********************.js 445 B 0 B
assets/js/chunk-web-stories-template-46-********************.js 8.65 kB 0 B
assets/js/chunk-web-stories-template-6-********************.js 478 B 0 B
assets/js/chunk-web-stories-textset-0-********************.js 5.29 kB 0 B
assets/js/chunk-web-stories-textset-1-********************.js 6.81 kB 0 B
assets/js/chunk-web-stories-textset-2-********************.js 7.92 kB 0 B
assets/js/chunk-web-stories-textset-3-********************.js 15.4 kB 0 B
assets/js/chunk-web-stories-textset-4-********************.js 4.43 kB 0 B
assets/js/chunk-web-stories-textset-5-********************.js 5.71 kB 0 B
assets/js/chunk-web-stories-textset-6-********************.js 5.5 kB 0 B
assets/js/chunk-web-stories-textset-7-********************.js 10.4 kB 0 B
assets/js/edit-story.js 524 kB 0 B
assets/js/lightbox.js 986 B 0 B
assets/js/stories-dashboard.js 436 kB 0 B
assets/js/tinymce-button.js 3.48 kB 0 B
assets/js/vendors-chunk-ffmpeg-********************.js 5.65 kB 0 B
assets/js/vendors-edit-story-********************.js 61.5 kB 0 B
assets/js/vendors-edit-story-stories-dashboard-********************.js 245 kB 0 B
assets/js/vendors-web-animations-js-********************.js 14.6 kB 0 B
assets/js/web-stories-activation-notice.js 65.1 kB 0 B
assets/js/web-stories-embed.js 493 B 0 B
assets/js/web-stories-widget.js 984 B 0 B

compressed-size-action

@spacedmonkey spacedmonkey added this to the 1.9.0 milestone Jun 14, 2021
@spacedmonkey spacedmonkey changed the title [DO NOT MERGE] Blocks: Remove WP <5.5 compatibility code Blocks: Remove WP <5.5 compatibility code Jun 17, 2021
@spacedmonkey spacedmonkey requested a review from miina June 17, 2021 14:44
Copy link
Contributor

@miina miina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good but we should wait with merging to ensure the WP min level is set to 5.5 in code, too.

Would be good to send this to QA as well, to see if everything works as expected.

Ok, tested just now a little bit and I can see this for example when selecting stories:
Screenshot 2021-06-18 at 16 07 32

Not sure if related or not -- all good in your local?

@swissspidy
Copy link
Collaborator

Seems good but we should wait with merging to ensure the WP min level is set to 5.5 in code, too.

Absolutely. All tickets were created separately for this effort and set as blockers accordingly.

@spacedmonkey
Copy link
Contributor Author

Ok, tested just now a little bit and I can see this for example when selecting stories:
Not sure if related or not -- all good in your local?

This is broken on main from 5.3-5.7 :( I will create a create a follow up issue.

@spacedmonkey
Copy link
Contributor Author

Follow up issue #7991

@miina
Copy link
Contributor

miina commented Jun 21, 2021

E2E Tests are still running for 5.3 here:
Screenshot 2021-06-21 at 15 40 56

Was that changed in another issue? I know running PHP 7 was changed elsewhere for PHP unit tests, what about the e2e and WP version?

@spacedmonkey
Copy link
Contributor Author

This PR is blocked by #7926

@miina
Copy link
Contributor

miina commented Jun 21, 2021

@spacedmonkey The e2e tests seem to be consistently failing, thoughts?

@@ -87,26 +81,11 @@ const EmbedControls = (props) => {
<>
<BlockControls>
<ToolbarGroup>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do wp.components.ToolbarGroup and wp.components.ToolbarButton exist in 5.5 already?

The e2e test failures seem to indicate otherwise.


/**
* Internal dependencies
*/
import { BLOCK_TYPES } from '../constants';

function BlockTypeSwitcher({ selectedBlockType, setAttributes }) {
// Note: ToolbarGroup and ToolbarButton are only available in Gutenberg 7.0 or later,
// so they do not exist in WP 5.3.
const ToolbarComponent = ToolbarGroup ? ToolbarGroup : Toolbar;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do wp.components.ToolbarGroup and wp.components.ToolbarButton exist in 5.5 already?

The e2e test failures seem to indicate otherwise.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines -40 to -45
// Note: Card, CardBody, CardMedia are only available in Gutenberg 7.0 or later,
// so they do not exist in WP 5.3.
const {
Card = FallbackComponent,
CardBody = FallbackComponent,
CardMedia = FallbackComponent,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doe they exist in WP 5.5 though?

@spacedmonkey
Copy link
Contributor Author

@swissspidy Seems like all the components that have backfill are valid, as they were only added in 5.6. So for that reason I am going to close out this PR.

@swissspidy swissspidy deleted the feature/remove-backwards-compat branch June 22, 2021 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Group: Blocks Issues related to the provided Gutenberg Blocks Group: WordPress Changes related to WordPress or Gutenberg integration Type: Code Quality Things that need a refactor, rewrite or just some good old developer ❤️
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Blocks: Remove WP <5.5 compatibility code
3 participants