For the title we have the block_core_breadcrumbs_get_post_title helper function. We shouldn't remove it and see how it fits with the new helper functions.

I’m keeping the current line:

$breadcrumb_items[] = block_core_breadcrumbs_create_current_item(
    block_core_breadcrumbs_get_post_title( $post ),
    true
);

This handles the current post title as before. The new logic I’m adding for paginated posts (Page X) would come after this line so that the breadcrumb shows both the post title and the page number.

Does that approach look good to you?

It would be easier if you make the changes make reviewing/testing easier.

In general the flow would be to check if is paged and either call block_core_breadcrumbs_create_item or inline the logic there to work for the page query var, as I'm not sure if get_pagenum_link will handle it properly (I think it handles paged query var).

So if is paged -> block_core_breadcrumbs_create_link -> add the Page X item.
If not paged -> you call the existing line in trunk with block_core_breadcrumbs_create_current_item.

Does this help?

Yes, I’ve implemented it as suggested: the current post title is always added as a non-linked item, and if the post is paginated, a “Page X” item is appended. Changes have been pushed for review/testing.

This function has been removed and we have new helper functions since this PR. We should see what we can use in this case, but given the query param is different for pages (page), it might even not need new helper functions and we could possibly inline the logic (last else block in the main render_block_core_breadcrumbs function).

Hi @ntsekouras

I’ve implemented the inline logic for paginated post content directly in the else block after the post title — instead of using the removed block_core_breadcrumbs_maybe_add_paged() function.

It checks get_query_var( 'page' ) and adds a “Page X” breadcrumb when greater than 1:

$page_number = max( 1, get_query_var( 'page' ) );
if ( $page_number > 1 ) {
	/* translators: %d: current page number. */
	$breadcrumb_items[] = block_core_breadcrumbs_create_current_item(
		sprintf( esc_html__( 'Page %d', 'gutenberg' ), number_format_i18n( $page_number ) ),
		true
	);
}

Could you please confirm if this approach looks good before I commit and push the changes?

This is close, but we should also handle the case where is paged. When it is, we should create a link.

I think the below code would be enough:

$is_paged = (int) get_query_var( 'page' ) > 1;
$title    = block_core_breadcrumbs_get_post_title( $post );

if ( $is_paged ) {
	$breadcrumb_items[] = block_core_breadcrumbs_create_link( get_permalink( $post ), $title, true );
	$breadcrumb_items[] = block_core_breadcrumbs_create_page_number_item( 'page' );
} else {
	$breadcrumb_items[] = block_core_breadcrumbs_create_current_item( $title, true );
}

For that to work though, we'll need another small change in block_core_breadcrumbs_create_page_number_item to pass the query_var like this:

function block_core_breadcrumbs_create_page_number_item( $query_var = 'paged' ) {
	$paged = (int) get_query_var( $query_var );
	return block_core_breadcrumbs_create_current_item(
		/* translators: %s: page number */
		sprintf( __( 'Page %s' ), number_format_i18n( $paged ) )
	);
}