WordPress / theme-check

Theme Check plugin
https://wordpress.org/plugins/theme-check/
341 stars 114 forks source link

Adapt checks for Full-Site Editing themes #279

Open fklein-lu opened 4 years ago

fklein-lu commented 4 years ago

Full-site editing themes do not use mosts of the existing PHP template tags, CSS classes, or theme supports. They therefore currently do not pass the Theme Checker.

We should implement a way to modify the existing checks when an FSE theme is uploaded. These include:

At the same time we should ensure that FSE themes have at least an index.html template, and a theme config file.

joyously commented 4 years ago

Would there be a way to check if there are corresponding blocks in the templates for post navigation and tags and avatars? How does that other stuff happen (comment reply, widget support, post thumbnail support)?

fklein-lu commented 4 years ago

The point of this issue is to enable developers right now to upload FSE themes without needing to include code that they don't need just for the sake of passing the Theme Checker.

There are ways to check FSE themes for compliance, and I'm sure that will be done in the future.

But FSE is still experimental, and therefore there are no guidelines. So including checks in the Theme Checker before FSE is stable, and the guidelines have been updated is putting the cart before the horse.

Otto42 commented 4 years ago

The theme checker should be modified to add support for ways of checking for these things, certainly, but if these "FSE" themes are missing basic requirements, then they should not be added to the theme directory.

Disabling checks just to allow these is premature. Themes should be complete and cover all the use cases for them. If any "FSE" themes have used this approach to bypass requirements, then they should be removed from the theme directory. Merely being "new" does not absolve them from being complete as well.

aristath commented 4 years ago

All themes go through a manual review once they get uploaded to the repo. Nothing goes live without first being reviewed, and this theme-check is just the 1st step in the process. It doesn't however stop there... This is just some sanity checks that run on theme-upload to avoid some of the most common errors, security issues and bad practices. Right now block-based themes can't even be uploaded and go to the step where they can be manually reviewed, that's what needs to be fixed. If there are any issues the manual review will find them, but first the theme needs to be uploaded to the repo.

carolinan commented 3 years ago

A first version has been added with https://github.com/WordPress/theme-check/pull/280.

carolinan commented 3 years ago

Current requirements for block themes are: Include theme.json. Include block-templates/index.html

+requirements that apply to all themes: GPL compatible, secure, no remote resources, no JS or PHP errors, screenshot size and so on.

The following checks are excluded right now:

Style_Suggested           -Block themes do not output the same classes as classic themes
WidgetsCheck              -Block themes do not use widgets or widget areas. 
GravatarCheck             -Gravatar support is built in with the post author block
Title_Checks              -Block themes have build in support for the title tag theme support.
PostPaginationCheck       -Blocks are used for these. When PHP functions are used, those functions are part of the block.
Comments                  -Blocks are used for these. When PHP functions are used, those functions are part of the block.
CommentPaginationCheck    -Blocks are used for these. When PHP functions are used, those functions are part of the block.
Comment_Reply             -The comment reply script is part of the comments block.
Basic_Checks              -Blocks are used for these. When PHP functions are used, those functions are part of the block.
NavMenuCheck              -Block themes use the navigation block
PostThumbnailCheck        -Featured images are blocks (and the theme support is not required for the block).
ThemeSupport              -Most theme support is replaced by block settings or theme.json
EditorStyleCheck          -Styles from theme.json are applied to the editor.
UnderscoresCheck          -Block themes are not likely to be based on underscores :)
Constants                  -Old PHP constants are not likely to be used in block themes
CustomizerCheck           -Block themes do not use the customizer.
Deprecated_Recommended    -Not likely to be used in block themes
PostFormatCheck           -Block themes do not use these functions to apply/style post formats.
SearchFormCheck           -Replaced by the search block
Screen_Reader_Text        -Automatic in block themes
IncludeCheck              -Block themes does not include PHP _templates._
pbking commented 3 years ago

I have found the following items failing on "empty theme" which, while small, I expected to pass the bare minimum requirements:

REQUIRED: No reference to add_theme_support( "title-tag" ) was found in the theme.
REQUIRED: Could not find wp_link_pages. See: wp_link_pages <?php wp_link_pages( $args ); ?>
REQUIRED: Could not find wp_head. See: wp_head <?php wp_head(); ?>
REQUIRED: Could not find wp_footer. See: wp_footer <?php wp_footer(); ?>
REQUIRED: Could not find wp_body_open action or function call at the very top of the body just after the opening body tag. See: wp_body_open <?php wp_body_open(); ?>
REQUIRED: Could not find post_class. See: post_class <div id="post-<?php the_ID(); ?>" <?php post_class(); ?>>
REQUIRED: Could not find language_attributes. See: language_attributes<html <?php language_attributes(); ?>
REQUIRED: Could not find charset. There must be a charset defined in the Content-Type or the meta charset tag in the head.
REQUIRED: Could not find body_class call in body tag. See: body_class <?php body_class( $class ); ?>
REQUIRED: Could not find DOCTYPE. See: https://codex.wordpress.org/HTML_to_XHTML<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN""http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd"?>
carolinan commented 3 years ago

I have found the following items failing on "empty theme" which, while small, I expected to pass the bare minimum requirements:

These are not requirements for block themes. Did you place empty theme in a sub folder? I think you may have ran into this issue: https://github.com/WordPress/theme-check/issues/318

(Also there is an issue with empty theme specifically. Theme Name: Empty Theme should actually use the slug empty-theme as the folder name and now it is placed inside emptytheme)

pbking commented 3 years ago

Ah, yes I did that's exactly what happened. Thank you @carolinan. I wasn't aware of that bug (and I always have themes in folders so perhaps that's one I should work on. :P )

carolinan commented 3 years ago

It looks like the problem mentioned above is preventing block themes from being uploaded to the theme directory. The themes passes the checks locally, but during upload, the correct checks are not skipped.

Example of local result:

RECOMMENDED: No reference to register_block_style was found in the theme. Theme authors are encouraged to implement new block styles as a transition to block themes.
RECOMMENDED: No reference to register_block_pattern was found in the theme. Theme authors are encouraged to implement custom block patterns as a transition to block themes.
RECOMMENDED: No content width has been defined. Example: if ( ! isset( $content_width ) ) $content_width = 900;
INFO Only one text-domain is being used in this theme. Make sure it matches the theme's slug correctly so that the theme will be compatible with WordPress.org language packs. The domain found is rick

Example result of uploading a block theme, it's long so I will only post the list of the required items. WordPress slack account required: https://wordpress.slack.com/archives/C01JDB9UW2H/p1625248043102400 There should be several upload attempts logged with the "Rick" theme by @kafleg

Theme Check Required
Basic Check:
Could not find DOCTYPE.
Could not find wp_body_open action or function call at the very top of the body just after the opening body tag.
Could not find wp_footer.
Could not find wp_head.
Could not find language_attributes.
Could not find charset.
Could not find body_class call in body tag.
Could not find wp_link_pages.
Could not find post_class.

Screen Reader Text Check:
.screen-reader-text CSS class is generated by WordPress and needs to be styled with your theme CSS.
Theme Support Title Tag Check:
No reference to add_theme_support( "title-tag" ) was found in the theme.

@dd32 Can you confirm that the folder structure is different and that during upload to the directory, the check can not find block-templates/index.html? How do we solve it? How do we assure that this upload step is tested before the release of the next version of the plugin?

kafleg commented 3 years ago

I tried uploading a block theme(FSE Theme) and got the issues that @carolinan mentioned above.

To check the uploader as well,

  1. Added invalid tags and theme check shows Error message. - Perfect
  2. Added a ZIP file inside the theme folder and try to upload it. I see Error message. - Perfect.
  3. I fixed the above two issues and upload it again, but still the uploader is showing Error. Those errors are suitable for classic themes(PHP Themes) but should not be applicable for FSE theme.

Can we host theme check plugin in somewhere we can test these issues? We can't figureout which issues is going to block the uploads.