Checkbox style toggling doesn't work correctly in value_pattern situation #1778

kodeoagency · 2018-01-27T10:17:50Z

Depending on the combination of the default and exclude value, styling toggling with a checkbox (switch on/off) is working or not.

You can use this code to reproduce the problem.

Kirki::add_config( 'my_config', array(
	'capability'  => 'edit_theme_options',
	'option_type' => 'theme_mod',
) );

Kirki::add_section( 'my_section', array(
    'title'          => esc_attr__( 'My Section', 'textdomain' ),
) );

Kirki::add_field( 'my_config', [
	'settings' => 'h1_italic',
	'type'     => 'checkbox',
	'default'  => 1,
	'transport' => 'auto',
	'output'   => [
		[
			'element'     => 'h1',
			'property'    => 'font-style',
			'value_pattern' => 'italic',
			'exclude'       => [ false, 0, '0' ],
		]
	],
	'section'  => 'my_section',
] );

To make the problem clearer:

THIS IS WORKING:

Kirki::add_field( 'my_config', [
	'settings' => 'h1_italic',
	'type'     => 'checkbox',
	'default'  => 0,
	'transport' => 'auto',
	'output'   => [
		[
			'element'     => 'h1',
			'property'    => 'font-style',
			'value_pattern' => 'italic',
			'exclude'       => [ false, 0, '0' ],
		]
	],
	'section'  => 'my_section',
] );

THIS IS NOT WORKING:

Kirki::add_field( 'my_config', [
	'settings' => 'h1_italic',
	'type'     => 'checkbox',
	'default'  => 1,
	'transport' => 'auto',
	'output'   => [
		[
			'element'     => 'h1',
			'property'    => 'font-style',
			'value_pattern' => 'italic',
			'exclude'       => [ false, 0, '0' ],
		]
	],
	'section'  => 'my_section',
] );

THIS IS WORKING:

Kirki::add_field( 'my_config', [
	'settings' => 'h1_italic',
	'type'     => 'checkbox',
	'default'  => 1,
	'transport' => 'auto',
	'output'   => [
		[
			'element'     => 'h1',
			'property'    => 'font-style',
			'value_pattern' => 'italic',
			'exclude'       => [ true, 1, '1' ],
		]
	],
	'section'  => 'my_section',
] );

THIS IS NOT WORKING:

Kirki::add_field( 'my_config', [
	'settings' => 'h1_italic',
	'type'     => 'checkbox',
	'default'  => 0,
	'transport' => 'auto',
	'output'   => [
		[
			'element'     => 'h1',
			'property'    => 'font-style',
			'value_pattern' => 'italic',
			'exclude'       => [ true, 1, '1' ],
		]
	],
	'section'  => 'my_section',
] );

The text was updated successfully, but these errors were encountered:

kodeoagency · 2018-02-06T10:03:20Z

Hi Aristath,
due to your suggestion in Issue #1787 I have re-structured the code so that it's easy for you to copy and paste it your test installation.

aristath · 2018-02-06T18:16:47Z

@kodeoagency thank you for taking the time to format the code to make it easier to test, I really really really appreciate it! I can test things easier this way and have the time to reply to more people ❤️

This works in all cases:

Kirki::add_config( 'my_config', array(
	'capability'  => 'edit_theme_options',
	'option_type' => 'theme_mod',
) );

Kirki::add_section( 'my_section', array(
    'title'          => esc_attr__( 'My Section', 'textdomain' ),
) );

Kirki::add_field( 'my_config', [
	'settings' => 'h1_italic',
	'type'     => 'checkbox',
	'default'  => 1,
	'transport' => 'auto',
	'output'   => [
		[
			'element'     => 'h1',
			'property'    => 'font-style',
			'value_pattern' => 'italic',
			'exclude'       => [ false, 0, '0' ],
		],
		[
			'element'     => 'h1',
			'property'    => 'font-style',
			'value_pattern' => 'normal',
			'exclude'       => [ true, 1, '1' ],
		]
	],
	'section'  => 'my_section',
] );

If you define what you want both on and off cases to do, then no matter what the default value is, it will work.
The logic:
Suppose the checkbox is set. In that case, it will be italic. If the checkbox is then turned off and you don't have the 2nd array in output, it will not do anything because there is no rule for the off value. It will not turn off the italic that was added So it will just leave the italic that was added previously when it was on because it just doesn't know what to do. There are no rules for off, so it will completely skip processing things.
I hope that makes sense...

kodeoagency · 2018-02-07T08:43:55Z

Hi Aristath.
You are right with the second array, but unfortunately, it's not working in all orders.

Let me show you:

THIS IS WORKING:

Kirki::add_field( 'my_config', [
	'settings' => 'h1_italic',
	'type'     => 'checkbox',
	'default'  => 1,
	'transport' => 'auto',
	'output'   => [
		[
			'element'     => 'h1',
			'property'    => 'font-style',
			'value_pattern' => 'italic',
			'exclude'       => [ false, 0, '0' ],
		],
		[
			'element'     => 'h1',
			'property'    => 'font-style',
			'value_pattern' => 'normal',
			'exclude'       => [ true, 1, '1' ],
		]
	],
	'section'  => 'my_section',
] );

THIS IS NOT WORKING

Kirki::add_field( 'my_config', [
	'settings' => 'h1_italic',
	'type'     => 'checkbox',
	'default'  => 1,
	'transport' => 'auto',
	'output'   => [
		[
			'element'     => 'h1',
			'property'    => 'font-style',
			'value_pattern' => 'normal',
			'exclude'       => [ true, 1, '1' ],
		],
		[
			'element'     => 'h1',
			'property'    => 'font-style',
			'value_pattern' => 'italic',
			'exclude'       => [ false, 0, '0' ],
		],
	],
	'section'  => 'my_section',
] );

For us this is a huge Problem, because in our parent theme we have more than 500 controls and the default values will be loaded from different child themes config files, so that there is always a mixed situation.

Is there a way to fix this? Should I open a new issue?

aristath · 2018-02-07T09:30:56Z

extremely weird.
Reopened the issue and I'll take a look later today. 👍
Thank you for being so thorough! Helps us build a bug-free plugin

aristath · 2018-03-11T11:32:52Z

I was able to replicate the issue, but only when all the below conditions were met:

transport was set to auto
default was set to false
The output with the false exclusion was before the one with the true exclusion.

So this is the only combination that I found was causing the issue:

'default'     => false,
'transport'   => 'auto',
'output'      => array(
	array(
		'element'     => 'h1',
		'property'    => 'font-style',
		'value_pattern' => 'italic',
		'exclude'       => [ false, 0, '0' ],
	),
	array(
		'element'     => 'h1',
		'property'    => 'font-style',
		'value_pattern' => 'normal',
		'exclude'       => [ true, 1, '1' ],
	),
),

This is probably caused by a bug in the class-kirki-modules-postmessage.php which will soon be refactored completely so I know it's not what you want to hear, but I'm not going to work on fixing it because the new implementation will fix it anyway. The current code in there is too chaotic to debug and tracking this down will take a lot of time.

Workarounds until the refactor of the postMessage module is completed:

Workaround 1: There's no reason to have exclude in both args, this would work too:

'default'     => false,
'transport'   => 'auto',
'output'      => array(
	array(
		'element'     => 'h1',
		'property'    => 'font-style',
		'value_pattern' => 'normal',
	),
	array(
		'element'     => 'h1',
		'property'    => 'font-style',
		'value_pattern' => 'italic',
		'exclude'       => [ false, 0, '0' ],
	),
),

Workaround 2:
in the stylesheet have

h1 { font-style: normal; }

and then in the control only include what overrides the default:

'default'     => false,
'transport'   => 'auto',
'output'      => array(
	array(
		'element'     => 'h1',
		'property'    => 'font-style',
		'value_pattern' => 'italic',
		'exclude'       => [ false, 0, '0' ],
	),
),

Workaround 3:
Use a radio-buttonset instead of checkbox:

Kirki::add_field( 'my_config', [
	'settings'  => 'h1_font_style',
	'section'   => 'my_section',
	'type'      => 'radio-buttonset',
	'default'   => 'normal',
	'transport' => 'auto',
	'output'    => [
		[
			'element'     => 'h1',
			'property'    => 'font-style',
		],
	],
	'choices'   => [
		'normal' => esc_attr__( 'Normal', 'textdomain' ),
		'italic' => esc_attr__( 'Italic', 'textdomain' ),
	],
] );

I'm hoping I'll be able to complete the refactor soon.

kodeoagency · 2018-03-12T09:47:34Z

Hello.
I totally understand you not fixing this since you will refactor this module.
It's very nice of you to give me this workaround.
THANK YOU SO MUCH!
I really appreciate!

aristath · 2018-03-17T15:07:05Z

@kodeoagency I tagged you on the ticket here #1709 for the postMessage module refactor but someone reported they did not get notified from that so I'm also posting it here so you can check it out when you have some time to spare. 👍

kodeoagency · 2018-03-20T21:09:58Z

Thank you very much.
I will test the new branch as soon as possible.

aristath closed this as completed Feb 6, 2018

aristath reopened this Feb 7, 2018

kodeoagency closed this as completed Mar 12, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Checkbox style toggling doesn't work correctly in value_pattern situation #1778

Checkbox style toggling doesn't work correctly in value_pattern situation #1778

kodeoagency commented Jan 27, 2018 •

edited

Loading

kodeoagency commented Feb 6, 2018

aristath commented Feb 6, 2018

kodeoagency commented Feb 7, 2018

aristath commented Feb 7, 2018

aristath commented Mar 11, 2018

kodeoagency commented Mar 12, 2018

aristath commented Mar 17, 2018

kodeoagency commented Mar 20, 2018

Checkbox style toggling doesn't work correctly in value_pattern situation #1778

Checkbox style toggling doesn't work correctly in value_pattern situation #1778

Comments

kodeoagency commented Jan 27, 2018 • edited Loading

kodeoagency commented Feb 6, 2018

aristath commented Feb 6, 2018

kodeoagency commented Feb 7, 2018

aristath commented Feb 7, 2018

aristath commented Mar 11, 2018

kodeoagency commented Mar 12, 2018

aristath commented Mar 17, 2018

kodeoagency commented Mar 20, 2018

kodeoagency commented Jan 27, 2018 •

edited

Loading