-
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
Template Editing Mode: Fix options dropdown #37442
Conversation
Size Change: +1.99 kB (0%) Total Size: 1.13 MB
ℹ️ View Unchanged
|
I don't know that we necessarily have to hide the dropdown here. For example it may be useful to view the description to better understand the currently-applied template – especially as some of them have somewhat ambiguous names. Here's the dropdown when editing the Page template in the site editor: I'm inclined to say we should work towards consistency across editing contexts. IE display the dropdown, without any actions. |
Thanks for catching that. So only hide dropdown when we have nothing to display, like custom templates by theme. |
Yes I think so. Although it may be worth building in a condition to check for a description and display the dropdown when one exists. It seems very likely that it will be possible for custom theme templates to include descriptions in the future. |
I fixed the logic and added back descriptions. |
Sure. I'm all for consistency. |
@jameskoster started working on this, and I changed my mind 😅 I think displaying the Template title twice isn't necessary. |
Hah, I don't have a strong feeling but would be inclined to say we should match the Site Editor for now, and then explore this particular detail in a separate issue. I suspect we'll revise this popover soon, possibly as a part of #36951. |
@jameskoster sounds good. I will update the component tomorrow 👍 |
packages/edit-post/src/components/header/template-title/edit-template-title.js
Show resolved
Hide resolved
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.
Thanks George! Code wise this LGTM.
@jameskoster sounds good. I will update the component tomorrow 👍
Is there anything left to decided/changed?
Yup we need to display the title in the popover to match the site editor. |
Some brainstorming based on what I read above. (It might be a sidetrack...:) Template editing mode should be present in one way or another be it in the Site Editor and (somewhat similar much more limited in the) Post Editor. Let's say that a template area does not have any areas included. It would be helpful to keep the same structure in place. Now let's say that we also need to remove the "Browse all templates" bottom link. Here is the link removed but the bottom black area is still there. Of course it would be better to actually mention why the link is removed instead of just removing the link like I did. |
I updated the styling to match the site editor more closely. This affected the changes in #37450 so I brought those over here as well. |
* Template Editing Mode: Fix options dropdown * Remove unused TemplateDescription component * Revert "Remove unused TemplateDescription component" This reverts commit e56caee. * Display description for default core templates * Match Site Editor design * Update styling to match site editor Co-authored-by: James Koster <james@jameskoster.co.uk>
Description
PR makes the following changes in the Template Title component:
P.S. We should change the "Delete" text to "Clear customizations" when a template has a theme file. I will address that in separate PR since we're in string freeze, and I want this to be backported in beta 4.
Resolves #37427.
How has this been tested?
Using TT2
Screenshots
Types of changes
Bugfix
Checklist:
*.native.js
files for terms that need renaming or removal).