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.

Sign up for GitHub

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

Jump to bottom

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

Open
ArekZw opened this issue Sep 21, 2017 · 7 comments · May be fixed by #2348
Open

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

ArekZw opened this issue Sep 21, 2017 · 7 comments · May be fixed by #2348
Labels
control:repeater

Comments

@ArekZw
Copy link

ArekZw commented Sep 21, 2017 •
edited

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' => '',
			) ,
		);
	));
@mapsteps
Copy link
Contributor

Confirmed. Just came across the same issue.

@aristath
Copy link
Contributor

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

@ArekZw ArekZw mentioned this issue Nov 16, 2018
Closed
@BenceSzalai
Copy link

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.

@mapsteps
Copy link
Contributor

mapsteps commented Jan 4, 2019

Is this a possible solution @aristath?

Thanks @Grapestain!

@jeradshepherd
Copy link

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!

@BenceSzalai
Copy link

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
Copy link

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 added a commit to BenceSzalai/kirki that referenced this issue Sep 8, 2020
@BenceSzalai
Removed wrong check from updateField function
32509fa 
This fixes themeum#1540
This was referenced Sep 8, 2020
Closed
Open
@nervewax nervewax mentioned this issue Dec 10, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
control:repeater
Projects
None yet
Milestone
No milestone
6 participants
@DannyCooper @aristath @ArekZw @BenceSzalai @jeradshepherd @mapsteps