-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Query loop / Post template: Enable post format filter #64167
base: trunk
Are you sure you want to change the base?
Conversation
Size Change: +514 B (+0.03%) Total Size: 1.78 MB
ℹ️ View Unchanged
|
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
packages/block-library/src/query/edit/inspector-controls/post-format-controls.js
Outdated
Show resolved
Hide resolved
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
lib/compat/wordpress-6.7/class-gutenberg-rest-posts-controller-6-7.php
Outdated
Show resolved
Hide resolved
* @param WP_REST_Request $request Full details about the request. | ||
* @return array Updated query arguments. | ||
*/ | ||
private function prepare_tax_query( array $args, WP_REST_Request $request ) { |
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.
No changes were made here, the function is only included because it is private in the original class...
* @param array $query_params Collection schema. | ||
* @return array Updated schema. | ||
*/ | ||
private function prepare_taxonomy_limit_schema( array $query_params ) { |
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.
No changes were made here, the function is only included because it is private in the original class...
|
||
$args = $this->prepare_tax_query( $args, $request ); | ||
|
||
if ( isset( $request['format'] ) ) { |
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 is the only change to get_items.
Post template edit.js: Don't convert the post format to a string. This conversion prevented selecting multiple post formats in the query filter.
Change the type to a string that matches the format slugs. Each post can only have one format.
$schema['properties']['format'] = array( | ||
'description' => __( 'The format for the post.' ), | ||
'type' => 'string', | ||
'context' => array( 'view', 'edit' ), | ||
'enum' => array_values( get_post_format_slugs() ), | ||
); |
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 is the only change to get_item_schema.
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.
The format
property is only included in the schema when the post type supports it above (see case 'post-formats
). So why do we need unconditional addition here?
Flaky tests detected in 5154069. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/10501791656
|
I could not get this change to work. Maybe it should not even be added, so I am removing it.
I'm sorry if this has already been discussed, but maybe we should standardize post-format parameter names between the REST API and the editor. The tags' taxonomy name also has a How about using |
There are some issues that I will need to leave for another day (likely Monday)
|
Rename "postFormat" parameter to "format". Simplify the name of the control from PostFormatControls to FormatControls. Simplify the name of the constant that checks if the FormatControls should be used, from showPostFormatControl to showFormatControl.
I think the limitation is in the blocks.php file, gutenberg_filter_query_loop_block_query_vars_post_format, since if the |
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 for standardizing the naming convention, @carolinan!
Is there a reason to use format
instead of formats
when an array for post formats is used for querying a post collection? It's not a blocker, but I've seen plural forms used in similar cases.
We should also update get_collection_params
and list formats when the post type supports it. The post-status schema could be used as a blueprint for the new value.
! empty( $block->context['query']['format'] ) ) { | ||
// Add the required "post-format-" prefix to each format. | ||
$formats = array_map( | ||
function ( $format ) { |
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.
function ( $format ) { | |
static function ( $format ) { |
I think even closures outside classes should be static
.
|
||
$args = $this->prepare_tax_query( $args, $request ); | ||
|
||
if ( isset( $request['format'] ) && ! empty( $request['format'] ) ) { |
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.
Just ! empty
check should be enough here.
if ( ! empty( $formats ) ) { | ||
// Add the post-format- prefix. | ||
$terms = array_map( | ||
function ( $format ) { |
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.
function ( $format ) { | |
static function ( $format ) { |
Micro-optimization: Closures/anon-functions in classes should use static
if they're not accessing $this
.
$schema['properties']['format'] = array( | ||
'description' => __( 'The format for the post.' ), | ||
'type' => 'string', | ||
'context' => array( 'view', 'edit' ), | ||
'enum' => array_values( get_post_format_slugs() ), | ||
); |
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.
The format
property is only included in the schema when the post type supports it above (see case 'post-formats
). So why do we need unconditional addition here?
<SelectControl | ||
__nextHasNoMarginBottom | ||
__next40pxDefaultSize | ||
multiple | ||
label={ __( 'Formats' ) } | ||
value={ format } | ||
options={ formatOptions } | ||
onChange={ ( value ) => { | ||
onChange( { format: value } ); | ||
} } | ||
/> |
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.
Is this a temporary placeholder component? Are we planning to switch to the FormTokenField
later?
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.
I don't think this should use the FormTokenField because the options are always the ones supported by the theme.
The number of options is low, and I would like to know if the format is supported or not, without typing the name of the format into an input?
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.
That should be possible by displaying suggestions (supported formats) on focus. See https://wordpress.github.io/gutenberg/?path=/story/components-formtokenfield--dropdown-selector.
What?
Enables filtering the query loop result by post format.
Partial for #53049
Props @justintadlock @SergeyBiryukov
Why?
This feature is often requested by users, see the linked issue.
How?
Description updated August 22 2024.
This PR attempts to enable filtering the post query by post format, but handles the post format separately from
other taxonomies, as requested by reviewers.
Known limitations and issues include:
REST API changes:
Help is wanted with this part!
A new class
Gutenberg_REST_Posts_Controller_6_7
that extendsWP_REST_Posts_Controller
is added to the 6.7 compatibility folder.The intention of this new temporary class is to add 'format' to the Posts controller, so that it can be used to filter the query result per post format.
Updates are made to these class methods: get_item, get_item_schema.
Earlier I also tried updating get_collection_params, but I was not able to make it work, so it is removed for now.
Query loop block:
postFormat
, to thequery
block attribute in the query loop block.PostFormatControls
that uses a list of post formats in aSelectControl
. The SelectControl usesmultiple
to indicate to the user that they can select to show multiple formats in the query loop block.Help wanted: Using SelectControl may not be the best choice, I am very open to implementing other design ideas.
Post Template block:
postFormat
parameter is passed from the query loop block to the post template block as part of thequery
.query
using the newformat
key (as opposed to using the post format taxonomy).Filtering query_loop_block_query_vars
Filter the query vars (tax_query) for the Query Loop block to support post formats on the front of the site.
Testing Instructions
Enable post formats in a theme by adding the theme support. Example:
Add some test content with different post formats.
Test that there are no regressions with the query loop features, and that all filters work.
The correct posts should show when the post format filter is used.
Screenshots or screencast