-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Site Editor: fix template for page-on-front option #66739
Conversation
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. |
} else { | ||
// Still resolving `templates`. | ||
return undefined; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See https://github.com/WordPress/gutenberg/pull/66579/files#diff-90ac1a8edea4ffca3eb4acd2bc2aeed05d54392fc843eb1f253403ae66771dceL59-L66, which has the same logic, but spread over three different pieces: two in the first selector, and one in the second. I put them all together here.
Size Change: +26 B (0%) Total Size: 1.82 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This solves the problem in my testing.
Thanks for fixing this so quickly! |
Thanks for not reverting my PR 😄 |
Co-authored-by: ellatrix <[email protected]> Co-authored-by: draganescu <[email protected]> Co-authored-by: ntsekouras <[email protected]> Co-authored-by: mdawaffe <[email protected]>
What?
I removed a bit too much logic in #66579. I thought the
/lookup
REST API request could handle fetching the front-page template when the page-on-front option is set, but it can't. Ideally we should fix the endpoint, but in the meantime, I'll restore the previous logic (and add more inline comments).Why?
Fixes #66687.
How?
Restores logic from #66579, closer to where it's needed and with more comments.
When a front page ID is set, the correct front-page hierarchy is first the
front-page
template, then the specific page template for the front page ID. This was previously manually checked in JS. It looked specifically for the front page template and if it doesn't exist, continue with the remaining logic to find the page template.Testing Instructions
Set a page to be the front page in Reading settings. The front-end's front page should have changed. Check that it's the same in the site editor.
Testing Instructions for Keyboard
Screenshots or screencast