Skip to content
This repository has been archived by the owner on Jan 26, 2022. It is now read-only.

Stop using __DIR__ to load block pattern files #221

Closed
desrosj opened this issue Nov 9, 2021 · 5 comments · Fixed by #277
Closed

Stop using __DIR__ to load block pattern files #221

desrosj opened this issue Nov 9, 2021 · 5 comments · Fixed by #277
Labels
[Type] Question Further information is requested

Comments

@desrosj
Copy link
Contributor

desrosj commented Nov 9, 2021

When reviewing the theme for merge into Core, I noticed that block patterns are being included using require __DIR__ . '/patterns/' . $block_pattern . '.php'.

I was curious if there was a reason for this. If this gets switched to using something like get_stylesheet_directory(), or get_template_directory() (these may not do exactly what we need), it could become possible for a child theme to include one pattern override file in patterns directory.

@kjellr kjellr added the [Type] Question Further information is requested label Nov 9, 2021
@MaggieCabrera
Copy link
Collaborator

I tried this and it doesn't work as you described. Using get_template_directory() will always return the parent's pattern, even if the same file exists on the child theme. Using get_stylesheet_directory() fails if the child theme doesn't have a replacement for each and every one of the patterns.

At the very least a child theme could override twentytwentytwo_register_block_patterns directly if they need to.

@desrosj
Copy link
Contributor Author

desrosj commented Nov 15, 2021

Thanks for looking into it @MaggieCabrera!

I think the function that we'd want to use here would be locate_template(). But could not use it in just one line.

foreach ( $block_patterns as $block_pattern ) {
	$pattern_file = locate_template( array( 'patterns/' . $block_pattern . '.php' ) );

	register_block_pattern(
		'twentytwentytwo/' . $block_pattern,
		require $pattern_file
	);
}

I don't feel strongly about this and happy to be overruled in favor of better practices for modifying patterns. But here is the scenario I would like to protect against:

  • Someone creates a child theme.
  • They don't like how one or two block templates are set up.
  • They override the entire twentytwentytwo_register_block_patterns() function.
  • Any future updates to this function to add new patterns or categories in the theme won't be seen by those sites.

@jffng
Copy link
Collaborator

jffng commented Nov 17, 2021

@desrosj is the scenario you describe solved by the addition of a filter #238?

@desrosj
Copy link
Contributor Author

desrosj commented Nov 17, 2021

The filters would allow a pattern to be removed and re-added elsewhere. But it would not allow a child theme to supply a customized block pattern by including their own version of a pattern in their patterns directory.

@justintadlock
Copy link

Since these are not templates, I'd use get_theme_file_path() over locate_template(): https://developer.wordpress.org/reference/functions/get_theme_file_path/

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
[Type] Question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants