Skip to content

adds skip links check for block themes - #457

Open
MaggieCabrera wants to merge 12 commits into
WordPress:masterfrom
MaggieCabrera:try-skip-links-check
Open

adds skip links check for block themes#457
MaggieCabrera wants to merge 12 commits into
WordPress:masterfrom
MaggieCabrera:try-skip-links-check

Conversation

@MaggieCabrera

@MaggieCabrera MaggieCabrera commented Sep 3, 2024

Copy link
Copy Markdown

This PR adds a check only for block themes to see if templates have a main tag present. If they don't they will be missing the skip links from said template.

I need to check when a template is made out of a pattern and the main tag will be inside the pattern instead of the template file

@MaggieCabrera

Copy link
Copy Markdown
Author

We might want to check if there's multiple tags too

@MaggieCabrera
MaggieCabrera marked this pull request as draft September 3, 2024 11:05
@MaggieCabrera
MaggieCabrera marked this pull request as ready for review September 5, 2024 15:10
Comment thread checks/class-skip-links.php

@matiasbenedetto matiasbenedetto left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

the <main> tag is being rendered in the HTML instead of being printed as text:

image

@matiasbenedetto

Copy link
Copy Markdown
Collaborator

It would be nice to make the main tag search recusive for nested patterns.

@MaggieCabrera

Copy link
Copy Markdown
Author

I fixed those! ready for another review

Comment thread checks/class-skip-links.php Outdated
Co-authored-by: Matias Benedetto <matias.benedetto@gmail.com>
Comment thread checks/class-skip-links.php Outdated
return false;
}

$files = glob( $theme_dir . '/' . $directory . '/*.php' );

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Same as in #457 (comment)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It looks like $php_files doesn't store the contents of the comment in the pattern, so I have no way of checking if the slug of the pattern is the one I'm trying to match

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

seems like that's done here so I will leave this part of the code as I implemented it:

$php[ $filename ] = tc_strip_comments( $php[ $filename ] );

@kafleg

kafleg commented Sep 17, 2024

Copy link
Copy Markdown
Member

I checked by removing "tagName":"main" from the template files in Twenty Twenty Four theme but this PR won't show anything.

Would you please let me know how did you test that?

@kafleg

kafleg commented Sep 17, 2024

Copy link
Copy Markdown
Member

I checked by removing "tagName":"main" from the template files in Twenty Twenty Four theme but this PR won't show anything.

Would you please let me know how did you test that?

I find it. I was checking different PR. Sorry!

@kafleg

kafleg commented Sep 17, 2024

Copy link
Copy Markdown
Member

REQUIRED Skip links are missing from the following templates: single.html, single.html Please make sure the templates have a

tag.

Why the message show single.html, single.html twice?

@MaggieCabrera

Copy link
Copy Markdown
Author

REQUIRED Skip links are missing from the following templates: single.html, single.html Please make sure the templates have a
tag.

Why the message show single.html, single.html twice?

I haven't seen that on my tests, what did you try to get that to happen?

@kafleg

kafleg commented Sep 17, 2024

Copy link
Copy Markdown
Member

In single.html file of TT4 theme, I simply changed <main class="wp-block-group alignfull"> to <div class="wp-block-group alignfull">

@kafleg

kafleg commented Sep 17, 2024

Copy link
Copy Markdown
Member

I think I found the reason,
I changed on index.html and single.html in two files and it is happended.

Comment thread checks/class-skip-links.php
@MaggieCabrera

Copy link
Copy Markdown
Author

I think I found the reason, I changed on index.html and single.html in two files and it is happended.

I just fixed this

@MaggieCabrera

Copy link
Copy Markdown
Author

Sorry, I have been super busy; I just caught up with the reviews. It should be ready to go now!

Comment on lines +19 to +37
/**
* Returns true if the theme is a block theme.
*
* @var array $is_block_theme
*/
protected $is_block_theme = false;

/**
* The WP_Theme instance being checked.
*
* @var WP_Theme $wp_theme
*/
protected $wp_theme = false;

function set_context( $data ) {
if ( isset( $data['theme'] ) ) {
$this->wp_theme = $data['theme'];
$this->is_block_theme = wp_is_block_theme();
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

As far as I can see is_block_theme is not being used in this check class. Is it used somewhere?

@matiasbenedetto matiasbenedetto left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think the Skip_Links_Check class needs to be added to the list of FSE specific checks here:

theme-check/checkbase.php

Lines 386 to 387 in dabaf1d

// Add FSE specific checks.
$themechecks[] = new FSE_Required_Files_Check();

So this line needs to be removed:

$themechecks[] = new Skip_Links_Check();

See FSE_Required_Files_Check class as reference.

@carolinan
carolinan requested a review from Copilot March 19, 2026 09:50

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a new Theme Check rule intended to validate that block-theme templates include a <main> landmark (directly or via referenced patterns) so skip links can be present.

Changes:

  • Adds Skip_Links_Check to scan block template files for a main landmark.
  • Adds pattern parsing to detect <main> within referenced (including nested) block patterns.
  • Registers the new check in the global $themechecks list.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +56 to +58
$file_name = tc_filename( $php_key );
$has_main_tag = strpos( $file, '<main' ) !== false;

Comment on lines +117 to +123
$files = glob( $theme_dir . '/' . $directory . '/*.php' );

$has_tag = false;

foreach ( $files as $file ) {
if ( is_file( $file ) ) {
$contents = file_get_contents( $file );
Comment on lines +33 to +37
function set_context( $data ) {
if ( isset( $data['theme'] ) ) {
$this->wp_theme = $data['theme'];
$this->is_block_theme = wp_is_block_theme();
}

foreach ( $other_files as $php_key => $file ) {
// if the file is a template, print the name of the file
if ( strpos( $php_key, 'templates/' ) !== false ) {
Comment on lines +63 to +69
$has_main_tag = $this->pattern_has_tag( $slug );
if ( ! $has_main_tag ) {
if ( ! in_array( $file_name, $templates_without_main_tag ) ) {
$templates_without_main_tag[] = $file_name;
}
}
}
Comment on lines +121 to +127
foreach ( $files as $file ) {
if ( is_file( $file ) ) {
$contents = file_get_contents( $file );
$pattern = '/\* Slug: ' . preg_quote( $slug, '/' ) . '\b/';
if ( preg_match( $pattern, $contents ) ) {
$has_tag = strpos( $contents, '<main' ) !== false;
if ( ! $has_tag ) {
Comment on lines +107 to +117
$directory = 'patterns';
$theme_dir = $this->wp_theme->get_stylesheet_directory();

if ( ! is_dir( $theme_dir . '/' . $directory ) ) {
$directory = 'block-patterns';
}
if ( ! is_dir( $theme_dir . '/' . $directory ) ) {
return false;
}

$files = glob( $theme_dir . '/' . $directory . '/*.php' );
Comment on lines +127 to +131
if ( ! $has_tag ) {
$nested_patterns_slugs = $this->template_has_patterns( $contents );
if ( $nested_patterns_slugs ) {
foreach ( $nested_patterns_slugs as $slug ) {
$has_tag = $this->pattern_has_tag( $slug );

$info = '';
$templates_without_main_tag = array();

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

4 participants