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

adds skip links check for block themes #457

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
150 changes: 150 additions & 0 deletions checks/class-skip-links.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
<?php
/**
* Check if skip links are present on block themes
*
* @package Theme Check
*/

/**
* Check if skip links are present on block themes
*/
class Skip_Links_Check implements themecheck {
/**
* Error messages, warnings and info notices.
*
* @var array $error
*/
protected $error = array();

/**
* 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();
}
MaggieCabrera marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Check that return true for good/okay/acceptable, false for bad/not-okay/unacceptable.
*
* @param array $php_files File paths and content for PHP files.
* @param array $css_files File paths and content for CSS files.
* @param array $other_files Folder names, file paths and content for other files.
*/
public function check( $php_files, $css_files, $other_files ) {

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

$directory = 'templates'; // Path to the folder containing HTML files
$theme_dir = $this->wp_theme->get_stylesheet_directory();

// Get all HTML files in the directory
$files = glob( $theme_dir . '/' . $directory . '/*.html' );
Copy link
Collaborator

@matiasbenedetto matiasbenedetto Sep 16, 2024

Choose a reason for hiding this comment

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

Accessing the file system from the check function doesn't seems to be necesary and it looks like it's not in line with the other implementation. The check function is already getting 3 arrays as parameters containing the list of file paths of the theme.

https://github.com/MaggieCabrera/theme-check/blob/17ee4214ab4331440865f75ab27b0aa17e212fda/checks/class-skip-links.php#L47


foreach ( $files as $file ) {
$contents = file_get_contents( $file );
$has_main_tag = strpos( $contents, '<main' ) !== false;
$file_name = basename( $file );

if ( ! $has_main_tag ) {
$pattern_slugs = $this->template_has_patterns( $contents );
if ( $pattern_slugs ) {
foreach ( $pattern_slugs as $slug ) {
$has_main_tag = $this->pattern_has_tag( $slug );
if ( ! $has_main_tag ) {
$templates_without_main_tag[] = $file_name;
}
}
} else {
$templates_without_main_tag[] = $file_name;
}
}
}

$info = implode( ', ', $templates_without_main_tag );

if ( $info !== '' ) {
$this->error[] = sprintf(
'<span class="tc-lead tc-required">%s</span> %s ',
__( 'REQUIRED', 'theme-check' ),
sprintf(
__( 'Skip links are missing from the following templates: %s Please make sure the templates have a &lt;main&gt; tag.', 'theme-check' ),
$info
)
);
}
Comment on lines +81 to +89
Copy link
Collaborator

@matiasbenedetto matiasbenedetto Sep 17, 2024

Choose a reason for hiding this comment

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

When a REQUIRED change is returned by the check function, it should return false:
See #465 as reference


return true;
}

function template_has_patterns( $contents ) {
$pattern = '/<!-- wp:pattern \{"slug":"([^"]+)"\} \/-->/';
if ( preg_match_all( $pattern, $contents, $matches ) ) {
$slugs = $matches[1];
return $slugs;
} else {
return false;
}
}

function pattern_has_tag( $slug ) {
$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' );
Copy link
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)


$has_tag = false;

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 ) {
$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 );
}
}
}
}
}
}

return $has_tag;
}

/**
* Get error messages from the checks.
*
* @return array Error message.
*/
public function getError() {
return $this->error;
}
}

$themechecks[] = new Skip_Links_Check();
Loading