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.

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

Remove sticky posts setting when we inherit the query #40656

Merged
merged 10 commits into from
Jul 6, 2022

Conversation

draganescu
Copy link
Contributor

@draganescu draganescu commented Apr 27, 2022

What?

Fixes #38979

Why?

Because the setting not just doesn't work.

How?

We hide the setting when inherit is true.

Testing Instructions

  1. Switch to a block theme (e.g. TT2)
  2. Create a sticky post
  3. Look at a template with a query loop in it (e.g. home.html)
  4. Confirm that if the block has inherit query you cannot set sticky posts.

Notes

This PR has explored allowing sticky post behaviour on inherit but the result was consistently unreliable, one or another query var would be overwritten either by WP_Query or by the query block.

Therefore the PR hides the setting and fixes the server rendering of the block to truly inherit, without interfering with the current WP_Query in any way.

Co-authored-by: Jonny Harris [email protected]
Co-authored-by: Huub [email protected]

@draganescu draganescu requested a review from ajitbohra as a code owner April 27, 2022 14:21
@draganescu draganescu added [Type] Bug An existing feature does not function as intended Needs Technical Feedback Needs testing from a developer perspective. [Block] Query Loop Affects the Query Loop Block labels Apr 27, 2022
Copy link
Contributor

@scruffian scruffian left a comment

Choose a reason for hiding this comment

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

This fixes it for me in the front end. We still need to fix the editor as well, but that can happen in a followup.

@carolinan
Copy link
Contributor

carolinan commented Apr 28, 2022

I am not able to reproduce the bug, I don't want to be rude but it would help me if the instructions were more detailed.
The issues says:

  • Create a sticky post
  • Look at a template with a query loop in it (e.g. home.html)

What settings exactly does the query loop need to have for me to reproduce the issue?

setting the inherit global query setting in the query block

I am assuming this means the option labeled with "Inherit query from template"
But I honestly do not see the bug with or without this setting, with or without the PR, using 6.0 beta or 5.9.3.

@scruffian
Copy link
Contributor

For me the reproduction steps are these:

  1. Switch to TT2
  2. Create an old sticky post
  3. Open the home page
  4. The sticky post is not at the top of the list

Copy link
Contributor

@ntsekouras ntsekouras left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @draganescu ! Sticky posts have many nuances in the Query Loop context.

For example your comment here:

Another problem here is that when Inherit query from template is set the sticky preferences should either be hidden or they should be merged in the global query. Which one is more expected for the user?

I was thinking a while ago to hide them, but it seems preferable if we find a proper solution for this.

Another 'sticky' problem we have is that they're shown in every page results in a custom Query Loop block that has include sticky.

Screen.Recording.2022-04-29.at.12.19.58.PM.mov

packages/block-library/src/post-template/index.php Outdated Show resolved Hide resolved
Copy link
Contributor

@ntsekouras ntsekouras left a comment

Choose a reason for hiding this comment

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

I forgot to make the review more prominent not to merge right now because of the approval and the breakage.

@draganescu
Copy link
Contributor Author

@ntsekouras in 066437c I added a change that unsets all query block defaults which are not editable by the user when inherit is true. That to me makes the most sense:

  • when we use the global WP_Query use its defaults and settings
  • allow stick post behavior to be specified even when using global WP_Query because sometimes we need sticky post exclusive query blocks: for example if I want to show sticky posts at the top in an archive page global WP_Query won't do it by default, but I can use a secondary query block.

@ntsekouras
Copy link
Contributor

Haven't yet looked into the sticky use case here, but sharing a comment of mine to a relevant PR about inherit.

TLDR:

I think the big issue we're having with Query Loop and inherit is that while some controls are hidden in the UI, their values are preserved and I observed that while testing with at least orderby - this value is not set in the global query. We need to have no arbitrary results from block query attributes and what we expect when we set inherit to true.

Maybe we should inherit all options from global query but maybe there are some other concerns.. @draganescu had mentioned some nuances in the sticky use cases that I'm also planning to look into more..

@ntsekouras
Copy link
Contributor

I'm not sure yet about this and how to approach it. Right now I'm thinking that it should be better to just hide the sticky control when we inherit. It seems confusing to have these two(and probably other controls) that override the inherit - or we should communicate that better by altering the descriptions of the controls, etc..

For the specific use case now (sticky) I feel that we shouldn't honor every option and we should hide the control too. For example it doesn't make sense to have sticky -> only and inherit. This way it will just show the sticky posts...

A solution could be specific to sticky behavior
, which is: show them only on front page if we inherit the global query. So inside this block, something like this right after the merge of args might do the trick?

if ( is_front_page() ) {
	unset( $query_args['s'] );
}

What do you think?

@draganescu
Copy link
Contributor Author

I think sticky posts are important for certain workflows. I would proceed from the feedback gathered here, plus a chat with @jameskoster, plus the fact that we don't really have straight answers to:

  • revert changes to the query block's rendering
  • simply hide any UI that contains overriding options for WP_Query if inherit is ON (do so programatically in the edit component not only to the "sticky" override).
  • come back to it with better approaches, maybe even make a "sticky" posts block as a variation of query or something.

@github-actions
Copy link

github-actions bot commented Jun 27, 2022

Size Change: +2.46 kB (0%)

Total Size: 1.25 MB

Filename Size Change
build/block-editor/index.min.js 152 kB +12 B (0%)
build/block-library/blocks/comment-template/style-rtl.css 187 B +60 B (+47%) 🚨
build/block-library/blocks/comment-template/style.css 185 B +58 B (+46%) 🚨
build/block-library/blocks/search/theme-rtl.css 114 B +50 B (+78%) 🆘
build/block-library/blocks/search/theme.css 114 B +50 B (+78%) 🆘
build/block-library/index.min.js 183 kB -133 B (0%)
build/block-library/style-rtl.css 11.5 kB +52 B (0%)
build/block-library/style.css 11.5 kB +52 B (0%)
build/block-library/theme-rtl.css 695 B +18 B (+3%)
build/block-library/theme.css 700 B +18 B (+3%)
build/blocks/index.min.js 47.1 kB +39 B (0%)
build/components/index.min.js 230 kB +193 B (0%)
build/components/style-rtl.css 14 kB -13 B (0%)
build/components/style.css 14 kB -13 B (0%)
build/core-data/index.min.js 14.7 kB -3 B (0%)
build/edit-navigation/index.min.js 16 kB -9 B (0%)
build/edit-post/index.min.js 30.4 kB -20 B (0%)
build/edit-post/style-rtl.css 7.03 kB -50 B (-1%)
build/edit-post/style.css 7.03 kB -48 B (-1%)
build/edit-site/index.min.js 52 kB +1.14 kB (+2%)
build/edit-site/style-rtl.css 8.28 kB +139 B (+2%)
build/edit-site/style.css 8.26 kB +139 B (+2%)
build/edit-widgets/index.min.js 16.5 kB +14 B (0%)
build/editor/index.min.js 39.4 kB +661 B (+2%)
build/editor/style-rtl.css 3.65 kB +24 B (+1%)
build/editor/style.css 3.65 kB +25 B (+1%)
build/keyboard-shortcuts/index.min.js 1.78 kB -11 B (-1%)
build/rich-text/index.min.js 11.1 kB +13 B (0%)
build/shortcode/index.min.js 1.53 kB +11 B (+1%)
ℹ️ View Unchanged
Filename Size
build/a11y/index.min.js 982 B
build/annotations/index.min.js 2.76 kB
build/api-fetch/index.min.js 2.26 kB
build/autop/index.min.js 2.14 kB
build/blob/index.min.js 475 B
build/block-directory/index.min.js 6.58 kB
build/block-directory/style-rtl.css 990 B
build/block-directory/style.css 991 B
build/block-editor/default-editor-styles-rtl.css 378 B
build/block-editor/default-editor-styles.css 378 B
build/block-editor/style-rtl.css 14.5 kB
build/block-editor/style.css 14.5 kB
build/block-library/blocks/archives/editor-rtl.css 61 B
build/block-library/blocks/archives/editor.css 60 B
build/block-library/blocks/archives/style-rtl.css 65 B
build/block-library/blocks/archives/style.css 65 B
build/block-library/blocks/audio/editor-rtl.css 150 B
build/block-library/blocks/audio/editor.css 150 B
build/block-library/blocks/audio/style-rtl.css 103 B
build/block-library/blocks/audio/style.css 103 B
build/block-library/blocks/audio/theme-rtl.css 110 B
build/block-library/blocks/audio/theme.css 110 B
build/block-library/blocks/avatar/editor-rtl.css 116 B
build/block-library/blocks/avatar/editor.css 116 B
build/block-library/blocks/avatar/style-rtl.css 59 B
build/block-library/blocks/avatar/style.css 59 B
build/block-library/blocks/block/editor-rtl.css 161 B
build/block-library/blocks/block/editor.css 161 B
build/block-library/blocks/button/editor-rtl.css 441 B
build/block-library/blocks/button/editor.css 441 B
build/block-library/blocks/button/style-rtl.css 543 B
build/block-library/blocks/button/style.css 543 B
build/block-library/blocks/buttons/editor-rtl.css 292 B
build/block-library/blocks/buttons/editor.css 292 B
build/block-library/blocks/buttons/style-rtl.css 275 B
build/block-library/blocks/buttons/style.css 275 B
build/block-library/blocks/calendar/style-rtl.css 207 B
build/block-library/blocks/calendar/style.css 207 B
build/block-library/blocks/categories/editor-rtl.css 84 B
build/block-library/blocks/categories/editor.css 83 B
build/block-library/blocks/categories/style-rtl.css 79 B
build/block-library/blocks/categories/style.css 79 B
build/block-library/blocks/code/style-rtl.css 103 B
build/block-library/blocks/code/style.css 103 B
build/block-library/blocks/code/theme-rtl.css 124 B
build/block-library/blocks/code/theme.css 124 B
build/block-library/blocks/columns/editor-rtl.css 108 B
build/block-library/blocks/columns/editor.css 108 B
build/block-library/blocks/columns/style-rtl.css 406 B
build/block-library/blocks/columns/style.css 406 B
build/block-library/blocks/comment-author-avatar/editor-rtl.css 125 B
build/block-library/blocks/comment-author-avatar/editor.css 125 B
build/block-library/blocks/comment-content/style-rtl.css 92 B
build/block-library/blocks/comment-content/style.css 92 B
build/block-library/blocks/comments-pagination-numbers/editor-rtl.css 123 B
build/block-library/blocks/comments-pagination-numbers/editor.css 121 B
build/block-library/blocks/comments-pagination/editor-rtl.css 222 B
build/block-library/blocks/comments-pagination/editor.css 209 B
build/block-library/blocks/comments-pagination/style-rtl.css 235 B
build/block-library/blocks/comments-pagination/style.css 231 B
build/block-library/blocks/comments-title/editor-rtl.css 75 B
build/block-library/blocks/comments-title/editor.css 75 B
build/block-library/blocks/comments/editor-rtl.css 95 B
build/block-library/blocks/comments/editor.css 95 B
build/block-library/blocks/cover/editor-rtl.css 615 B
build/block-library/blocks/cover/editor.css 616 B
build/block-library/blocks/cover/style-rtl.css 1.55 kB
build/block-library/blocks/cover/style.css 1.55 kB
build/block-library/blocks/embed/editor-rtl.css 293 B
build/block-library/blocks/embed/editor.css 293 B
build/block-library/blocks/embed/style-rtl.css 410 B
build/block-library/blocks/embed/style.css 410 B
build/block-library/blocks/embed/theme-rtl.css 110 B
build/block-library/blocks/embed/theme.css 110 B
build/block-library/blocks/file/editor-rtl.css 300 B
build/block-library/blocks/file/editor.css 300 B
build/block-library/blocks/file/style-rtl.css 253 B
build/block-library/blocks/file/style.css 254 B
build/block-library/blocks/file/view.min.js 346 B
build/block-library/blocks/freeform/editor-rtl.css 2.44 kB
build/block-library/blocks/freeform/editor.css 2.44 kB
build/block-library/blocks/gallery/editor-rtl.css 948 B
build/block-library/blocks/gallery/editor.css 950 B
build/block-library/blocks/gallery/style-rtl.css 1.5 kB
build/block-library/blocks/gallery/style.css 1.49 kB
build/block-library/blocks/gallery/theme-rtl.css 108 B
build/block-library/blocks/gallery/theme.css 108 B
build/block-library/blocks/group/editor-rtl.css 333 B
build/block-library/blocks/group/editor.css 333 B
build/block-library/blocks/group/style-rtl.css 57 B
build/block-library/blocks/group/style.css 57 B
build/block-library/blocks/group/theme-rtl.css 78 B
build/block-library/blocks/group/theme.css 78 B
build/block-library/blocks/heading/style-rtl.css 76 B
build/block-library/blocks/heading/style.css 76 B
build/block-library/blocks/html/editor-rtl.css 327 B
build/block-library/blocks/html/editor.css 329 B
build/block-library/blocks/image/editor-rtl.css 738 B
build/block-library/blocks/image/editor.css 737 B
build/block-library/blocks/image/style-rtl.css 524 B
build/block-library/blocks/image/style.css 530 B
build/block-library/blocks/image/theme-rtl.css 110 B
build/block-library/blocks/image/theme.css 110 B
build/block-library/blocks/latest-comments/style-rtl.css 284 B
build/block-library/blocks/latest-comments/style.css 284 B
build/block-library/blocks/latest-posts/editor-rtl.css 199 B
build/block-library/blocks/latest-posts/editor.css 198 B
build/block-library/blocks/latest-posts/style-rtl.css 463 B
build/block-library/blocks/latest-posts/style.css 462 B
build/block-library/blocks/list/style-rtl.css 88 B
build/block-library/blocks/list/style.css 88 B
build/block-library/blocks/media-text/editor-rtl.css 266 B
build/block-library/blocks/media-text/editor.css 263 B
build/block-library/blocks/media-text/style-rtl.css 493 B
build/block-library/blocks/media-text/style.css 490 B
build/block-library/blocks/more/editor-rtl.css 431 B
build/block-library/blocks/more/editor.css 431 B
build/block-library/blocks/navigation-link/editor-rtl.css 705 B
build/block-library/blocks/navigation-link/editor.css 703 B
build/block-library/blocks/navigation-link/style-rtl.css 115 B
build/block-library/blocks/navigation-link/style.css 115 B
build/block-library/blocks/navigation-submenu/editor-rtl.css 296 B
build/block-library/blocks/navigation-submenu/editor.css 295 B
build/block-library/blocks/navigation-submenu/view.min.js 402 B
build/block-library/blocks/navigation/editor-rtl.css 2.03 kB
build/block-library/blocks/navigation/editor.css 2.04 kB
build/block-library/blocks/navigation/style-rtl.css 1.96 kB
build/block-library/blocks/navigation/style.css 1.95 kB
build/block-library/blocks/navigation/view-modal.min.js 2.78 kB
build/block-library/blocks/navigation/view.min.js 423 B
build/block-library/blocks/nextpage/editor-rtl.css 395 B
build/block-library/blocks/nextpage/editor.css 395 B
build/block-library/blocks/page-list/editor-rtl.css 363 B
build/block-library/blocks/page-list/editor.css 363 B
build/block-library/blocks/page-list/style-rtl.css 175 B
build/block-library/blocks/page-list/style.css 175 B
build/block-library/blocks/paragraph/editor-rtl.css 157 B
build/block-library/blocks/paragraph/editor.css 157 B
build/block-library/blocks/paragraph/style-rtl.css 260 B
build/block-library/blocks/paragraph/style.css 260 B
build/block-library/blocks/post-author/style-rtl.css 175 B
build/block-library/blocks/post-author/style.css 176 B
build/block-library/blocks/post-comments-form/editor-rtl.css 96 B
build/block-library/blocks/post-comments-form/editor.css 96 B
build/block-library/blocks/post-comments-form/style-rtl.css 495 B
build/block-library/blocks/post-comments-form/style.css 495 B
build/block-library/blocks/post-comments/editor-rtl.css 77 B
build/block-library/blocks/post-comments/editor.css 77 B
build/block-library/blocks/post-comments/style-rtl.css 632 B
build/block-library/blocks/post-comments/style.css 630 B
build/block-library/blocks/post-excerpt/editor-rtl.css 73 B
build/block-library/blocks/post-excerpt/editor.css 73 B
build/block-library/blocks/post-excerpt/style-rtl.css 69 B
build/block-library/blocks/post-excerpt/style.css 69 B
build/block-library/blocks/post-featured-image/editor-rtl.css 605 B
build/block-library/blocks/post-featured-image/editor.css 605 B
build/block-library/blocks/post-featured-image/style-rtl.css 153 B
build/block-library/blocks/post-featured-image/style.css 153 B
build/block-library/blocks/post-template/editor-rtl.css 99 B
build/block-library/blocks/post-template/editor.css 98 B
build/block-library/blocks/post-template/style-rtl.css 282 B
build/block-library/blocks/post-template/style.css 282 B
build/block-library/blocks/post-terms/style-rtl.css 73 B
build/block-library/blocks/post-terms/style.css 73 B
build/block-library/blocks/post-title/style-rtl.css 80 B
build/block-library/blocks/post-title/style.css 80 B
build/block-library/blocks/preformatted/style-rtl.css 103 B
build/block-library/blocks/preformatted/style.css 103 B
build/block-library/blocks/pullquote/editor-rtl.css 198 B
build/block-library/blocks/pullquote/editor.css 198 B
build/block-library/blocks/pullquote/style-rtl.css 370 B
build/block-library/blocks/pullquote/style.css 370 B
build/block-library/blocks/pullquote/theme-rtl.css 167 B
build/block-library/blocks/pullquote/theme.css 167 B
build/block-library/blocks/query-pagination-numbers/editor-rtl.css 122 B
build/block-library/blocks/query-pagination-numbers/editor.css 121 B
build/block-library/blocks/query-pagination/editor-rtl.css 221 B
build/block-library/blocks/query-pagination/editor.css 211 B
build/block-library/blocks/query-pagination/style-rtl.css 234 B
build/block-library/blocks/query-pagination/style.css 231 B
build/block-library/blocks/query/editor-rtl.css 369 B
build/block-library/blocks/query/editor.css 369 B
build/block-library/blocks/quote/style-rtl.css 213 B
build/block-library/blocks/quote/style.css 213 B
build/block-library/blocks/quote/theme-rtl.css 223 B
build/block-library/blocks/quote/theme.css 226 B
build/block-library/blocks/read-more/style-rtl.css 132 B
build/block-library/blocks/read-more/style.css 132 B
build/block-library/blocks/rss/editor-rtl.css 202 B
build/block-library/blocks/rss/editor.css 204 B
build/block-library/blocks/rss/style-rtl.css 289 B
build/block-library/blocks/rss/style.css 288 B
build/block-library/blocks/search/editor-rtl.css 165 B
build/block-library/blocks/search/editor.css 165 B
build/block-library/blocks/search/style-rtl.css 385 B
build/block-library/blocks/search/style.css 386 B
build/block-library/blocks/separator/editor-rtl.css 146 B
build/block-library/blocks/separator/editor.css 146 B
build/block-library/blocks/separator/style-rtl.css 233 B
build/block-library/blocks/separator/style.css 233 B
build/block-library/blocks/separator/theme-rtl.css 194 B
build/block-library/blocks/separator/theme.css 194 B
build/block-library/blocks/shortcode/editor-rtl.css 464 B
build/block-library/blocks/shortcode/editor.css 464 B
build/block-library/blocks/site-logo/editor-rtl.css 708 B
build/block-library/blocks/site-logo/editor.css 708 B
build/block-library/blocks/site-logo/style-rtl.css 192 B
build/block-library/blocks/site-logo/style.css 192 B
build/block-library/blocks/site-tagline/editor-rtl.css 86 B
build/block-library/blocks/site-tagline/editor.css 86 B
build/block-library/blocks/site-title/editor-rtl.css 84 B
build/block-library/blocks/site-title/editor.css 84 B
build/block-library/blocks/social-link/editor-rtl.css 177 B
build/block-library/blocks/social-link/editor.css 177 B
build/block-library/blocks/social-links/editor-rtl.css 674 B
build/block-library/blocks/social-links/editor.css 673 B
build/block-library/blocks/social-links/style-rtl.css 1.37 kB
build/block-library/blocks/social-links/style.css 1.36 kB
build/block-library/blocks/spacer/editor-rtl.css 322 B
build/block-library/blocks/spacer/editor.css 322 B
build/block-library/blocks/spacer/style-rtl.css 48 B
build/block-library/blocks/spacer/style.css 48 B
build/block-library/blocks/table/editor-rtl.css 494 B
build/block-library/blocks/table/editor.css 494 B
build/block-library/blocks/table/style-rtl.css 611 B
build/block-library/blocks/table/style.css 609 B
build/block-library/blocks/table/theme-rtl.css 175 B
build/block-library/blocks/table/theme.css 175 B
build/block-library/blocks/tag-cloud/style-rtl.css 226 B
build/block-library/blocks/tag-cloud/style.css 227 B
build/block-library/blocks/template-part/editor-rtl.css 149 B
build/block-library/blocks/template-part/editor.css 149 B
build/block-library/blocks/template-part/theme-rtl.css 101 B
build/block-library/blocks/template-part/theme.css 101 B
build/block-library/blocks/text-columns/editor-rtl.css 95 B
build/block-library/blocks/text-columns/editor.css 95 B
build/block-library/blocks/text-columns/style-rtl.css 166 B
build/block-library/blocks/text-columns/style.css 166 B
build/block-library/blocks/verse/style-rtl.css 87 B
build/block-library/blocks/verse/style.css 87 B
build/block-library/blocks/video/editor-rtl.css 561 B
build/block-library/blocks/video/editor.css 563 B
build/block-library/blocks/video/style-rtl.css 159 B
build/block-library/blocks/video/style.css 159 B
build/block-library/blocks/video/theme-rtl.css 110 B
build/block-library/blocks/video/theme.css 110 B
build/block-library/common-rtl.css 987 B
build/block-library/common.css 984 B
build/block-library/editor-rtl.css 10.2 kB
build/block-library/editor.css 10.2 kB
build/block-library/elements-rtl.css 54 B
build/block-library/elements.css 54 B
build/block-library/reset-rtl.css 478 B
build/block-library/reset.css 478 B
build/block-serialization-default-parser/index.min.js 1.11 kB
build/block-serialization-spec-parser/index.min.js 2.83 kB
build/compose/index.min.js 11.7 kB
build/customize-widgets/index.min.js 11.2 kB
build/customize-widgets/style-rtl.css 1.4 kB
build/customize-widgets/style.css 1.4 kB
build/data-controls/index.min.js 653 B
build/data/index.min.js 7.95 kB
build/date/index.min.js 32 kB
build/deprecated/index.min.js 507 B
build/dom-ready/index.min.js 324 B
build/dom/index.min.js 4.65 kB
build/edit-navigation/style-rtl.css 4.03 kB
build/edit-navigation/style.css 4.04 kB
build/edit-post/classic-rtl.css 546 B
build/edit-post/classic.css 547 B
build/edit-widgets/style-rtl.css 4.36 kB
build/edit-widgets/style.css 4.36 kB
build/element/index.min.js 4.27 kB
build/escape-html/index.min.js 537 B
build/format-library/index.min.js 6.75 kB
build/format-library/style-rtl.css 571 B
build/format-library/style.css 571 B
build/hooks/index.min.js 1.64 kB
build/html-entities/index.min.js 448 B
build/i18n/index.min.js 3.77 kB
build/is-shallow-equal/index.min.js 527 B
build/keycodes/index.min.js 1.38 kB
build/list-reusable-blocks/index.min.js 1.74 kB
build/list-reusable-blocks/style-rtl.css 835 B
build/list-reusable-blocks/style.css 835 B
build/media-utils/index.min.js 2.9 kB
build/notices/index.min.js 953 B
build/nux/index.min.js 2.05 kB
build/nux/style-rtl.css 732 B
build/nux/style.css 728 B
build/plugins/index.min.js 1.94 kB
build/preferences-persistence/index.min.js 2.22 kB
build/preferences/index.min.js 1.3 kB
build/primitives/index.min.js 933 B
build/priority-queue/index.min.js 612 B
build/react-i18n/index.min.js 696 B
build/react-refresh-entry/index.min.js 8.44 kB
build/react-refresh-runtime/index.min.js 7.31 kB
build/redux-routine/index.min.js 2.69 kB
build/reusable-blocks/index.min.js 2.22 kB
build/reusable-blocks/style-rtl.css 256 B
build/reusable-blocks/style.css 256 B
build/server-side-render/index.min.js 1.61 kB
build/token-list/index.min.js 644 B
build/url/index.min.js 3.61 kB
build/vendors/react-dom.min.js 38.5 kB
build/vendors/react.min.js 4.34 kB
build/viewport/index.min.js 1.08 kB
build/warning/index.min.js 268 B
build/widgets/index.min.js 7.19 kB
build/widgets/style-rtl.css 1.16 kB
build/widgets/style.css 1.16 kB
build/wordcount/index.min.js 1.06 kB

compressed-size-action

@spacedmonkey spacedmonkey self-requested a review June 27, 2022 15:38
@spacedmonkey
Copy link
Member

Will this functionality be ported to 6.0.1 or 6.0.2 Seems like a big enough bug that worth including.

@draganescu
Copy link
Contributor Author

I did something and I see some changes that should not be here. Will fix.

@spacedmonkey I don't see why not.

Copy link
Contributor

@ntsekouras ntsekouras left a comment

Choose a reason for hiding this comment

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

Thanks for iterating on this one @draganescu !

In general after testing this seems to work well. We would need to also make similar changes to No Results block.

A small concern would be this removed snippet:

if ( empty( $query_args['post_type'] ) && is_singular() ) {
$query_args['post_type'] = get_post_type( get_the_ID() );
}

But I don't think it breaks anything. My understanding is that this would just have some fallback in the case of inherit in a single page, where now it shows nothing. This is not a blocker for me as in general it doesn't make sense to have such a query in a single page. I think this is part of the context detection issue we have, where we should better handle the Query Loop based on the context it lives. --cc @aristath

Finally, I don't think we should rush it in core, as it's not a bug that creates problems. To the contrary, it would be better to give it time to test it in the plugin for any regression/issues.

@aristath
Copy link
Member

aristath commented Jul 5, 2022

My understanding is that this would just have some fallback in the case of inherit in a single page, where now it shows nothing. This is not a blocker for me as in general it doesn't make sense to have such a query in a single page. I think this is part of the context detection issue we have, where we should better handle the Query Loop based on the context it lives. --cc @aristath

If a page/post template does not exist, then the WP template system and hierarchies fallback to using the main index template. If I'm not mistaken that's why this was added... It was just an extra line of defense to accommodate scenarios like that.
It is possible to have a block theme with nothing but an index.html template... and WP should be able to handle these cases. So I should be able to build a theme with an index.html template which can handle both archives and single posts

@ntsekouras
Copy link
Contributor

Thanks for explaining Ari!

It is possible to have a block theme with nothing but an index.html template... and WP should be able to handle these cases. So I should be able to build a theme with an index.html template which can handle both archives and single posts

I just tested that(only index.html and inherit:true) and seems to work well because it actually adds the name param. I'd love some more testing though to be sure 😄

@adamziel adamziel added the Backport to WP Minor Release Pull request that needs to be backported to a WordPress minor release label Jul 5, 2022
@adamziel
Copy link
Contributor

adamziel commented Jul 5, 2022

@ntsekouras said:

Finally, I don’t think we should rush it in core, as it’s not a bug that creates problems. To the contrary, it would be better to give it time to test it in the plugin for any regression/issues.

And so I won't be rushing this into 6.0.1 RC1 that happens in a few hours. Instead, I added the backport label to flag this for 6.0.2 release squad.

@draganescu draganescu changed the title Reset the sticky posts query var for inherited query Remove sticky posts setting when we inherit the query Jul 5, 2022
@draganescu
Copy link
Contributor Author

@ntsekouras I'll add the updates to the no results block. @huubi and @spacedmonkey I've added you both as co-authors since the optimisation to the query was not part of the original intention or code of the PR.

@draganescu
Copy link
Contributor Author

draganescu commented Jul 5, 2022

@ntsekouras while adding the update to the no results block I realized I don't understand why do we need to query anything if we render a no results page, since we know there are no results? What was the intention there?

Later edit:

It seems this block could get the information if there are no posts from context rather than re-querying just to decide if it is going to render itself.

Copy link
Contributor

@ntsekouras ntsekouras left a comment

Choose a reason for hiding this comment

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

Thanks @draganescu and also @huubi and @spacedmonkey for contributing in this one!

I'd appreciate some more testing about this #40656 (comment), but other than that looks good. With 🟢 CI let's ship.

@aristath aristath merged commit 2caadbf into trunk Jul 6, 2022
@aristath aristath deleted the fix/sticky-posts-not-sticky branch July 6, 2022 07:27
@github-actions github-actions bot added this to the Gutenberg 13.7 milestone Jul 6, 2022
gziolo pushed a commit that referenced this pull request Aug 23, 2022
* reset the sticky posts query var for inherited query

* merge query vars into query defaults and unset empty s query var

* unser all query block defaults which are not editable when inherit is true

* fixes linting PHP problems

* uses global query as is and hides and UI that overrides it

* removes mistaken changes

* removes query overriding on inherit for no results block, although why?

* Update packages/block-library/src/query-no-results/index.php

* Adds co-authors

Co-authored-by: Jonny Harris <[email protected]>
Co-authored-by: Huub <[email protected]>

* Update packages/block-library/src/query-no-results/index.php

CS alignments fix

Co-authored-by: Jonny Harris <[email protected]>
Co-authored-by: Huub <[email protected]>
Co-authored-by: Ari Stathopoulos <[email protected]>
@gziolo
Copy link
Member

gziolo commented Aug 23, 2022

Cherry-picked with 493d14f for WordPress 6.0.2 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Query Loop Affects the Query Loop Block Needs Technical Feedback Needs testing from a developer perspective. [Type] Bug An existing feature does not function as intended
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Sticky Posts not sticking
9 participants