This repository has been archived by the owner on Nov 6, 2022. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Repeater row count and row index function. #429
Repeater row count and row index function. #429
Changes from 10 commits
d43aad2
2326b9d
5b50bef
432554e
6300f33
574db41
6e972b1
f4080fe
a1bb136
8d0b4f6
2b61b64
6c39603
3e81bf3
39fade9
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this conditional could be moved above to line
309
, instead of creating a newif
block:Right now, it's possible (though unlikely) for
$sub_field_settings
to be unset at line 318.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in 39fade9.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good thing to bring up. In PHP, this would be an
array()
, and in JS it'd be an object. I don't see many examples of array or object attributes in Core blocks.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah – makes much more sense for
$type
to represent the Javascript type, rather than the PHP type. Addressed in 2b61b64.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about something like:
It looks like that removes empty array values, like
'' => ''
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be nice, except that
array_filter
filters empty values (not empty keys) – so if there were a repeater with an empty value, that item in the array would be removed.We could supply a custom callback to array_filter which targets empty keys, or alternatively use something like this:
but ultimately, I think just unsetting the two is a better, simpler, solution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, it removes empty values. That's not good.