Opened 6 years ago
Closed 6 years ago
#39481 closed defect (bug) (wontfix)
Twenty Seventeen: Allowing Google fonts in child themes
Reported by: |
|
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)
Change History (8)
#2
@
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
#4
@
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
@
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
@
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
@
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.
any chance it'll be milestoned for 4.7.1 ? I understood that the last bug scrub is due in 3.5 hours.