Template activation: don't set site option on edit #71811
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Size Change: +267 B (+0.01%) Total Size: 1.95 MB
ℹ️ View Unchanged
|
kmanijak
reviewed
Sep 22, 2025
packages/edit-site/src/components/template-save-dialog/index.js
Outdated
Show resolved
Hide resolved
6 tasks
|
Flaky tests detected in 93a638c. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/18123574499
|
|
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
youknowriad
reviewed
Sep 29, 2025
packages/editor/src/components/post-publish-panel/postpublish.js
Outdated
Show resolved
Hide resolved
packages/editor/src/components/post-publish-panel/postpublish.js
Outdated
Show resolved
Hide resolved
packages/editor/src/components/post-publish-panel/postpublish.js
Outdated
Show resolved
Hide resolved
youknowriad
reviewed
Sep 29, 2025
| .getEditorSettings(); | ||
|
|
||
| // Don't open for focused entity. | ||
| if ( editorSettings.onNavigateToPreviousEntityRecord ) { |
Contributor
There was a problem hiding this comment.
This is probably not the right fix to be honest. It's an indirect check. I'd be ok keeping it but I think the commend should be a lot more clear than that. we want to check "custom template". It's not out of the question that a regular template is also accessed as a focused entity, and also in the site editor you can also create custom templates without going through the page first.
Contributor
There was a problem hiding this comment.
I'm surprised we don't have previous cases of "isHierarchyTemplate" or something like that.
Member
Author
There was a problem hiding this comment.
I don't think isHierarchyTemplate will help us tbh. It would be possible to have multiple post or page templates and edit them as a focussed entity here without it being the active post/page template. While it needs to be possible to set it active generally, I don't think showing the activation panel in the focussed mode is expected.
Member
Author
There was a problem hiding this comment.
So in other words, the goal is not to check for "custom" templates, but rather if you're editing it outside of a page/post editor context.
youknowriad
reviewed
Sep 29, 2025
youknowriad
reviewed
Sep 29, 2025
packages/editor/src/components/post-publish-panel/postpublish-next-panel-template.js
Outdated
Show resolved
Hide resolved
ellatrix
force-pushed
the
fix/template-activation-site-option
branch
from
b3ecb11 to
54424ed
Compare
youknowriad
reviewed
Sep 30, 2025
|
|
||
| await editor.saveSiteEditorEntities(); | ||
| await editor.saveSiteEditorEntities( { | ||
| isOnlyCurrentEntityDirty: true, |
Contributor
There was a problem hiding this comment.
What is this flag about?
Member
Author
There was a problem hiding this comment.
I think it's to signal that it's not a multi-entity save. This just restores the test to pre-#67125.
youknowriad
approved these changes
Sep 30, 2025
kmanijak
reviewed
Sep 30, 2025
Contributor
kmanijak
left a comment
kmanijak
left a comment
There was a problem hiding this comment.
I ran WooCommerce E2E tests with GB built on this feature branch (woocommerce/woocommerce#61046). We have better result than before. Some tests are still failing, especially related to template customisations but I need to add a step to activate the template through the new flow.
Tomorrow morning I'll add this step so we have better confidence everything works and check few remaining tests that are failing. Two of them fail definitely due to UI change in new templates creation and there just fe that require manual checking. I don't think I'll be able to test the PR deeply due to other assignments but E2E tests run is promising! (FYI: @sunyatasattva)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Closes #71724
Why?
Editing a template should not be setting site settings at the same time.
Additionally, I had to do some major refactoring of the back-end filters due to an e2e correctly failing for post templates. Somehow the test in trunk passes while even though there's an error in the test. Basically, when the template resolution found a specific page template to show (vs the hierarchy), it just adds the template slug to the top of the hierarchy. I decided to reimplement the way Gutenberg intercepts template resolution with a way earlier filter to make the logic a lot more solid and prepare us better for a core backport. It requires us to copy over some core code, but the logic has become a lot clearer.
To do: we need to find a way to NOT show the activation post-publish panel when saving a template after creating one specifically for a post in the post editor.How?
Let's ask the user after editing a theme template if they want to activate the template.
Testing Instructions
With a theme template active for home/index, navigate to the root of the site editor. Edit the template and save. You should see a notice asking whether to activate the new template.
Similarly for theme templates in the templates list, when you edit it and save, you should see the same notice.
Testing Instructions for Keyboard
Screenshots or screencast
Before
After
template-activate-notice.mov