Repeater row count and row index function.

[lukecarbis]

This adds some helper functions for retrieving the total count of repeater rows, and the index of an individual repeater row.

The helper functions are:

block_row_count( $name )
This takes a repeater field name as the argument, and returns the row count for that repeater. (It can also return false, if you pass it an invalid repeater name.)

block_row_index()
This should be used inside a repeater loop (after the block_row() call). This returns the index of the current row.

Here's a code example:

if ( block_rows( 'repeater' ) && block_row_count( 'repeater' ) > 3 ):

    while ( block_rows( 'repeater' ) ) :
        block_row( 'repeater' );

        // Note: The index is zero-based, so we should add 1 to get a non-programmer count.
        echo '<li>This is row number ' . block_row_index() + 1 . '</li>';
    endwhile;

    echo '</ul>';
endif;

Closes #403.

[kienstra]

This function looks good overall.

Is this conditional necessary?

It seems that if it's not set, the block below will return false:

if ( ! isset( $block_lab_attributes[ $name ]['rows'] ) ) {
	return false;
}

[lukecarbis]

That's a really good point.

The thinking here is:

1. Check if a row even exists
2. Check if it's a repeater (has rows)

Given this, do you still think it's worth only doing the single (second) condition? I'm more than happy to make that change. Just wanted to explain my approach.

[kienstra]

Sure, it's kind of a nitpick 😄

My guess is that there's not much performance difference between the existing approach, and an approach with only one isset() check.

[lukecarbis]

Addressed in 6300f33.

[kienstra]

Could the logic in this class be in the Repeater class, in a validate() method?

[lukecarbis]

Done in a1bb136.

[lukecarbis]

@kienstra Thanks for your review. Ready for you to take another look.

Wanted to make note of 8d0b4f6. I was trying to remove repeater specific functionality from the Field class, and move it into the Repeater control. If you don't think it makes much sense, though, I'd be happy to revert this commit.

[kienstra]

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.

[lukecarbis]

Ah – makes much more sense for $type to represent the Javascript type, rather than the PHP type. Addressed in 2b61b64.

[kienstra]

Hi @lukecarbis,
Nice work here. I have to go to bed, and I'll take a look at it again tomorrow.

[kienstra]

Looks Good
Some Minor Points

Hi @lukecarbis,
This looks good. The new functions should be really useful.

There are just a few minor points. I'll be in and out today, so feel free to merge when you're happy.

[kienstra]

Maybe this conditional could be moved above to line 309, instead of creating a new if block:

if ( isset( $field->settings['sub_fields'] ) && isset( $attributes[ $field->name ]['rows'] ) ) {

Right now, it's possible (though unlikely) for $sub_field_settings to be unset at line 318.

[lukecarbis]

Good catch.

[lukecarbis]

Addressed in 39fade9.

[kienstra]

Thanks 😄

[kienstra]

How about something like:

array_filter( $value['rows'] );

It looks like that removes empty array values, like '' => ''

[lukecarbis]

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:

array_diff_key( $value['rows'][ $key ], array_flip( array( '', 0 ) ) );

but ultimately, I think just unsetting the two is a better, simpler, solution.

[kienstra]

Ah, it removes empty values. That's not good.

[kienstra]

Like we talked about, this should be good to merge whenever you'd like 😄