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

Fix theme requirement validation with WP 5.8 #37226

Merged
merged 3 commits into from
Dec 8, 2021

Conversation

Mamaduka
Copy link
Member

@Mamaduka Mamaduka commented Dec 8, 2021

Description

Fixes regression when it wasn't possible to activate block theme with WP 5.8. The validate_theme_requirements method is using gutenberg_is_fse_theme in older versions of WP.

Introduced in #37218.

How has this been tested?

Try activating block theme with WP 5.8

Screenshots

CleanShot 2021-12-08 at 19 37 25

Types of changes

Bugfix/Regression

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

@Mamaduka Mamaduka requested a review from youknowriad December 8, 2021 15:38
@Mamaduka Mamaduka self-assigned this Dec 8, 2021
@Mamaduka Mamaduka added [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Type] Regression Related to a regression in the latest release labels Dec 8, 2021
*
* @return boolean Whether the current theme is a block theme or not.
*/
function gutenberg_is_fse_theme() {
Copy link
Contributor

Choose a reason for hiding this comment

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

We should move this in compat/wordpress-5.9/theme.php.

@Mamaduka
Copy link
Member Author

Mamaduka commented Dec 8, 2021

Merging this should also fix failing performance checks.


/**
* Note: We have to maintain this function for backward compatibility with WP 5.8.
* Only remove once 5.9 is the minimum supported WordPress version for the Gutenberg plugin.
Copy link
Contributor

Choose a reason for hiding this comment

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

We could remove this line for 5.9, as it's already suggested by structure we are trying to follow for compat changes per wp version.

I think your PR description (with a small rewording/changes) might be a better fit as a comment:

The validate_theme_requirements method is using gutenberg_is_fse_theme in older versions of WP.

@Mamaduka
Copy link
Member Author

Mamaduka commented Dec 8, 2021

Thanks, Nik. I've updated the commend and will merge this once all checks are green.

@Mamaduka Mamaduka merged commit 8aa8230 into trunk Dec 8, 2021
@Mamaduka Mamaduka deleted the fix/validate-theme-requirements branch December 8, 2021 18:03
@github-actions github-actions bot added this to the Gutenberg 12.2 milestone Dec 8, 2021
@Mamaduka
Copy link
Member Author

Mamaduka commented Dec 9, 2021

@noisysocks, we might want to backport this to the core branch since original PR is also marked for backporting.

@youknowriad youknowriad added the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Dec 9, 2021
@youknowriad
Copy link
Contributor

So we backport this to wp/trunk but not core

@Mamaduka
Copy link
Member Author

Mamaduka commented Dec 9, 2021

@youknowriad, right, backport only to wp/trunk.

@noisysocks noisysocks removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Dec 13, 2021
noisysocks pushed a commit that referenced this pull request Dec 13, 2021
* Fix theme requirement validation with WP 5.8

* Use compat file

* Update comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Type] Regression Related to a regression in the latest release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants