Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#39481 closed defect (bug) (wontfix)

Twenty Seventeen: Allowing Google fonts in child themes

Reported by: tsvikas's profile tsvikas Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: has-patch
Focuses: Cc:

Description

In the latest version of WordPress:
If I try to redeclare twentyseventeen_fonts_url() in my child theme, in order to add more fonts to it, I cannot.
(In twenty sixteen it was possible)

it can be easily fixed by adding to the original theme's functions.php file:

if ( !function_exists( 'twentyseventeen_fonts_url' ) ):

before the function twentyseventeen_fonts_url(), and

endif;

after.

(similar code is used in the 'twenty sixteen' theme)

Attachments (1)

39481.patch (571 bytes) - added by johnpgreen 6 years ago.

Download all attachments as: .zip

Change History (8)

@johnpgreen
6 years ago

#1 @johnpgreen
6 years ago

  • Keywords has-patch added

#2 @SergeyBiryukov
6 years ago

  • Summary changed from allowing google fonts in childs of the 'twenty seventeen' theme to Twenty Seventeen: Allowing Google fonts in child themes

#3 @tsvikas
6 years ago

any chance it'll be milestoned for 4.7.1 ? I understood that the last bug scrub is due in 3.5 hours.

#4 @dd32
6 years ago

TwentySeventeen doesn't have pluggable functions (function_exists()), and I'd argue that it shouldn't.

You shouldn't be redefining twentyseventeen_fonts_url() in the child theme IMHO, but rather either unhooking, or adding an extra filter.

I'd be doing something like:

add_action( 'wp_enqueue_scripts', 'mychild_scripts' );
function mychild_scripts() {
   wp_enqueue_style( 'mychild-fonts', 'https://fonts.googleapis.com/css/....', array(), null );
   // optionally, remove the existing fonts:
   wp_dequeue_style( 'twentyseventeen-fonts' );
}

I'll leave this up to @karmatosed and @davidakennedy though

#5 @superpoincare
6 years ago

There are complications in dequeuing parent fonts in bundled themes. This is because they sometimes add a body class (such as twenty twelve) and all add a preconnect hint.

So dequeuing implies one has to add the body class again and also write code for preconnect.

This is how I do it in the child theme:

function twentytwelve_child_get_font_url( $src, $handle ) {
	if ( 'twentytwelve-fonts' === $handle ) {
		return str_replace( array( 'twentytwelve-fonts', 'latin,latin-ext' ), array( 'twentytwelve-child-fonts', 'latin,latin-ext,greek,greek-ext' ), $src );
	}
	return $src;
}
add_filter( 'style_loader_tag', 'twentytwelve_child_get_font_url', 10, 2 );

Making the functions in the parent theme pluggable has potential side effects. Suppose I change the font url in the child theme by overriding the hypothetical pluggable function with another font-service. The html output will still have the preconnect hints and a body class (in some themes).

This is because the code for preconnect and body class check to see if the font handle is in queue.

Hence the best way of modifying is by using style_loader_tag

#6 @superpoincare
6 years ago

Actually since

a) the child functions.php loads first and
b) that wp_enqueue_style doesn't override,

just this would do:

add_action( 'wp_enqueue_scripts', 'mychild_scripts' );
function mychild_scripts() {
   wp_enqueue_style( 'twentyseventeen-fonts', 'https://fonts.googleapis.com/css/....', array(), null );
}

Since anyway child functions.php enqueues other styles and scripts, the above is effectively just one line.

#7 @davidakennedy
6 years ago

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

Hey @tsvikas! Thanks for the request, and welcome to Trac!

I would agree with @dd32 here. If you're looking to override and/or add new fonts, you should do that in a new function and dequeue any styles if you need to do that.

Especially with fonts, many have different subsets and weights that may need to be handled, and it's easier to do that in a new function.

Also, with Twenty Seventeen, we went with less pluggable functions for simplicity sake. See: https://github.com/WordPress/twentyseventeen/pull/419 for some history.

With that said, I'm going to close this, since the preferred approach to modify this function is already possible.

Note: See TracTickets for help on using tickets.