Breadcrumbs: Add archive link if enabled in posts
#72832
Conversation
ntsekouras
requested review from
jorgefilipecosta,
justintadlock,
mcsf and
t-hamano
ntsekouras
added
[Type] Enhancement
| return sprintf( | ||
| '<a href="%s">%s</a>', | ||
| esc_url( $archive_link ), | ||
| esc_html( $post_type_object->labels->name ) |
There was a problem hiding this comment.
There's also the archives label, which I agree isn't necessarily that broadly used. I wonder if we considered if we want to go with that, potentially.
There was a problem hiding this comment.
That's a good point! Actually it seems like something we should do, but I'd also like @justintadlock's thoughts since in his plugin it seems it doesn't use that (below is core block with archives label.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
| * @return array Array of breadcrumb HTML items. | ||
| */ | ||
| function block_core_breadcrumbs_get_hierarchical_post_type_breadcrumbs( $post_id ) { | ||
| function block_core_breadcrumbs_get_hierarchical_post_type_breadcrumbs( $post_id, $post_type ) { |
There was a problem hiding this comment.
It feels non-ergonomic that we also have to pass the post type explicitly. I'd rather grab it from the $post_id:
$post_type = get_post_type( $post_id );In theory, it should be free to do that since the post object is already in the WP_Query.
There was a problem hiding this comment.
From one hand you're probably right because this block doesn't make much sense in blocks that update the block context manually (like Query Loop). On the other hand though why call another function (get_post_type) when we already have the info?
There was a problem hiding this comment.
I would agree if we had it inline, or if we didn't have it loaded from the database. But it should already be loaded, and we don't have it inline in that function either - we pass it between functions, and to do so we need to add an extra argument. This makes it less ergonomic, which is something to cosnider when designing functions and APIs.
That being said, I don't feel strongly about it, I don't think it's a major deal.
|
Flaky tests detected in 5cac11c. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/18968754081
|
What?
Part of: #72498
This PR adds the
archivelink if enabled in posts, as it seems like a good default to have for the breadcrumbs.Testing Instructions
has_archiveenabled and disabled.Screenshots or screencast
Below I'm sharing some screenshots in a site that I've added @justintadlock's plugin and the core block in the main header.
Above is x3p0-breadcrumbs/ and below is the core block.
Product (Woo) with terms
CPT without terms
Hierarchical CPT with ancestors