Post Template: Update template title selector #42091
Merged
Conversation
This file contains 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
Mamaduka
added
[Type] Enhancement
|
Size Change: -9 B (0%) Total Size: 1.26 MB
ℹ️ View Unchanged
|
|
This worked fine for me ✨ That said, I was wondering if we could improve the way we load that item: the Template field appears after the names of the templates are fetched, causing the contents below to move, and once it's loaded we show "Default template" for a brief moment. Do you think it would be a good idea to address those things in a new issue? Screen.Recording.2022-07-01.at.15.20.43.mov |
|
@javierarce, I tried to improve that, but the selector is a bit slow. Maybe @noisysocks has some ideas on how to improve this. |
noisysocks
requested changes
Jul 4, 2022
There was a problem hiding this comment.
This doesn't play nicely with the old style of post template which you can see by activating Twenty Twenty.
Kapture.2022-07-04.at.11.37.09.mp4
Probably we need to use template.title if template exists but fall back to settings.availableTemplates[ templateSlug ] otherwise.
packages/edit-post/src/components/sidebar/post-template/index.js
Outdated
Show resolved
Hide resolved
|
Another thing: Should we update "Default template" in the dropdown to be "Single" or "Default template (Single)"?
|
I suppose we have to preload the request to Or, an idea 💡: What if, instead of the approach in this PR, we update the PHP which sets |
|
@noisysocks, I like the idea, but I remember there was an issue, which is why we're supplying only I will experiment with this and post my findings. |
Mamaduka
force-pushed
the
try/post-template-change-selector
branch
from
6bf0c6b to
65ec4dc
Compare
|
@noisysocks, the selector now checks This fixed issue with Classic Themes. No more delay until the template name is displayed unless we have to resolve the default template. Re: #41449: Unfortunately, we don't have a method to get the current post template in WP admin on the server side. But I think we can replace the default option on the client side. I want to try this in a separate PR, if you don't mind. |
|
This works but I'm confused about why it works 😅. In a theme like Twenty Twenty, |
We can return early, before fetching templates, if a theme doesn't support template mode. |
|
Right but |
Mamaduka
force-pushed
the
try/post-template-change-selector
branch
from
65ec4dc to
7ff4411
Compare
|
@noisysocks, I've updated the logic once again. Now the This mostly matches the previous behavior of the Template panel. |
noisysocks
approved these changes
Aug 3, 2022
24 tasks
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?
PR updates template title selector to use
getEditedPostTemplate.Why?
The
getEditedPostTemplatealso tried to resolve titles for default WP templates, like "Single" and "Page." The Template panel used this selector as well.Testing Instructions
Screenshots or screencast