Skip to content
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.

Sign up for GitHub

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

Add Ignore option to sticky options for Query Loop block #66222

Closed
wants to merge 7 commits into from
Closed

Add Ignore option to sticky options for Query Loop block #66222

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7eaeb8c
Add `Ignore` option to sticky options for Query Loop block
coreyworrell Oct 17, 2024
03b5e04
Add compat for query loop vars
coreyworrell Oct 24, 2024
1ce1e42
Remove whitespace at end of line
coreyworrell Oct 29, 2024
60cb502
Create backport-changelog file
coreyworrell Oct 31, 2024
2d9c182
Merge branch 'WordPress:trunk' into patch-1
coreyworrell Nov 4, 2024
ccc2ca9
Add compat for 6.8
coreyworrell Nov 4, 2024
a1a4394
Remove patch from 6.7
coreyworrell Nov 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions lib/compat/wordpress-6.7/blocks.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,3 +103,31 @@
return $query;
}
add_filter( 'query_loop_block_query_vars', 'gutenberg_add_format_query_vars_to_query_loop_block', 10, 2 );

/**
* Adds `ignore` option for sticky posts to the Query block.
*

Check failure on line 109 in lib/compat/wordpress-6.7/blocks.php

View workflow job for this annotation

GitHub Actions / PHP coding standards 

Whitespace found at end of line
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The linting has to be addressed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed now

* @see 'query_loop_block_query_vars'
*

Check failure on line 111 in lib/compat/wordpress-6.7/blocks.php

View workflow job for this annotation

GitHub Actions / PHP coding standards 

Whitespace found at end of line
* @param array $query The query vars.
* @param WP_Block $block Block instance.
* @return array The filtered query vars.
*/
function gutenberg_add_ignore_sticky_posts_to_query_loop_block( $query, $block ) {
if ( ! empty( $block->context['query']['sticky'] ) && 'ignore' === $block->context['query']['sticky'] ) {
// Core function excludes all sticky posts if the `sticky` value is anything
// other than `only`. We must reset that here, but it could potentially also
// re-allow a sticky post that had been excluded in some other way. This

Check failure on line 120 in lib/compat/wordpress-6.7/blocks.php

View workflow job for this annotation

GitHub Actions / PHP coding standards 

Whitespace found at end of line
// works okay for testing, but the real fix will need to be in the
// core function.
$sticky = get_option( 'sticky_posts' );

$query['post__not_in'] = array_diff( $query['post__not_in'], ! empty( $sticky ) ? $sticky : array() );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we exlude sticky posts? Ignore set below should include them and just list them in their natural order, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line 125 doesn't exclude them, it's actually re-including them after core already excluded them. That's what the comment above line 123 is about.


// Ignore sticky posts.
$query['ignore_sticky_posts'] = 1;
}

return $query;
}
add_filter( 'query_loop_block_query_vars', 'gutenberg_add_ignore_sticky_posts_to_query_loop_block', 10, 2 );
1 change: 1 addition & 0 deletions packages/block-library/src/query/edit/inspector-controls/sticky-control.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const stickyOptions = [
{ label: __( 'Include' ), value: '' },
{ label: __( 'Exclude' ), value: 'exclude' },
{ label: __( 'Only' ), value: 'only' },
{ label: __( 'Ignore' ), value: 'ignore' },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this affect the editor? Shouldn't there be some change to the querying done clientside in the editor too?

Copy link
Contributor

@carolinan carolinan Oct 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great question.

In the editor, the sticky posts are already bugged on trunk. For example sticky posts do not show as the first posts when the "include" option is set, which is the default. The result in the editor and front already does not match.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should at least try to see if we can match the preview in editor. As @carolinan says it's already buggy and if I remember correctly is about what REST API supports how it maps with WP_Query. That's probably the reason (the mapping) for not having this as a separate option for sticky control.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ignore label is not self explanatory in this context and is quite similar to the exclude, where it's ignore post stickiness.

Maybe a different control can be used with help texts? --cc @jameskoster

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps 'Ignore stickiness' is okay for now?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ignore stickiness 😂

];

export default function StickyControl( { value, onChange } ) {
Expand Down
Loading