Output CSS are not generated when using `option` mode

[daviedR]

Issue description:

When using option mode, the new version of Kirki seemed failed to generate the output CSS. Please check the snippet below. After changing the value via Customizer, the value is saved in the correct option key in the wp_options table, but no inline CSS is printed on the frontend's page source.

This works find on older version.

Version used:

4.0.21

Using theme_mods or options?

options

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

new \Kirki\Field\Color( array(
    'settings'    => 'color_button_bg',
    'option_type' => 'option',
    'option_name' => 'mytheme_options',
    'section'     => 'styles',
    'type'        => 'color',
    'label'       => esc_html__( 'Button BG color', 'jackrose' ),
    'choices'     => array( 'alpha' => true ),
    'default'     => '#b4d2c8',
    'transport'   => 'auto',
    'output'      => array(
        array(
            'element'  => '.button, button, input[type="button"], input[type="reset"], input[type="submit"]',
            'property' => 'background-color',
        ),
    ),
) );

Possible bug location

I think the bug is from the CSS generator. Here's what I found.

If we looked at the kirki/packages/kirki-framework/module-css/src/CSS/Generator.php file

self::$value = apply_filters( 'kirki_get_value', get_theme_mod( $field['settings'], $default ), $field['settings'], $default, $option_type );

This line will use theme_mod value by default. But it also provides kirki_get_value filter there, which is then used by kirki/packages/kirki-framework/data-option/src/Option.php file to modify the value for option mode.

public function kirki_get_value( $value = '', $option = '', $default = '', $type = 'theme_mod' ) {

    if ( 'option' === $type ) {

        /**
         * If the option doesn't contain a '[', then it's not a sub-item
         * of another option. Get the option value and return it.
         */
        if ( false === strpos( $option, '[' ) ) {
            return get_option( $option, $default );
        }

        /**
         * If we got here then this is part of an option array.
         * We need to get the 1st level, and then find the item inside that array.
         */
        $parts = \explode( '[', $option );
        $value = get_option( $parts[0], [] );

        foreach ( $parts as $key => $part ) {
            /**
             * Skip the 1st item, it's already been dealt with
             * when we got the value initially right before this loop.
             */
            if ( 0 === $key ) {
                continue;
            }

            $part = str_replace( ']', '', $part );

            /**
             * If the item exists in the value, then change $value to the item.
             * This runs recursively for all parts until we get to the end.
             */
            if ( is_array( $value ) && isset( $value[ $part ] ) ) {
                $value = $value[ $part ];
                continue;
            }

            /**
             * If we got here, the item was not found in the value.
             * We need to change the value accordingly depending on whether
             * this is the last item in the loop or not.
             */
            $value = ( isset( $parts[ $key + 1 ] ) ) ? [] : '';
        }
    }

    return $value;

}

This filter checks if the provided mode is option and have sub items (with [ character found in the settings name), then try to decompose the settings name. The problem is your CSS generator doesn't provide the full format of the settings name.

self::$value = apply_filters( 'kirki_get_value', get_theme_mod( $field['settings'], $default ), $field['settings'], $default, $option_type );

This line is the one responsible. $field['settings'] only contains the sub-key, not the option key. Therefore, the filter will always return get_option( 'color_button_bg' ), which does not exist.

I think the code should be:

$setting_name = $field['settings'];
if ( 'option' === $field['option_type'] ) {
    $setting_name = $field['option_name'] . '[' . $field['settings'] . ']';
}
self::$value = apply_filters( 'kirki_get_value', get_theme_mod( $field['settings'], $default ), $setting_name, $default, $option_type );

This way your filter will work fine.

Another workaround

I tried another workaround from the theme / plugin end. If we add the option_name as prefix of the settings_name attribute when adding the field, it works. e.g. mytheme_options[color_button_bg] instead of just color_button_bg.

new \Kirki\Field\Color( array(
    'settings'    => 'mytheme_options[color_button_bg]',
    'option_type' => 'option',
    'option_name' => 'mytheme_options',
    'section'     => 'styles',
    'type'        => 'color',
    'label'       => esc_html__( 'Button BG color', 'jackrose' ),
    'choices'     => array( 'alpha' => true ),
    'default'     => '#b4d2c8',
    'transport'   => 'auto',
    'output'      => array(
        array(
            'element'  => '.button, button, input[type="button"], input[type="reset"], input[type="submit"]',
            'property' => 'background-color',
        ),
    ),
) );

This works fine, but like I said, older version doesn't have issues on this. So I think this bug should be fixed.

[mapsteps]

Hey @daviedR,

thank you for the detailed explanation! We were able to replicate the issue and are going to fix it asap. I'll keep you posted :)

[JiveDig]

Jumping it to say that I'm purposely excluding option_name param and using 'settings' => 'mytheme_options[color_button_bg]', for all my settings to save as a single option. Mostly because it makes my code easier since a bunch of settings are created dynamically based on post_types and taxonomies. I still hit the same issue as here. Testing the next update in a few.

[mapsteps]

We're releasing a fix in a couple minutes. Thanks for reporting this guys!

[JiveDig]

I saw 4.0.22 go out already, is that it? So far that has fixed it for me. Still doing more testing and will push an update to our beta testers as well.

[mapsteps]

yep, 4.0.22 takes care of that :) The update is now live in the WordPress repository.

[JiveDig]

Using 4.0.22, we now have 2 reports of the default value not being output. It only works if you first save a non-default value, then change to the default and save, then it seems to output css.

I'm going to try to test this tomorrow (it's late here now).

[daviedR]

Thanks for the update @MapSteps , we will do further test

[mapsteps]

@JiveDig, do you know what fields/controls are affected?

[JiveDig]

As of now it's the same field I originally hit the issue with, a radio buttonset.

[mapsteps]

Alright, thank you!

We'll do some more testing on the fields that don't output any CSS by default like checkbox, select field, radio-buttonset, etc.. Will report back asap :)

[mapsteps]

One more question.

Usually when setting a default when registering the control in Kirki, you would also pass the default in php like this: get_option( 'your_option', 'left' ); where 'left' is the default.

Can you please share an example of the field in question and how the output is handled on the frontend?

[JiveDig]

Sure, as soon as I get back to my computer in a couple hours.

We were seeing the issue even when hard-coding the default parameter value.

We are only using this value via the output parameter, so there is no other code besides the field registry.

[JiveDig]

Okay confirming I see this issue when no value is saved. Here is a simplified version of my actual field, that still shows the issue of no CSS being output.

new \Kirki\Field\Radio_Buttonset(
    [
        'settings'        => mai_get_kirki_setting( 'header-right-menu-alignment' ), // This is actually 'mai-engine[header-right-menu-alignment]'
        'option_type'     => 'option',
        'section'         => $section,
        'label'           => __( 'Header Right Menu Alignment', 'mai-engine' ),
        'default'         => 'flex-end',
        'choices'         => [
            'flex-start' => __( 'Left', 'mai-engine' ),
            'center'     => __( 'Center', 'mai-engine' ),
            'flex-end'   => __( 'Right', 'mai-engine' ),
        ],
        'output'          => [
            [
                'element'  => '.header-right',
                'property' => '--menu-justify-content',
            ],
        ],
    ]
);

If I set the menu to non-default (center) and save, then it works. Then I can change it to Right (flex-end) and save it, and it works. The issue is that the CSS is not output when there is no value saved to the DB yet. It doesn't render the CSS from the default.

[JiveDig]

In the Generator class css() method there is:

self::$value = apply_filters( 'kirki_get_value', get_theme_mod( $field['settings'], $default ), $setting_name, $default, $option_type );

I'm not sure how get_theme_mod is able to get the correct option value, but dumping this gives the correct value (returns the default):

apply_filters( 'kirki_get_value', get_theme_mod( $field['settings'], $default ), $setting_name, $default, $option_type )

However, when I dump self::$value the default is not used, it returns empty.

[JiveDig]

Sorry I jumped the gun.

This does not work:

self::$value = apply_filters( 'kirki_get_value', get_theme_mod( $field['settings'], $default ), $setting_name, $default, $option_type );

However, removing the kirki_get_value filter does:

self::$value = get_theme_mod( $field['settings'], $default );

[JiveDig]

Pardon my documenting as I go...

The kirki_get_value filter in /data-option/src/Option.php Option class is the source of the issue.

In the scenario I'm hitting, my main option does have some values in it, just not the key for this specific field.

The conditions end up here (line 92):

/**
 * If we got here, the item was not found in the value.
 * We need to change the value accordingly depending on whether
 * this is the last item in the loop or not.
 */
$value = ( isset( $parts[ $key + 1 ] ) ) ? [] : '';

However, it never uses the $default here, it's either an empty array or an empty string. Somehow the $default should be used.

I'm not sure of the consenquences here as I'm not exactly sure what that $key + 1 is actually doing, but this works for me:

$value = ( isset( $parts[ $key + 1 ] ) ) ? [] : $default;

[JiveDig]

Final somewhat related thoughts... it seems like relying on kirki_get_value to check if it's an option or theme_mod is extra work. It's first retrieving the theme_mod then the filter is doing a secondary call to get the option. I'm feeling like there should be a helper function that only uses get_theme_mod or get_option, not both.

[JiveDig]

Something like this:

function kirki_get_setting( $setting_name, $default, $option_type = 'theme_mods', $option_name = '' ) {
    $value = null;

    if ( 'theme_mods' === $option_type ) {
        $value = get_theme_mod( $setting_name, $default );

    } elseif ( 'option' === $option_type ) {

        /**
         * If the option doesn't contain a '[', then it's not a sub-item
         * of another option. Get the option value and return it.
         */
        if ( false === strpos( $option_name, '[' ) ) {
            return get_option( $option_name, $default );
        }

        /**
         * If we got here then this is part of an option array.
         * We need to get the 1st level, and then find the item inside that array.
         */
        $parts = \explode( '[', $option_name );
        $value = get_option( $parts[0], [] );

        // If there's no value, return the default.
        if ( empty( $value ) ) {
            return $default;
        }

        foreach ( $parts as $key => $part ) {
            /**
             * Skip the 1st item, it's already been dealt with
             * when we got the value initially right before this loop.
             */
            if ( 0 === $key ) {
                continue;
            }

            $part = str_replace( ']', '', $part );

            /**
             * If the item exists in the value, then change $value to the item.
             * This runs recursively for all parts until we get to the end.
             */
            if ( is_array( $value ) && isset( $value[ $part ] ) ) {
                $value = $value[ $part ];
                continue;
            }

            /**
             * If we got here, the item was not found in the value.
             * We need to change the value accordingly depending on whether
             * this is the last item in the loop or not.
             */
            $value = ( isset( $parts[ $key + 1 ] ) ) ? [] : $default;
        }
    }

    return apply_filters( 'kirki_get_value', $value, $setting_name, $default, $option_type );
}

Then anywhere that has this:

self::$value = apply_filters( 'kirki_get_value', get_theme_mod( $field['settings'], $default ), $setting_name, $default, $option_type );

Can just do something like this:

self::$value = kirki_get_setting( $setting_name, $default, $option_type, $option_name );

[contactjavas]

Hi @JiveDig , thanks for your thoughts and codes! Just to re-confirm, in your use-case, this issue only happens to Radio_Buttonset field right?

[JiveDig]

Any of the fields with choices, but based on the scenario it's likely any field. If the main option is set at all, but a key isn't in that option, the default won't get used.

My fix from above is in place on some of our test sites and is working well for all types of fields.

[JiveDig]

Hey @contactjavas @MapSteps, is this issue on the radar? It's definitely a real bug. I've manually put in my patch by using the default in $value = ( isset( $parts[ $key + 1 ] ) ) ? [] : $default; but I think this or a similar fix needs to get in an official release. Thanks :)

[mapsteps]

@JiveDig, apologies for the delay. Time is a bit rare currently. We're getting back to Kirki asap.

[JiveDig]

No prob, thanks.

A TL;DR quick test to see the bug is to create 2 fields that save to the same option. Make sure a default is set when registering both. Then only update one of the fields. The updated field works but the default does not work for the field that doesn't have a value in the DB yet.

[daviedR]

Hi guys, any update on the default value issue?

[contactjavas]

Hi @daviedR , I think we've implemented the fix based on your suggestion. Have you tried the newest version?

The issue posted by @JiveDig hasn't been fixed though.