Size Change: +69 B (0%)

Total Size: 1.85 MB

_{compressed-size-action}

Flaky tests detected in 90be4ab.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/14193351177
📝 Reported issues:

• [Flaky Test] Redirection after template creation #60678 in /test/e2e/specs/site-editor/site-editor-url-navigation.spec.js

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.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @genemma, @cheestudio.

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.

Unlinked contributors: genemma, cheestudio.

Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>
Co-authored-by: fabiankaegy <fabiankaegy@git.wordpress.org>
Co-authored-by: luminuu <luminuu@git.wordpress.org>
Co-authored-by: jeflopodev <jeflopodev@git.wordpress.org>
Co-authored-by: carolinan <poena@git.wordpress.org>
Co-authored-by: rinkalpagdar <rinkalpagdar@git.wordpress.org>

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

Thanks for the feedback, @fabiankaegy!

Are you suggesting that we revert #65067 and add a notice near the "Query type" control? I thought the original problem was people changing this setting and expecting default queries to work.

I'm also thinking of updating the Query Loop placeholder logic so that the inherit default value is correctly derived. This way, newly inserted Query blocks will have the correct inheritance value. Unfortunately, this can't be applied to Patterns added via inserter.

I personally prefer UX, when everything "just works", but that always gets complicated when block attributes are enforced during the page load.

@Mamaduka what I am trying to say is that I am very open to which approach gets us there. And I don't know all the intricacies that make it difficult. So again if this is the best we can do with the constraints we have we can totally do this.

For me the current behavior after #65067 was merged is really close to the ideal case from a UX perspective. (not saying that the way it accomplishes it code wise makes sense).

I think what you are suggesting about the block automatically setting it's attribute based on the context where it gets inserted upon insertion makes sense to me. But I think that would build on the work in #65067 🤔

I took the PR for a test and I get the way you implemented it, but I have to agree with @fabiankaegy that this feels more confusing. What I had in mind was something like this:

• Adding a query loop to the block editor has it on inherit: false by default
• The inherit setting comes back to the right sidebar
• When setting it to inherit, a warning message (like the color contrast one) appears telling the user that "this has no effect on a singular page, or you know what you're doing"
• If inherit: true is set, I'd still like to be able to actually change the query loop contents

With the current setup, the last step is not possible, as even setting inherit: true via code editor will bring back the "error" message.

Thank you, @luminuu!

Adding a query loop to the block editor has it on inherit: false by default.

I would like to avoid touching the block defaults in this PR. I think that's a separate discussion.

With the current setup, the last step is not possible, as even setting inherit: true via code editor will bring back the "error" message.

Great point. I'll adjust the logic accordingly.

Changed the place where the warning is displayed. See the updated testing instructions and screenshot.

As I was finishing up this comment I saw your recent update @Mamaduka, so I'll post this comment now and going to test your latest update!

Adding a query loop to the block editor has it on inherit: false by default.

I would like to avoid touching the block defaults in this PR. I think that's a separate discussion.

This is the current default in 6.7, so this wouldn't change. In your PR, when adding the Query Loop to a post and selecting a layout, it's set to to inherit: true:

What I've been pointed out to be a colleague is that with your current setup, it's not possible to create a pattern in the site editor, as the same things happen there too, making it impossible to create a pattern in the site editor with a query loop to be able to reuse it in multiple archive templates:

Here's a mockup of what I'd like to see, having the Default/Custom toggle back and showing a warning message when selecting "Default" in the block editor:

The wording in the message is random and is open for refinement.

I have put together a small flowchart, so it may be easier for us to discuss this:

@Mamaduka Follow-up for your latest commit, that looks great! I was basically too slow with my last comment 😅

One thing I've been thinking about writing my comment above was if for the Query Type option, the Default and Custom options should be swapped, but I'm aware that this would have effect everywhere. But I think that's out of the scope for this PR.

The default value for inherit is true; this hasn't changed to the control that was introduced almost 5 years ago. See #27128. This is why I'm hesitant to change it; it is hard to anticipate the fallout.

One thing I've been thinking about writing my comment above was if for the Query Type option, the Default and Custom options should be swapped, but I'm aware that this would have effect everywhere.

I'm not 100% sure what you mean here, but I'm happy to discuss it separately.

The default value for inherit is true; this hasn't changed to the control that was introduced almost 5 years ago. See #27128. This is why I'm hesitant to change it; it is hard to anticipate the fallout.

gutenberg/packages/block-library/src/query/block.json

Line 27 in 132e8c3

"inherit": true,

My bad, I was too confused here with what is the "default". I agree that this change could cause more harm than good, so lets leave it as is.

One thing I've been thinking about writing my comment above was if for the Query Type option, the Default and Custom options should be swapped, but I'm aware that this would have effect everywhere.

I'm not 100% sure what you mean here, but I'm happy to discuss it separately.

Right now it looks like this:

What I was thinking about was swapping the position of Custom and Default:

But looking at it, I'm not sure if this would make any difference because it depends on the context where the query loop is inserted to make sense (blog post vs. template).

But looking at it, I'm not sure if this would make any difference because it depends on the context where the query loop is inserted to make sense (blog post vs. template).

100%. It might cause more confusion if we flip labels based on context.

@luminuu, @fabiankaegy, any other blockers here?

But looking at it, I'm not sure if this would make any difference because it depends on the context where the query loop is inserted to make sense (blog post vs. template).

I've this one to-do item listed in the description, but I am thinking of working on it in a separate PR. I'm not sure how it will work out and don't want more blocks on this change.

Thank you both for fantastic feedback 🙇

I'm going to let this PR sit over the weekend and merge on Monday if there are no objections.

@Mamaduka Added this to the 6.8.x Editor Tasks board as I'd like to see this change being included in one of the next updates. It needs the requirement of only editing existing files.

Thanks, @luminuu!

It needs the requirement of only editing existing files.

Do you mean that it meets the requirement?

Uhm yeah, that's a bad typo of mine, sorry!

I am checking the PRs with the "Backport to WP Minor Release" label applied to clarify which PRs should be backported to the WordPress 6.8.2 release.

As I understand it, this PR is just to show a small hint, so it can be included as a candidate for the 6.8.2 release. What do you think?

Yes, please add this PR to 6.8.2. It would fix an issue on client sites we're having.

This PR was cherry-picked into the wp/6.8 branch for the WP 6.8.2 release: #70557