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

Discussion: Identifying themes with support for full site editing #297

Open
carolinan opened this issue Mar 12, 2021 · 13 comments
Open

Discussion: Identifying themes with support for full site editing #297

carolinan opened this issue Mar 12, 2021 · 13 comments

Comments

@carolinan
Copy link
Collaborator

Pending WordPress/gutenberg#29026
There are 4 types of themes that the plugin needs to be able to identify.

  1. Classic PHP based themes
    All standard checks
  2. Child themes
    Displays specific child theme information
  3. Full site editing themes -no PHP templates, but one required PHP file: index.php.
    Skips flagged checks
    Validate block markup
  4. Classic PHP based themes with full site editing templates
    All standard checks
    Validate block markup

This means that we won't be able to identify 3, FSE themes, only based on if the
block-templates/index.html exist
https://github.com/WordPress/theme-check/pull/280/files#diff-ee4805a450bdb608e6a3daee4ead2822e33a8d8fd48ead59501cdb3e2ee699b2R324

@aristath
Copy link
Member

I think in 5.8 where the FSE MVP will land, we'll also need to make a few adjustments to core...
The index.php file in FSE themes should not be required.
So if we make that change, we can detect FSE themes by checking that the theme has a block-templates/index.html file AND does not have an index.php file.

@carolinan
Copy link
Collaborator Author

The 5.8 milestone is already open on core trac

@pattonwebz
Copy link
Member

I have some idea of how checks could be flagged for different types of themes but I have no idea how I would write a block validator 🤔

@aristath
Copy link
Member

I have no idea how I would write a block validator 🤔

Why would we need that? Themes are not and will probably not be) allowed to add blocks...
Unless I misunderstood what you mean by "block validator"?

@pattonwebz
Copy link
Member

I have no idea how I would write a block validator thinking

Why would we need that? Themes are not and will probably not be) allowed to add blocks...
Unless I misunderstood what you mean by "block validator"?

In the opening post here for type 3 and 4, it suggests that block markup validation would be needed.

@aristath
Copy link
Member

Ah OK, gotcha 👍
I misunderstood what "block validator" means 😅

@pattonwebz
Copy link
Member

Sorry, I should have said block markup validator. Missing a word means it becomes a whole other, much bigger, thing haha

@carolinan
Copy link
Collaborator Author

carolinan commented Mar 12, 2021

I have not looked at this closer. Validate submitted patterns:
WordPress/pattern-directory#22
WordPress/pattern-directory#38

@carolinan
Copy link
Collaborator Author

@fklein-lu
Copy link

I have some idea of how checks could be flagged for different types of themes but I have no idea how I would write a block validator

That's actually pretty simple.

For a HTML template in a block-based theme, you would:

  1. Get the contents of the file, which is serialized blocks.
  2. Pass the file content through parse_blocks().
  3. Analyze the array of parsed block objects.

The parsed block object contains everything you need to know about a block. So you can check things like whether a template has a Query block, what the attributes of this block are etc. If a block can't be parsed, this means the block isn't valid--that's easy to detect.

This all means that the introduction of FSE is an opportunity to make Theme Check a lot better than it was before. Due to the structured nature of a block template, it's straightforward to find out what's in it. In opposition to the current PHP-based templates.

@carolinan
Copy link
Collaborator Author

If the block theme specific testing will be in theme review action I see no reason -other than easy of use - why FSE checks
should also be in Theme Check?
WordPress/theme-review-action#25

@StevenDufresne
Copy link

The theme review action does use the same JS parse function that Gutenbergs does to check for block templates in a very similar manner to what @fklein-lu mentions.

https://github.com/WordPress/theme-review-action/tree/trunk/actions/ui-check/tests/unit/block-templates

@carolinan
Copy link
Collaborator Author

I think in 5.8 where the FSE MVP will land, we'll also need to make a few adjustments to core...
The index.php file in FSE themes should not be required.
So if we make that change, we can detect FSE themes by checking that the theme has a block-templates/index.html file AND does not have an index.php file.

@aristath Are you able to open an issue for this and start working on a patch? It would need to be committed before the feature freeze on the 25th.

Or should we go with something like checking for the index.html file in combination with the full-site-editing style tag? The latter will only work as long as there is a manual review step.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants