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

Query Loop: It doesn't show sticky posts at the top when the query type is the default in the editor. #68595

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from

Conversation

dhananjaykuber
Copy link
Contributor

@dhananjaykuber dhananjaykuber commented Jan 10, 2025

Fixes #68570

What?

Fixes the issue where sticky posts were not appearing at the top of Query Loop block results in the editor view, while they do appear at the top on the front end.

Why?

There was an inconsistency between how sticky posts were displayed in the editor versus the front end. This caused confusion for users as the editor preview didn't accurately reflect what visitors would see on the site. The issue occurs because the editor view wasn't properly handling the sticky post ordering, while the PHP rendering on the front end did show sticky posts at the top by default.

Testing Instructions

  1. Create or edit a page that uses the Query Loop block
  2. Make sure you have at least one sticky post on your site (you can make a post sticky in the post editor's Document Settings panel)
  3. In the editor, observe that sticky posts now appear at the top of the Query Loop results
  4. Preview the page on the front end to verify that the editor view matches the front-end display

Screenshots or screencast

Screenshot 2025-01-13 at 11 35 44 AM
Screenshot 2025-01-13 at 11 35 50 AM
Screenshot 2025-01-13 at 11 36 00 AM

@dhananjaykuber dhananjaykuber marked this pull request as ready for review January 12, 2025 16:22
Copy link

github-actions bot commented Jan 12, 2025

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: dhananjaykuber <[email protected]>
Co-authored-by: carolinan <[email protected]>
Co-authored-by: iamtakashi <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@Mamaduka Mamaduka added [Type] Bug An existing feature does not function as intended [Block] Query Loop Affects the Query Loop Block labels Jan 13, 2025
Copy link
Contributor

@carolinan carolinan left a comment

Choose a reason for hiding this comment

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

Thank you for working on this, it would be great to have this fixed.

I don't think the logic is correct: When I switch the post type option on the query block to use pages, they show twice in the editor. If I then switch back from pages to posts, the pages still show in the editor.

@dhananjaykuber
Copy link
Contributor Author

Hi @carolinan,

Thank you for pointing out the bug, I have fixed it and tested on my local

@carolinan
Copy link
Contributor

I have tested order, filters, switching post type, include, exclude and only without finding any issues.
I would like to see some more reviews in case I missed something.

// If sticky is not set, it will return all posts in the results.
// If sticky is set to `only`, it will limit the results to sticky posts only.
// If it is anything else, it will exclude sticky posts from results. For the record the value stored is `exclude`.
if ( sticky === 'only' ) {
Copy link
Contributor

@carolinan carolinan Jan 22, 2025

Choose a reason for hiding this comment

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

Perhaps these two conditions could be combined into one, where the value of sticky depends on if it is only/exclude?
It depends on what everyone prefers in terms of readability, but also on how we want to manage the introduction of ignore, #66222, which relies on this issue.

Copy link
Member

@Mamaduka Mamaduka left a comment

Choose a reason for hiding this comment

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

PR needs more work based on the conditions; the Query block might make three REST API calls to retrieve posts, negatively impacting the Site Editor loading performance.

Ideally, it should only make one and Sticky post logic should be baked in the REST controller.

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 [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Query Loop: It doesn't show sticky posts at the top when the query type is the default in the editor.
3 participants