kirki icon indicating copy to clipboard operation
kirki copied to clipboard

Modifying new fields of repeater field in existing rows doesn't work

Open ArekZw opened this issue 7 years ago • 7 comments

Issue description:

  1. In Kirki: Create repeater with one field
  2. In customizer: create one row using repeater
  3. In Kirki: Add second field to repeater
  4. In customizer: modifying second field in the existing row doesn't work (after save and reloading the value disappears). Value is not saved in DB.

But if I create second row, second field in second row works.

Version used:

3.0.9 and develop branch

Using theme_mods or options?

options

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

Step 1:

	Kirki::add_panel('modules_global', array(
		'priority' => 1000,
		'title' => __('Modules - global', 'textdomain') ,
		'description' => __('', 'textdomain') ,
	));
	Kirki::add_config('theme_is_styles_global', array(
		'option_type' => 'option',
		'option_name' => 'is_global_modules_styles',
	));
	Kirki::add_section('modules_section_global', array(
		'title' => __('Test - Global') ,
		'description' => __('') ,
		'panel' => 'modules_global'
	));
	Kirki::add_field('theme_is_styles_global', array(
		'type' => 'repeater',
		'label' => esc_attr__(' ', 'my_textdomain') ,
		'section' => 'modules_section_global',
		'settings' => 'isb_modules_section_global',
		'option_type' => 'option',
		'fields' => array(
			'id' => array(
				'type' => 'text',
				'label' => esc_attr__('ID', 'my_textdomain') ,
				'default' => 'gl_',
				'description' => 'Pamiętaj o profiksie gl_',
			) ,
		);
		,
	));

Step 3:

	Kirki::add_panel('modules_global', array(
		'priority' => 1000,
		'title' => __('Modules - global', 'textdomain') ,
		'description' => __('', 'textdomain') ,
	));
	Kirki::add_config('theme_is_styles_global', array(
		'option_type' => 'option',
		'option_name' => 'is_global_modules_styles',
	));
	Kirki::add_section('modules_section_global', array(
		'title' => __('Test - Global') ,
		'description' => __('') ,
		'panel' => 'modules_global'
	));
	Kirki::add_field('theme_is_styles_global', array(
		'type' => 'repeater',
		'label' => esc_attr__(' ', 'my_textdomain') ,
		'section' => 'modules_section_global',
		'settings' => 'isb_modules_section_global',
		'option_type' => 'option',
		'fields' => array(
			'id' => array(
				'type' => 'text',
				'label' => esc_attr__('ID', 'my_textdomain') ,
				'default' => 'gl_',
				'description' => '',
			) ,
			'second_field' => array(
				'type' => 'text',
				'label' => esc_attr__('Here is problem', 'my_textdomain') ,
				'default' => '',
				'description' => '',
			) ,
		);
	));

ArekZw avatar Sep 21 '17 16:09 ArekZw

Confirmed. Just came across the same issue.

mapsteps avatar Aug 16 '18 11:08 mapsteps

Yes, this is a known issue. Unfortunately this will have to wait for the 3.1 refactor where repeaters will be rebuilt from scratch

aristath avatar Aug 23 '18 16:08 aristath

I've faced the same issue. It looks like it is because of the check from row 2938 in script.js:

if ( _.isUndefined( currentSettings[ row.rowIndex ][ fieldId ] ) ) {
			return;
}

This check aborts the updateField method (row 2916) if the field being updated has no previous value.

I don't know why this check is necessary. Is there any situations, when we should expect someone trying to add a setting that is not allowed? Shouldn't even in that case sanitization happen on the server side?

I've simply commented out that JS check as a workaround, but please let me know if you know any specific cases when that check is important! If not, I think a quick fix could be easily done by removing that check even before the 3.1 refactor release mentioned above.

BenceSzalai avatar Jan 03 '19 17:01 BenceSzalai

Is this a possible solution @aristath?

Thanks @Grapestain!

mapsteps avatar Jan 04 '19 22:01 mapsteps

Has anyone figured out how to fix this yet, by chance? I see the last comment here was almost 2 years ago and I am still having this issue. Commenting out the code snippet that @BenceSzalai posted above did not work for me.

Any help would be greatly appreciated!

jeradshepherd avatar Sep 08 '20 16:09 jeradshepherd

Probably, because you client side uses the minified JS. Look for script.min.js and try to make the same change on that too.

BenceSzalai avatar Sep 08 '20 21:09 BenceSzalai

Looks like it has moved since here: https://github.com/kirki-framework/kirki/blob/develop/packages/kirki-framework/control-repeater/src/assets/scripts/control.js#L736

BenceSzalai avatar Sep 08 '20 21:09 BenceSzalai