Try add Post Type Archive in Link UI and Navigation Link via new Post Type Archive Search Handler #66821
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
getdave
added
[Type] Experimental
|
Size Change: +131 B (+0.01%) Total Size: 1.82 MB
ℹ️ View Unchanged
|
getdave
commented
Nov 7, 2024
| * @type string|int|WP_Error $total Numeric string containing the number of post-type archives found, or WP_Error object. | ||
| * } | ||
| */ | ||
| public function search_items( WP_REST_Request $request ) { |
There was a problem hiding this comment.
Need to check possible $request params for the search handler endpoints and ensure we're handling those correctly.
For example, offset and pagination...etc.
There was a problem hiding this comment.
Here are the Search endpoint docs.
We're handling pagination based on this example from post-formats.
getdave
commented
Nov 7, 2024
| * @param int $id Item ID. | ||
| * @return array[] Array of link arrays for the given item. | ||
| */ | ||
| public function prepare_item_links( $id ) { |
There was a problem hiding this comment.
Is it worth linking to the collection for each post archive returned? I have no use case for this so might be unnecessary at this stage,
getdave
commented
Nov 7, 2024
|
Obviously we'll need to write some unit tests for the new endpoint. |
|
Flaky tests detected in 5c63265. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/11723564937
|
getdave
requested review from
TimothyBJacobs,
spacedmonkey and
ajitbohra
as code owners
|
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 Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @gwwar, @mgratch, @wpchannel, @walton-alex, @nathan-schmidt-viget, @Bricobit. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. 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. |
45 tasks
|
Thank you for working on this ❤️ Really looking forward to having this in core 🚀 |
|
@cr0ybot As the contributor who provided the most popular work-around for the original Issue I wonder if you would consider reviewing this PR? You can use Playground to make this super simple. |
|
This doesn't really semantically feel like a search handler. We aren't querying the database at all. It seems like this should be using the types endpoint. It's probably worth looking at including non show_in_rest post types at this point. |
|
@getdave I've tested searching for custom post types with and without archives, with and without front. The first test worked well in that only post types with archives were findable, and the second worked well also until I changed the permalink structure, which broke the link in the navigation block. I realize that the current workaround with a custom link will also break if the permalink structure changes, but the old menu system would keep the link working. @TimothyBJacobs Does it matter if a subclass of |
👋 Thanks for taking a look at this. I did look at a few options before implementing a custom search handler. The
The only way I can see us being able to us the Types endpoint is by:
Also worth noting that we already have the Post Formats handler which is conceptually pretty similar to this Post Type Archive handler. I'm open to further advice on the REST API here @TimothyBJacobs. What's the best way for a client-side application to:
|
@cr0ybot That's a known issue but one I believe is separate to this PR. |
lib/compat/wordpress-6.8/class-gutenberg-rest-post-archive-search-handler.php
Outdated
Show resolved
Hide resolved
| // If any of the post types have `has_archive` set to true then add a post-type-archive variation. | ||
| $has_archive = array_filter( | ||
| $post_types, | ||
| function ( $post_type ) { |
There was a problem hiding this comment.
This closure can be declared as static (might improve performance).
Suggested change
| function ( $post_type ) { | |
| static function ( $post_type ) { |
lib/compat/wordpress-6.8/class-gutenberg-rest-post-archive-search-handler.php
Outdated
Show resolved
Hide resolved
| @@ -20,6 +20,9 @@ function gutenberg_register_block_editor_settings() { | |||
| add_action( 'rest_api_init', 'gutenberg_register_block_editor_settings' ); | |||
|
|
|||
|
|
|||
|
|
|||
There was a problem hiding this comment.
Nitpick: Are these empty lines needed for some reason?
|
Thank you @anton-vlasenko 🙇
I agree 💯 I'm going to wait on writing them until I get a confidence check from @TimothyBJacobs that my reasoning for using a new handler is justified. |
Co-authored-by: Anton Vlasenko <43744263+anton-vlasenko@users.noreply.github.com>
|
@TimothyBJacobs Do you think you might have a chance to review my response to the concerns you raised? Not a top priority, I'd like to get this Issue unblocked and resolved in 6.8 if possible. Very much appreciated. |
|
Here's the approach I'm suggesting taking here. I will raise an alternative PR which attempts to utilise the We can then review the options and get one or other of them merged so that folks can enjoy adding Post Type Archive links wherever they need to via the Link UI. |
getdave
changed the title
|
@TimothyBJacobs I've raised #67793 which attempts to utilise the existing Types endpoint. I'd be grateful if you would consider reviewing the two PRs and letting me know which seems more appropriate based on your experience with the REST API. |
|
Given the approval on #67793 (review) I'm going to close this one out in favour of #67793. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What?
Alternative to #67793
Enables searching for Post Type Archives using the Link UI and is specifically aimed at the Navigation block to bring parity with the Classic Menus system.
Closes #31452
Why?
Parity with Classic Menus where you could search for and add links to the Post Type Archives.
This also ensures that when visiting the Archives pages the
current-menu-itemclass is added to the menu item.How?
Testing Instructions
Code to register CPTs for testing
+ Add blockand then look for thePost Type Archive Linkblock variation. (hint if you can't see it then pressBrowse Alland search. It will come up automatically for subsequent rounds of testing).Testing Instructions for Keyboard
Screenshots or screencast