Make WordPress Core

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#39323 closed enhancement (wontfix)

Twenty Seventeen: wp_resource_hints when enqueue custom fonts in child theme

Reported by: samikeijonen's profile sami.keijonen Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.7
Component: Bundled Theme Keywords:
Focuses: Cc:

Description

In Twenty Seventeen we add preconnect for Google Fonts via wp_resource_hints.

In there we check that styles are enqueued: wp_style_is( 'twentyseventeen-fonts', 'queue' ).

In child theme it's pretty common that we dequeue parent fonts and load our own Google fonts: wp_dequeue_style( 'twentyseventeen-fonts' );

This means that our wp_style_is check is false and our preconnect code is not printed anymore.

Could we change the check like this so that preconnect would be in child themes also. For example:

wp_style_is( 'twentyseventeen-fonts', 'registered' ).

Downside is that child theme can use other than Google fonts also. Anyways I just wanted to put this out here for discussion.

Change History (7)

#1 @davidakennedy
7 years ago

  • Keywords reporter-feedback added

Hey @sami.keijonen! Thanks for the enhancement request!

I like the idea of this, but I also wonder: if you dequeue twentyseventeen-fonts, I think it makes sense the wp_style_is check is false. We don't know what a child theme's intentions are if they've dequeued something. Maybe its just system fonts the theme is after so this function or a themer's knowledge of it being there isn't needed. I think at a certain point, once a child theme does certain things, they're on their own, so to speak.

I'm leaning toward not doing this, but open to feedback and more discussion.

#2 @sami.keijonen
7 years ago

Maybe its just system fonts the theme is after so this function or a themer's knowledge of it being there isn't needed

My thoughts also. But I also try not to repeat the same functions in the child theme in case I do use Google fonts.

What if we add some kind of "standard" handle for child themes like this:

wp_style_is( 'twentyseventeen-fonts', 'queue' ) || wp_style_is( 'twentyseventeen-child-fonts', 'queue' )

#3 @peterwilsoncc
7 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed

I had similar thoughts to @davidakennedy when this was first opened but wanted to wait for his feedback.

twentyseventeen-child-fonts (or another standard) could refer to a commercial provider and pre-connecting to the Google server would hinder performance. In this instance repetition makes it more difficult to make a mistake.

#4 @dd32
7 years ago

  • Keywords reporter-feedback removed

I'll echo above thoughts, The child theme should make the call if it's removed the style that the resource hint was on in the parent.

#5 @sami.keijonen
7 years ago

  • Resolution wontfix deleted
  • Status changed from closed to reopened

I'm not giving up for two reasons:)

  • It feels so odd and wrong to duplicate the same function in a child theme.
  • This is an example why filters are better approach than pluggable function (not that this is).

So why not add a filter around fonts handle like this:

function twentyseventeen_resource_hints( $urls, $relation_type ) {
	// Fonts handle which we can filter in child theme.
	$fonts_handle = apply_filters( 'twentyseventeen_preconnect_fonts_handle', 'twentyseventeen-fonts' );

	if ( wp_style_is( esc_attr( $fonts_handle ), 'queue' ) && 'preconnect' === $relation_type ) {
		$urls[] = array(
			'href' => 'https://fonts.gstatic.com',
			'crossorigin',
		);
	}

	return $urls;
}
add_filter( 'wp_resource_hints', 'twentyseventeen_resource_hints', 10, 2 );

#6 @dd32
7 years ago

  • Resolution set to wontfix
  • Status changed from reopened to closed

Your proposed filter just makes it even more complicated.

If you're not going to use the Twenty Seventeen CSS fonts enqueue, and instead replace it with your own, specify what resource hints that file needs using the WordPress API.

I'm marking this as wontfix again, conversation can continue while closed, and a committer can re-open if they so desire.

For reference, this is what it would look like in a child theme:

add_action( 'wp_enqueue_scripts', function() {
    wp_dequeue_style( 'twentyseventeen-fonts' );
    wp_enqueue_style( 'my-theme-fonts', 'https://fonts.googleapis.com/css....' );
}, 11 );

function mytheme_resource_hints( $urls, $relation_type ) {
	if ( wp_style_is( 'my-theme-fonts', 'queue' ) && 'preconnect' === $relation_type ) {
		$urls[] = array(
			'href' => 'https://fonts.gstatic.com',
			'crossorigin',
		);
	}

	return $urls;
}
add_filter( 'wp_resource_hints', 'mytheme_resource_hints', 10, 2 );

There is nothing duplicative about specifically having that in there.

Ideally, WordPress would have a extra param on wp_register_style() which would take in that $urls[] section and automatically append it when the stylesheet was enqueued - but that should be proposed in a different ticket instead.

#7 @sami.keijonen
7 years ago

Ah sorry I was talking about duplicating the filter wp_resource_hints but that does make it a little different than duplicating a function.

I might also be misunderstanding how preconnect works.

Note: See TracTickets for help on using tickets.