Duplicate block field

[lukecarbis]

Wow! This was a lot more complicated than I thought it would be.

This isn't yet working for repeater fields (but it is for sub-fields).

This PR also includes a whole heap of JS coding standard fixes.

Closes #315.

[RobStino]

This is awesome!
Tested and the UX is great!

[kienstra]

Looks Good

Hi @lukecarbis,
This looks good, and duplication worked really well.

Initial Field

Duplicate

All of the fields that I tested had all of their values duplicated. Including Text, Post, and Checkbox.

[kienstra]

Wow, this is great. It looks like this now passes eslint!

What do you think of adding it to the eslint configuration?

diff --git a/package.json b/package.json
index c5cd510..6c7cf1a 100644
--- a/package.json
+++ b/package.json
@@ -11,7 +11,7 @@
     "dev": "cross-env BABEL_ENV=default webpack --watch",
     "build": "cross-env BABEL_ENV=default NODE_ENV=production webpack",
     "lint": "npm-run-all --parallel lint:*",
-    "lint:js": "eslint js/blocks",
+    "lint:js": "eslint js",
     "lint:js:fix": "eslint js/blocks --fix",
     "lint:php": "vendor/bin/phpcs",
     "lint:php:fix": "vendor/bin/phpcbf"

Also, creating an .eslintignore with the following will prevent it from running on the compiled files:

js/editor.blocks.js
js/scripts.js

[kienstra]

Side Note

We might consider refactoring this Edit Block UI in React at some point (though not necessarily re-designing it much):

js/admin.block-post.js had been long before, and is now 458 lines.

The DOM manipulation in that file gets really complex, and it's going to be hard to maintain. And that file's probably not going to get smaller 😄

[lukecarbis]

@kienstra @RobStino Requesting a re-review. Some big changes made in the last couple of commits.

In particular, please review duplication repeater fields!

[RobStino]

Hey all,
Tested and this works really well!
My testing included:

• Duplicating a field
• Duplicating a whole repeater field with subfields
• Duplicating a sub-field
• Duplicating a duplicated sub-field within a duplicated repeater field :P
  Basically I tried all the variations of everything and noted that:
• Duplications always worked
• The generated field name was always unique

Great work @lukecarbis This is a really useful feature!

[kienstra]

Great work!

Hi @lukecarbis,
Wow, this works well, even duplicating entire repeaters.

Duplicating single fields and repeater rows also work well.

It might be good to refactor the 'Edit Block' screen with React soon.

admin.block-post.js is over 500 lines, and will only get harder to refactor. Though it's worked really well, very few bugs.

[lukecarbis]

You guys. Thanks for testing, but you missed a critical bug!

Repeater child fields aren't saving. Will take a look at it asap.

[lukecarbis]

The bug is occurring because we're assigning the sub-fields their parent's UID instead of their parent's slug.

Actually, this made me realise that saving sub-fields with the parents name is a bad idea! It surfaced a new bug: If you create a repeater, add sub fields, then go back and change the repeater's name, then the sub fields will be deleted on save.

I'm going to refactor the sub fields to use the parent UID instead of the parent slug / name.

[lukecarbis]

@kienstra and @RobStino – please re-review? Especially testing:

• Do sub-rows from pre-existing blocks still save without making changes?
• Do sub-rows from pre-existing blocks still save with making changes?
• Do new sub-rows still save?
• Can you break the auto-incrementing field names by adding rows, sub rows, and duplicating things?

[RobStino]

Just testing this now, and looks like I hit a limit for the incremental numbering for duplicating fields.

Is this intentional?

[lukecarbis]

@RobStino Great pickup. Fixed in a650df0.

[RobStino]

Yep, that fixes it.

[kienstra]

Do sub-rows from pre-existing blocks still save without making changes?	yes
Do sub-rows from pre-existing blocks still save with making changes?	yes
Do new sub-rows still save?	yes

I'm working on this:

Can you break the auto-incrementing field names by adding rows, sub rows, and duplicating things?

[kienstra]

Can you break the auto-incrementing field names by adding rows, sub rows, and duplicating things?

No, I haven't been able to break this 😄

[kienstra]

Pending any testing you'd like, I think the code is ready to merge.