-
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 part placeholders - use label and icon per variation. #31721
Template part placeholders - use label and icon per variation. #31721
Conversation
Size Change: +470 B (0%) Total Size: 1.32 MB
ℹ️ View Unchanged
|
Nice. When I search for a "Header" I only see the regular Template Part block: Is that because an existing template part has already been assigned to the Not sure if this is relevant enough to solve here or work on separately, but it would be nice to use the corresponding icon / label in the parent selector and list view too: |
@jameskoster - that is interesting! It looks like the variations aren't registered on your end. I wonder why this is? 🤔 On my end, "Header" is still in the inserter, and the icons for parent selector and list view are the expected ones as well. |
I tried switching themes and deleting all my customised template parts, no change 🤔 |
Interesting. Im trying to investigate but having problems reproducing the issue. |
@jameskoster you should be able test this using |
Ah nice! I suspect these variations will change a bit as we work on #31750, but I can confirm that this is working and feels like an improvement over what we have. |
100% - an easy upgrade for the time being 😁 |
Would it be easy to change the copy as well?
And on the button:
|
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.
Added two minor comments, but this works well in my tests! ✨
packages/block-library/src/template-part/edit/placeholder/index.js
Outdated
Show resolved
Hide resolved
packages/block-library/src/template-part/edit/placeholder/index.js
Outdated
Show resolved
Hide resolved
Yup, updated. 😁 |
packages/block-library/src/template-part/edit/placeholder/index.js
Outdated
Show resolved
Hide resolved
packages/block-library/src/template-part/edit/placeholder/index.js
Outdated
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! That actually looks much better. I didn't notice the old "from the list" seems so out of context, I think that copy may have been from back before we had the list hidden under a 'choose existing' button. |
Curious, do we need to worry about casing here? (noticed this as i have to update e2e tests failing) Previously we were casing it as "New template part" now we are "New Template Part". I do prefer the title casing, especially considering it as a block's name, but I see it was all sentence case previously. (Similar Q with the untouched "Choose existing". |
…' of https://github.com/WordPress/gutenberg into try/template-part-placeholder-contextual-icon-and-label
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.
Working as expected! It's a nice improvement!
Since the copy refers to block variations generally, I suppose we shouldn't use title casing. |
Description
This updates the template part placeholder to have title and icon corresponding to their variation. Before this, inserting a "Header" or "Footer" looks like the same placeholder as inserting a general "Template Part". Here, the title and icon are updated to reflect the variation that was selected from the inserter:
How has this been tested?
Insert Header, Footer, and Template Part blocks. Verify that their placeholders resemble what was selected as in the screenshot above.
Screenshots
Types of changes
Checklist:
*.native.js
files for terms that need renaming or removal).