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

Display typography with no default values #1797

Closed
anthonyryanrusso opened this issue Feb 8, 2018 · 17 comments
Closed

Display typography with no default values #1797

anthonyryanrusso opened this issue Feb 8, 2018 · 17 comments
Labels
Milestone

Comments

@anthonyryanrusso
Copy link

anthonyryanrusso commented Feb 8, 2018

Issue description:

Is there a way to have the fields still display on the frontend even if you don't have default values for them? I've tried but it always hides the field.

The issue I am running into is I have default font values set for body/h1-h6 so unless the user wants to override them they should fall back to original body/h1-h6 values that are already set.

Version used:

3.0.25

Using theme_mods or options?

theme_mods

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

Kirki::add_field($opt_name, array(
    'label' => esc_html__('Header top typography'),
    'section' => 'header_top_typography_section',
    'settings' => 'header_top_typography',
    'type' => 'typography',
    'required' => array(
	array(
            'setting' => 'header_top_visibility',
	    'operator' => 'contains',
	    'value' => array('mobile', 'tablet', 'desktop')
        )
    ),
    'default' => array(
        'font-family' => '',
        'font-size' => '',
        'line-height' => '',
        'text-transform' => '',
        'letter-spacing' => '',
        'color' => '#777'
    ),
    'output' => array(
        array(
            'element' => '#header-top',
            'property' => 'typography'
        )
    )
));

Update: I used "inherit" to get by for now, however this causes a few issues.

  1. Using "inherit" as the default font-family causes a js error in console.
  2. Using "inherit" might be confusing to an end user so there should be a way to leave the values blank and still have the field display for the user.
  3. Using "inherit" causes unnecessary css output.
@anthonyryanrusso
Copy link
Author

@aristath Any feedback on this?

@aristath
Copy link
Contributor

aristath commented Mar 5, 2018

Marked as a feature request. Default values will be needed, we just have to add support for empty defaults.

@anthonyryanrusso
Copy link
Author

Ok great, looking forward to this.

@aristath
Copy link
Contributor

Implemented in the develop branch, will be included in v3.0.26. You will now be able to use empty defaults 👍

@anthonyryanrusso
Copy link
Author

anthonyryanrusso commented Mar 12, 2018

@aristath It looks like all values work empty except for color. Color outputs white if it's blank.

aristath added a commit that referenced this issue Mar 12, 2018
@aristath
Copy link
Contributor

I just pushed a commit just now that should fix it, could you please pull again and check? (apologies, I only had 5 minutes to spare so no time to test it)

@anthonyryanrusso
Copy link
Author

Works good now.

@aristath
Copy link
Contributor

thanks for checking 👍

@aristath aristath added this to the 3.0.26 milestone Mar 17, 2018
aristath added a commit that referenced this issue Mar 17, 2018
* develop: (73 commits)
  fixes #1730
  fixes #1830
  GDPR: Load webfont-loader locally
  Update fonts
  fixes #1834
  Apply WordPress Coding Standards
  Update kirki-helper-class.md
  see #1797
  cleanup unused vars
  See #1807
  Additional fix for #1809
  fixes #1828
  fixes #1808
  fix #1814
  fix #1797
  fixes #1809
  Update sortable.md
  fixes #1787
  update webfonts & grunt
  changelog
  ...

# Conflicts:
#	modules/postmessage/class-kirki-modules-postmessage.php
@JiveDig
Copy link

JiveDig commented Aug 10, 2018

I'm having this exact issue in 3.0.33. I can make a new issue if you'd rather not re-open this one.

The following fields do not work when the values are empty:

'font-size'      => '',
'letter-spacing' => '',
'text-align'     => '',
'text-transform' => '',

This does not change anything in preview or after save:

/**
 * Footer Widget Titles
 */
Kirki::add_field( $config_id, array(
	'type'      => 'typography',
	'settings'  => 'footer_widget_titles',
	'label'     => esc_attr__( 'Footer Widget Titles', 'mai-fonts' ),
	'section'   => 'widgets',
	'transport' => 'auto',
	'default'   => array(
		'font-family'    => '',
		'variant'        => '',
		'font-size'      => '',
		'letter-spacing' => '',
		'text-align'     => '',
		'text-transform' => '',
	),
	'output' => array(
		array(
			'element' => array( '.footer-widgets .widgettitle', '.footer-widgets .widget-title' ),
		),
	),
) );

Yet this does work, however I don't want to set a default value as my theme already has these defaults in the base stylesheet.

/**
 * Footer Widget Titles
 */
Kirki::add_field( $config_id, array(
	'type'      => 'typography',
	'settings'  => 'footer_widget_titles',
	'label'     => esc_attr__( 'Footer Widget Titles', 'mai-fonts' ),
	'section'   => 'widgets',
	'transport' => 'auto',
	'default'   => array(
		'font-family'    => '',
		'variant'        => '',
		'font-size'      => 'inherit',
		'letter-spacing' => '0',
		'text-align'     => 'left',
		'text-transform' => 'none',
	),
	'output' => array(
		array(
			'element' => array( '.footer-widgets .widgettitle', '.footer-widgets .widget-title' ),
		),
	),
) );

Update: I just tested this on the develop branch and the issue remains.

@aristath aristath reopened this Aug 23, 2018
@aristath
Copy link
Contributor

@JiveDig I just tested the develop branch and this seems to be working fine... Can you please try this on a fresh installation? Perhaps there's something wrong with the save theme-options

@JiveDig
Copy link

JiveDig commented Aug 23, 2018

I will try again. I am saving in a option/array not a theme_mod if that matters. I’ll try again though and report back.

@JiveDig
Copy link

JiveDig commented Aug 27, 2018

I'm still having all sorts of issues. Just tested on a clean install. develop and master branch show the same results.

This is the exact code I'm using right now. The output only works for font-family and variant.

add_action( 'init', 'maifonts_register_customizer_settings' );
function maifonts_register_customizer_settings() {

	if ( ! class_exists( 'Kirki' ) ) {
		return;
	}

	$config_id      = 'mai_fonts';
	$panel_id       = 'mai_fonts';
	$settings_field = 'mai_fonts';

	Kirki::add_config( $config_id, array(
		'capability'  => 'edit_theme_options',
		'option_type' => 'option',
		'option_name' => $settings_field,
	) );

	/**
	 * Mai Fonts
	 */
	Kirki::add_panel( $panel_id, array(
		'title' => esc_attr__( 'Mai Fonts', 'mai-fonts' ),
	) );

	/* *********************************************** *
	 * Headings                                        *
	 * *********************************************** */
	Kirki::add_section( 'maifonts_headings', array(
		'title' => esc_attr__( 'Headings', 'mai-fonts' ),
		'panel' => $panel_id,
	) );

	/**
	 * Banner Title
	 */
	Kirki::add_field( $config_id, array(
		'type'      => 'typography',
		'settings'  => 'banner_title',
		'label'     => esc_attr__( 'Banner Title', 'mai-fonts' ),
		'section'   => 'maifonts_headings',
		'transport' => 'auto',
		'choices'   => array(
			'fonts' => array(
				'google' => array( 'popularity', 30 ),
			),
		),
		'default' => array(
			'font-family'    => '',
			'variant'        => '',
			'font-size'      => '',
			'letter-spacing' => '',
			'text-transform' => '',
		),
		'output' => array(
			array(
				'element' => array( 'h1', '.banner-title' ),
			),
		),
	) );

}

With no default values set the font size, letter spacing, and text transform don't work at all when I add a value.

@JiveDig
Copy link

JiveDig commented Aug 28, 2018

Here is an even more simplified version that has the same issues.

add_action( 'init', 'maifonts_register_customizer_settings' );
function maifonts_register_customizer_settings() {

	if ( ! class_exists( 'Kirki' ) ) {
		return;
	}

	$config_id = 'mai_fonts';

	Kirki::add_config( $config_id, array(
		'capability'  => 'edit_theme_options',
		'option_type' => 'option',
		'option_name' => 'mai_fonts',
	) );

	/* ******** *
	 * Headings *
	 * ******** */
	Kirki::add_section( 'maifonts_headings', array(
		'title' => esc_attr__( 'Headings', 'textdomain' ),
	) );

	// Banner Title
	Kirki::add_field( $config_id, array(
		'type'      => 'typography',
		'settings'  => 'banner_title',
		'label'     => esc_attr__( 'Heading 1', 'textdomain' ),
		'section'   => 'maifonts_headings',
		'transport' => 'auto',
		'choices'   => array(
			'fonts' => array(
				'google' => array( 'popularity', 30 ),
			),
		),
		'default' => array(
			'font-family'    => '',
			'variant'        => '',
			'font-size'      => '',
			'letter-spacing' => '',
			'text-transform' => '',
		),
		'output' => array(
			array(
				'element' => array( 'h1', '.banner-title' ),
			),
		),
	) );

}

A new find, if I add values and save them, then refresh the page/customizer, the values are empty for everything but font-family (the only working field to begin with.

@JiveDig
Copy link

JiveDig commented Aug 28, 2018

Sorry for all the posts, just testing (and documenting) various things while i'm at it.

Changing the field to this:

	Kirki::add_field( $config_id, array(
		'type'      => 'typography',
		'settings'  => 'banner_title',
		'label'     => esc_attr__( 'Heading 1', 'textdomain' ),
		'section'   => 'maifonts_headings',
		'transport' => 'auto',
		'choices'   => array(
			'fonts' => array(
				'google' => array( 'popularity', 30 ),
			),
		),
		'default' => array(
			// 'font-family'    => '',
			// 'variant'        => '',
			'font-size'      => '',
			'letter-spacing' => '',
			'text-transform' => '',
		),
		'output' => array(
			array(
				'element' => array( 'h1', '.banner-title' ),
			),
		),
	) );

correctly displays this:
screen shot 2018-08-28 at 11 58 26 am
but doesn't allow save/publish. No matter what value I change, I can't save the field.

@JiveDig
Copy link

JiveDig commented Aug 28, 2018

Confirming same issues all around with 'option_type' => 'theme_mod',

@aristath
Copy link
Contributor

aristath commented Sep 1, 2018

Should be ok now for the reported issues.

@JiveDig
Copy link

JiveDig commented Sep 10, 2018

Confirming this all works well for me now #1869 in develop branch.

Assuming/hoping #1869 is a similar (easy?) fix too since it's similar behaviour?

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

No branches or pull requests

3 participants