-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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 resolution for templates assigned as home page #56418
Conversation
Size Change: +437 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
Flaky tests detected in 65760c2. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6954576182
|
I probably am missing how it should work. What I did:
How should I actually test this? |
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.
Would be good to get a second review, but this works nicely for me, and now allow us to edit the template of a page that has been set as "home" in the reading settings.
2023-11-23.10.31.18.mp4
Thanks!!
I tried to swap and the front page template didn't show
Edit: I didn't see @draganescu's comment:
Here, I'm editing the front page's template, but nothing apparent happens. What it's doing however, is changing the template of my static home page (set in reading settings)
2023-11-23.10.45.28.mp4
packages/edit-site/src/components/sync-state-with-url/use-init-edited-entity-from-url.js
Outdated
Show resolved
Hide resolved
// Don't resolve the template yet. | ||
if ( frontPageTemplateId === undefined ) { | ||
return undefined; | ||
} | ||
|
||
if ( !! frontPageTemplateId ) { | ||
return frontPageTemplateId; | ||
} |
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.
Are there any other values that frontPageTemplateId
could be other than undefined|string
?
Could we just return frontPageTemplateId;
without the two if
s?
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.
Further up in the useSelect
the value false
is used to denote that no templates match the front page template. In this case, a page is set to be the front page, but there's no front page template, so these two if statements (and early returns) should be skipped, with the rest of this function continuing to execute to find the right template to use.
I think that's what's happening, anyway!
Might need a second opinion - I'm not sure how template selection should operate here.
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 feels closer to me, thanks for the follow-up:
Trunk (uses Pages template) | This PR (uses Front Page template) |
---|---|
I also ran into a bit of friction using the Template controls. Perhaps since we know that when accessing the root of the site editor in this way, where the template is forced to override whatever is set at the page level, we could disable the control to Swap Template from this view (at the root of the site, versus explicitly editing that page), possibly with an info icon or something to hover over that flags that the front page template is fixed in position due to what's set under Reading settings? Otherwise I can imagine folks getting confused, as it looks like you can change which template you're using, but you can't really do that, since it's being overwritten.
Hope that all makes sense, I've tried rewriting that paragraph a couple times, but it still reads in a jumbled way 😅! Here's a screengrab of what I'm seeing for a user who attempts to switch templates when accessing the site editor when a page is set to front page and a front page template exists:
2023-11-23.11.54.25.mp4
In the above screengrab, it looks like you can change the template, and the Swap Template modal seems to reflect that you have (for that particular page), but the UI always shows "front page" since it's (correctly) being overwritten. In this case, this is where I'm wondering if the Swap Template option should be disabled with a note that it's locked due to the Reading setting.
Ah, I see — looks like it's different behaviour when we go to access the page via the Pages menu, too (versus the root of the site editor). Perhaps it should be consistent there as well, since the URL for the page winds up pointing to the root of the site, anyway, once we set a page to front page? |
Yeah, I noticed that too and I kind of wanted it to be a follow-up but I'll see if I can hide the swap or something quickly.
Yes, this should be consistent, I'll take a look. |
Something doesn't seem to be working correctly for me. Static homepage is set, Front Page exists, I'm still seeing the frontpage.mp4 |
@jameskoster it should be fixed (string to number comparison) |
yes, this is the same issue I mentioned in the original PR (if there's no post blocks in the front page template outside query loop). It's an issue that exists in trunk and that will be fixed separately. |
Co-authored-by: Ramon <[email protected]>
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.
LGTM, thanks!
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.
Nice fix, thanks for the follow-up!
closes #56267
Alternative to #56326
What
If a page was set as "static home page", and you had a "front page" template on your theme, this template was not being picked as the template to use for your home page, instead we were always using the default "page" template regardless of whether that template exited.
This PR fixes that by checking for the existence of the front page template if the page that we're resolving is the one set as "fixed home page".
Why?
I initially tried to solve this problem by relying on a server API to resolve the template, the problem is that unsaved user changes might impact the resolved template and we do want to reflect these changes in the site editor.
Testing Instructions
1- Create a random page and use it as "static home page" in your "reading" settings
2- Ensure that you have a front page template on your theme
3- Open the site editor: The front page template should be loaded for your page.