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

Standard Fonts in Typography field throw an error #1370

Closed
hellor0bot opened this Issue Jun 9, 2017 · 12 comments

Comments

Projects
None yet
3 participants
@hellor0bot

hellor0bot commented Jun 9, 2017

Issue description:

When using Standard Fonts and the output parameter, we're getting the error:

Notice: Array to string conversion in /kirki/modules/css/field/class-kirki-output-field-typography.php on line 65

Generated CSS:

font-family: Array;

Version used:

develop branch

@aristath

This comment has been minimized.

Show comment
Hide comment
@aristath

aristath Jun 9, 2017

Owner

Can you post the code you're using?
I'll deal with all typography issues this weekend so having some code to test against would make things easier 👍

Owner

aristath commented Jun 9, 2017

Can you post the code you're using?
I'll deal with all typography issues this weekend so having some code to test against would make things easier 👍

@hellor0bot

This comment has been minimized.

Show comment
Hide comment
@hellor0bot

hellor0bot Jun 9, 2017

Sure!

But there's really nothing special:

Kirki::add_field( 'theme_name_theme_mod', array(
	'type'        => 'typography',
	'settings'    => 'typography_headings',
	'label'       => esc_html__( 'Heading', 'text-domain' ),
	'section'     => 'typography_headings',
	'default'     => array(
		'font-family'    => 'Montserrat',
		'variant'        => '600',
		'subsets'        => array( 'latin-ext' ),
		),
	'priority'    => 10,
	'output'      => array(
		array(
			'element' => 'h1, h2, h3, h4, h5, h6',
			),
		),
) );

hellor0bot commented Jun 9, 2017

Sure!

But there's really nothing special:

Kirki::add_field( 'theme_name_theme_mod', array(
	'type'        => 'typography',
	'settings'    => 'typography_headings',
	'label'       => esc_html__( 'Heading', 'text-domain' ),
	'section'     => 'typography_headings',
	'default'     => array(
		'font-family'    => 'Montserrat',
		'variant'        => '600',
		'subsets'        => array( 'latin-ext' ),
		),
	'priority'    => 10,
	'output'      => array(
		array(
			'element' => 'h1, h2, h3, h4, h5, h6',
			),
		),
) );
@vlthemes

This comment has been minimized.

Show comment
Hide comment
@vlthemes

vlthemes Jun 10, 2017

Same mistake

vlthemes commented Jun 10, 2017

Same mistake

@aristath aristath added this to the 3.0 milestone Jun 10, 2017

@aristath

This comment has been minimized.

Show comment
Hide comment
@aristath

aristath Jun 18, 2017

Owner

Fixed.
Thanks for reporting this!

Owner

aristath commented Jun 18, 2017

Fixed.
Thanks for reporting this!

@aristath aristath closed this Jun 18, 2017

aristath added a commit that referenced this issue Jun 18, 2017

@hellor0bot

This comment has been minimized.

Show comment
Hide comment
@hellor0bot

hellor0bot Jun 18, 2017

I've just tested it, and seems there's an issue with switching font-weight for standard fonts. The weight doesn't get changed :(

hellor0bot commented Jun 18, 2017

I've just tested it, and seems there's an issue with switching font-weight for standard fonts. The weight doesn't get changed :(

@hellor0bot

This comment has been minimized.

Show comment
Hide comment
@hellor0bot

hellor0bot Jun 18, 2017

For reproducing the issue, try changing between weights in Serif, then switch to Sans-Serif and change font-weights.

hellor0bot commented Jun 18, 2017

For reproducing the issue, try changing between weights in Serif, then switch to Sans-Serif and change font-weights.

@aristath

This comment has been minimized.

Show comment
Hide comment
@aristath

aristath Jun 21, 2017

Owner

@hellor0bot I just tested it and can't replicate the issue you're describing

Owner

aristath commented Jun 21, 2017

@hellor0bot I just tested it and can't replicate the issue you're describing

@hellor0bot

This comment has been minimized.

Show comment
Hide comment
@hellor0bot

hellor0bot Jun 23, 2017

I've just added transport: auto to all typography fields and removed line breaks in the element parameter and it's working fine now.

By the way, transport: auto on typography fields is so awesome. Great job!

hellor0bot commented Jun 23, 2017

I've just added transport: auto to all typography fields and removed line breaks in the element parameter and it's working fine now.

By the way, transport: auto on typography fields is so awesome. Great job!

@hellor0bot

This comment has been minimized.

Show comment
Hide comment
@hellor0bot

hellor0bot Jun 23, 2017

Nope, the issue is still there and I can't debug it :(

All fonts appear in the Customizer correctly with transport set to auto, however after saving a former (or incorrect) variant is displayed on the front-end.

hellor0bot commented Jun 23, 2017

Nope, the issue is still there and I can't debug it :(

All fonts appear in the Customizer correctly with transport set to auto, however after saving a former (or incorrect) variant is displayed on the front-end.

@hellor0bot

This comment has been minimized.

Show comment
Hide comment
@hellor0bot

hellor0bot Jun 23, 2017

The issue is related to all fonts, not standard ones, as I'd thought before.

hellor0bot commented Jun 23, 2017

The issue is related to all fonts, not standard ones, as I'd thought before.

@aristath aristath reopened this Jun 23, 2017

@aristath aristath closed this in 8ff2e8c Jun 23, 2017

@aristath

This comment has been minimized.

Show comment
Hide comment
@aristath

aristath Jun 23, 2017

Owner

Should be ok now.

Owner

aristath commented Jun 23, 2017

Should be ok now.

@hellor0bot

This comment has been minimized.

Show comment
Hide comment
@hellor0bot

hellor0bot Jun 26, 2017

Just checked it. All is good. Thanks a lot!

hellor0bot commented Jun 26, 2017

Just checked it. All is good. Thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment