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.

Sign up for GitHub

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 #56326

Closed
wants to merge 1 commit into from
Closed

Site Editor: Fix template resolution for templates assigned as home page #56326

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions packages/edit-site/src/components/sync-state-with-url/use-init-edited-entity-from-url.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
NAVIGATION_POST_TYPE,
PATTERN_TYPES,
} from '../../utils/constants';
import { getPathAndQueryString } from '@wordpress/url';

const { useLocation } = unlock( routerPrivateApis );

Expand Down Expand Up @@ -54,7 +55,6 @@ function useResolveEditedEntityAndContext( { postId, postType } ) {
const {
getEditedEntityRecord,
getEntityRecords,
getDefaultTemplateId,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If this PR is considered good, I'm going to include the removal of this selector/resolver/reducer as well since it's something recent (a new API not in core) that is not needed anymore.

__experimentalGetTemplateForLink,
} = select( coreDataStore );

Expand All @@ -67,7 +67,11 @@ function useResolveEditedEntityAndContext( { postId, postType } ) {
postTypeToResolve,
postIdToResolve
);
if ( ! editedEntity ) {
if (
! editedEntity ||
// Edited entities are initialized as an empty object.
Object.keys( editedEntity ).length === 0
) {
return undefined;
}
// First see if the post/page has an assigned template and fetch it.
Expand All @@ -85,10 +89,10 @@ function useResolveEditedEntityAndContext( { postId, postType } ) {
}
}

// If no template is assigned, use the default template.
return getDefaultTemplateId( {
slug: `${ postTypeToResolve }-${ editedEntity?.slug }`,
} );
const path = getPathAndQueryString( editedEntity.link );

// If no template is assigned, use the link to fetch the right template.
return __experimentalGetTemplateForLink( path )?.id;
Copy link
Member

Choose a reason for hiding this comment

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

In my testing, this works well, i.e., the front page template is loaded in preference to the static home page in the reader settings.

However, I noticed that when opening the single page (Design > Pages > Static home page), the front page template is also loaded.

When on a page details view, should the page's template rather be loaded? The page's details in the left sidebar describe the page's meta but display's the front page template, which appears unrelated.

Trunk

On the static page's home details view, we get the pages template, e.g., twentytwentyfour//page.

Screenshot 2023-11-21 at 12 22 42 pm

This PR

Here we're loading twentytwentyfour//front-page

Screenshot 2023-11-21 at 12 22 50 pm

What's the preferred way to handle page details? Should we still load the page's true template so folks can edit it as per other pages?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the "front page" should load even in that case because that's the actual template that is used for that specific page anyway in the frontend.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's correct, Front Page will be used for the site homepage regardless.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the confirmation. 👍🏻

I suppose I just found it confusing to see the page details on the left, including excerpt etc, and not the corresponding representation of that page on the right.

}

// If we're rendering a specific page, post... we need to resolve its template.
Expand Down