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

Repeatable custom field inside repeatable group don't save properly #1035

Closed
bomsn opened this issue Oct 9, 2017 · 9 comments
Closed

Repeatable custom field inside repeatable group don't save properly #1035

bomsn opened this issue Oct 9, 2017 · 9 comments

Comments

@bomsn
Copy link

bomsn commented Oct 9, 2017

I created custom field based on the address field

Expected Behavior:

  • The data need to be saved properly, each field created need to be saved.

Actual Behavior:

  • When adding repeatable custom field, say 3 for example, after save I only find 2.
  • If there are for example 3 fields, and I added 4 other fields, after save, I only find 4 where the total should be 7
  • Sometimes it will save all fields except the last one
  • Saving doesn't work properly in general.

Steps to reproduce (I have confirmed I can reproduce this issue on the trunk branch):

  1. Create New Post
  2. Scroll to Book details metabox,
  3. Create 3 fields (or any other number), give them random names
  4. Update post (you'll see that only first 2 fields were saved)
  5. Now add another 3 fields and update (you'll see that only the first one of them was saved)
  6. If you add another group it will act the same way

I suspect it's javascript bug.

CMB2 Field Registration Code:

https://gist.github.com/bomsn/36174de42bfbaa74abcd5bce1bfc432d

@bomsn
Copy link
Author

bomsn commented Oct 9, 2017

When I removed HTML from the custom fields it will save the first group correctly, but when adding new group and creating 3 new fields in this second group, it will push the third field into a new group after updating the post (I updated the gist with the change)
https://gist.github.com/bomsn/36174de42bfbaa74abcd5bce1bfc432d

I thought it worth mentioning that **when looking at inputs IDs and the html for the fields in general, the js is giving wrong group IDs/Names ** , I'm not pretty sure, but I guess the issue goes with the 'cmb.elReplacements' function in cmb2.js

I also noticed if you click the "remove field" button on the an empty field (the only field in a group) it will create an empty field in the previous group.

  • the exact same issue happens with 'Text_Datetime_Timestamp_Timezone';

@bomsn
Copy link
Author

bomsn commented Oct 10, 2017

I was able to solve this by doing some changes in cmb.elReplacements function in the js file.
not sure if this is the most usable solution, but at least is solves my problem.
trunk...bomsn:patch-1

The solution was based on another fork by @nonsensecreativity nonsensecreativity@a15bf58

There is another issue tough, when trying to remove a field from empty group, it will create new empty field in previous group.

hope you can tackle these in the next release.

@tw2113
Copy link
Contributor

tw2113 commented Oct 10, 2017

If I recall right, CMB2 will visually provide at least 1 field, though it won't, or at least shouldn't, save anything with it visually shown. Only once items start getting filled in would anything save.

@bomsn
Copy link
Author

bomsn commented Oct 10, 2017

@tw2113 That is correct, I probably didn't explain this clearly. The issue I had is that the JS was not giving IDs/Names correctly which was causing issues when saving the post, similar issues were posted before:
#116
#521

I solved this, but I found another issue with cmb.removeAjaxRow function, when we have 3 groups for example, say first group have 3 field and second group have 2, and the 3rd is still empty (will have an empty field), say you clicked 'remove button' on the empty field inside the third group, this will trigger cmb.resetRow, and this function will trigger a click on remove row button and add row button at the same time, remove row trigger seem to be working correctly, however, add row button is not triggered within the third group only, but all other group, which will create additional field in all groups.

I tried changing parents to parent and closest in this variable:
var $parent = $this.parents('.cmb-row');
but this will always trigger infinite loop and cause Maximum call stack size exceeded error

So I ended up changing thisreturn cmb.resetRow( $parent.find( '.cmb-add-row-button' ), $this ); to this return cmb.resetAjaxRow( $parent.find( '.cmb-add-row-button' ), $this ); and added new function to match the change.

	cmb.resetAjaxRow = function( $addNewBtn, $removeBtn ) {
		// Click the "add new" button followed by the "remove this" button
		// in order to reset the repeat row to empty values.
		var addnew = $removeBtn.closest('.cmb-repeat-group-field').find( '.cmb-add-row-button' );

		addnew.trigger( 'click' );
		$removeBtn.trigger( 'click' );

	};

@tw2113
Copy link
Contributor

tw2113 commented Oct 10, 2017

@jtsternberg You have any ideas here?

@Mr2P
Copy link

Mr2P commented Oct 11, 2017

Thank @bomsn, I got the same issue. Your patch worked for me.
Anyway, the group index is correct but the field index is still not correct. In my case, when creating a new group (the fourth group), the ID of the first row of the repeatable field is: group_name[3][field_name][0][nested_field_name], the next row is group_name[3][field_name][1][nested_field_name] but the third row gets the ID is group_name[3][field_name][4][nested_field_name]

@bomsn
Copy link
Author

bomsn commented Oct 11, 2017

@Mr2P Make sure to use the patch on the trunk version. If it still doesn't work please share the code for rendering you custom field. Also does it work correctly on the 3 first groups, or you get the same issue on all groups?

@Mr2P
Copy link

Mr2P commented Oct 11, 2017

@bomsn Your patch solved my issue. I only needed the group index is correct for my hotfix.

@bomsn
Copy link
Author

bomsn commented Oct 11, 2017

@Mr2P Happy it helped.

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

No branches or pull requests

3 participants