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

Google font output broken in 3.0.8 #1443

Closed
colmtroy opened this issue Jun 29, 2017 · 11 comments
Closed

Google font output broken in 3.0.8 #1443

colmtroy opened this issue Jun 29, 2017 · 11 comments

Comments

@colmtroy
Copy link

Issue description:

There are font weights missing in the Google Font url being called in 3.0.8.
Example:

In 2.3.8 the Google font url rendered in source is:

'https://fonts.googleapis.com/css?family=Nunito%3A300%7CNunito+Sans%3A300%2C600%2Cregular%7CScope+One&subset=latin-ext

In 3.0.8 the url is

'https://fonts.googleapis.com/css?family=Nunito%3A300%7CNunito+Sans%3A300%2Cregular%7CScope+One&subset=latin-ext%2Clatin-ext%2Clatin-ext%2Clatin-ext%2Clatin-ext%2Clatin-ext%2Clatin-ext%2Clatin-ext%2Clatin-ext%2Clatin-ext%2Clatin-ext%2Clatin-ext%2Clatin-ext%2Clatin-ext%2Clatin-ext%2Clatin-ext%2Clatin-ext%2Clatin-ext%2Clatin-ext%2Clatin-ext%2Clatin-ext%2Clatin-ext%2Clatin-ext%2Clatin-ext%2Clatin-ext%2Clatin-ext%2Clatin-ext%2Clatin-ext%2Clatin-ext%2Clatin-ext%2Clatin-ext%2Clatin-ext%2Clatin-ext%2Clatin-ext%2Clatin-ext%2Clatin-ext

Version used:

3.0.8

Using theme_mods or options?

// h1.
accentuate_Kirki::add_field( 'accentuate_config', array(
	'type'			 => 'typography',
	'settings'		 => 'accentuate_typography_h1_fontfamily',
	'label'			 => esc_html__( 'Font Settings', 'accentuate' ),
	'description'	 => esc_html__( 'Select which font you would like to use', 'accentuate' ),
	'section'		 => 'accentuate_typography_section_headings_h1',
	'default'		 => array(
		'font-family'	 => 'Nunito Sans',
		'font-weight'	 => '600',
		'font-size'		 => '46px',
		'line-height'	 => '1.2',
		'letter-spacing' => '-0.3px',
		'subsets'		 => array( 'latin-ext' ),
		'color'			 => '#4F2D7F',
		'text-transform' => 'none',
	),
	'priority'		 => 20,
	'output'		 => array(
		array(
			'element'	 => 'h1, .ccfw-site-description',
			'property'	 => 'font-family',
		),
	),
) );

Code to reproduce the issue (config + field(s))

@aristath
Copy link
Contributor

aristath commented Jun 30, 2017

What exactly is missing?
Those missing parts, are they set anywhere in your options?

@colmtroy
Copy link
Author

hey @aristath - if you look at the 2 url strings I posted you can see the difference. In 3.0.8 the font weight "600" is missing - and you can see the "%2Clatin-ext" is being repeated many times.

@aristath
Copy link
Contributor

Do you have the 600 font-weight selected in the customizer for that font?

@colmtroy
Copy link
Author

@aristath yes - and as soon as I switch back to 2.3.8 it works just fine.

@aristath
Copy link
Contributor

I just tested using your control and this is what I'm getting:

<link rel='stylesheet' id='kirki_google_fonts-css'  href='https://fonts.googleapis.com/css?family=Nunito+Sans%3A600&#038;subset=latin-ext' type='text/css' media='all' />

The URL is https://fonts.googleapis.com/css?family=Nunito+Sans%3A600&#038;subset=latin-ext which looks perfectly fine to me.
The code of that single typography field works fine.
Do you have any extra typography fields?
Any custom code for typography options? Custom fonts or anything related to typography options?

@colmtroy
Copy link
Author

colmtroy commented Jul 5, 2017

@aristath - we're definitely seeing very different results :/
To rule out something else conflicting from our theme I took the following steps:
1.) Downloaded your fork of _s from here
https://github.com/aristath/_s
2.) Added the following to customizer.php

_s_Kirki::add_field( '_s_theme', array(
	'type'			 => 'typography',
	'settings'		 => 'accentuate_typography_h1_fontfamily',
	'label'			 => esc_html__( 'Font Settings', 'accentuate' ),
	'description'	 => esc_html__( 'Select which font you would like to use', 'accentuate' ),
	'section'		 => 'accentuate_typography_section_headings_h1',
	'default'		 => array(
		'font-family'	 => 'Nunito Sans',
		'font-weight'	 => '600',
		'font-size'		 => '46px',
		'line-height'	 => '1.2',
		'letter-spacing' => '-0.3px',
		'subsets'		 => array( 'latin-ext' ),
		'color'			 => '#4F2D7F',
		'text-transform' => 'none',
	),
	'priority'		 => 20,
	'output'		 => array(
		array(
			'element'	 => 'h1, .ccfw-site-description',
			'property'	 => 'font-family',
		),
	),
) );

I would expect output similar to what you reported and what I see with 2.3.8 - but with 3.0.8 I see this:

<link rel='stylesheet' id='kirki_google_fonts-css'  href='https://fonts.googleapis.com/css?family=Roboto%7CNunito+Sans%3Aregular&#038;subset=latin-ext%2Clatin-ext' type='text/css' media='all' />

So The Nunito Sans weight should be 600 - not regular

Any ideas?

@colmtroy
Copy link
Author

colmtroy commented Jul 5, 2017

With further testing - I can see the default font value is being set in the Customizer as "400" rather than "600" - this is on a brand new WP install with your fork of _s using the code above and before I save make any changes in the customizer (i.e. the default values). If I then change to 600 and save it loads the correct font weight - so the issue seems to be related to the loading of the default font values (which is definitely not an issue in 2.3.8)

@aristath
Copy link
Contributor

aristath commented Jul 5, 2017

Can you please try this on a plain _s theme (not my fork)?
The fork still needs to be updated.

@colmtroy
Copy link
Author

colmtroy commented Jul 5, 2017

@aristath - yip - issue persists.
Steps to recreate:
1.) Downloaded a brand new _s theme instance
2.) Added basic Kirki config - feel free to download a copy of what I'm using here
https://www.dropbox.com/s/p9lxnq33g5nju5s/kirkidebug.zip?dl=0
3.) Installed Kirki plugin
4.) Go to frontend
http://themedemo.commercegurus.com/kirkifontdebug/
5.) Observe this code in source (incorrect output)

<link rel='stylesheet' id='kirki_google_fonts-css'  href='https://fonts.googleapis.com/css?family=Roboto%7CNunito+Sans%3Aregular&#038;subset=latin-ext%2Clatin-ext' type='text/css' media='all' />

6.) Switch to Kirki 2.3.8
7.) Refresh homepage, observe this in source (correct output)

<link rel='stylesheet' id='kirki_google_fonts-css'  href='https://fonts.googleapis.com/css?family=Roboto%7CNunito+Sans%3A600&#038;subset=latin-ext' type='text/css' media='all' />

So in summary, the correct font weight is not being set in 3.0.8 by default. If you need anything else let me know.

Thanks,
Colm

aristath added a commit that referenced this issue Jul 5, 2017
@aristath
Copy link
Contributor

aristath commented Jul 5, 2017

ok, I see the problem now.

The issue is that one field uses variant in the default values while the other uses font-weight.
If you use variant for both fields it works fine.

I just pushed a commit that will add compatibility and allow you to use font-weight in the default values, can you please pull the latest develop branch and check if that does the trick? It seems to work now on my localhost.

@colmtroy
Copy link
Author

colmtroy commented Jul 5, 2017

@aristath - excellent - thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants