Template part 'replace' flow - don't show currently used template part as option. #31720
Conversation
Addison-Stavlo
requested review from
vindl,
jeyip,
mattwiebe,
Copons,
creativecoder,
david-szabo97 and
jameskoster
|
Size Change: +351 B (0%) Total Size: 1.62 MB
ℹ️ View Unchanged
|
Addison-Stavlo
added
[Block] Template Part
|
I still think that if there are no same-area TPs then we should exercise some subtraction and just hide the replace button altogether.
|
packages/block-library/src/template-part/edit/selection/template-part-previews.js
Outdated
Show resolved
Hide resolved
packages/block-library/src/template-part/edit/selection/template-part-previews.js
Outdated
Show resolved
Hide resolved
packages/block-library/src/template-part/edit/selection/template-part-previews.js
Outdated
Show resolved
Hide resolved
packages/block-library/src/template-part/edit/selection/template-part-previews.js
Outdated
Show resolved
Hide resolved
packages/block-library/src/template-part/edit/selection/template-part-previews.js
Outdated
Show resolved
Hide resolved
packages/block-library/src/template-part/edit/selection/template-part-previews.js
Outdated
Show resolved
Hide resolved
packages/block-library/src/template-part/edit/selection/template-part-previews.js
Outdated
Show resolved
Hide resolved
A user might want to replace the template part with a different area template part. I wonder how often that will happen though. So I'm not sure if we should remove the Replace button if there are no same-area TPs. |
|
I don't think that will happen very often and otherwise adds unnecessary weight to the UI. It is very easy to just remove the incorrect TP and insert the correct one. |
|
It shouldn't happen very often, but it definitely could by mistake. As in a user creating what they thought was a header and forgetting to assign it as such. If they went to swap their header, they would need to search for it as it would be general. Hiding the button altogether when there are no other variations in place will also require adding yet another db request on the editor's initial loading phase (as opposed to just when the component is triggered). I don't think it alone makes a huge difference, but in general I think that is something we try to limit if its not completely necessary. |
|
I remain unconvinced that we should be optimising the UI for flows that are essentially edge cases. That said I wasn't aware of the performance expense. Hopefully we get some more design feedback soon. |
|
This seems very unhelpful and confusing. The copy is hard to understand and relying on search (and knowing the name of what you're searching for) seems a little complicated. If we do need this, I think we should improve the copy and perhaps add some more helpful actions. Something like this maybe:
-- That said, I think @jameskoster's suggestion of simply hiding the button might be the simplest option overall. |
|
I like this idea in the sense that it educates the user that certain template parts can be swapped, before they are swappable. A shortcut to create a new TP could be really handy as well. Searching seems a little awkward for the reasons you mentioned. It could be a dead end if you don't know what to search for, and swapping a header with a non-header still feels like an edge-case to me. |
I guess my edge case was more swapping a header for header that happens to be uncategorized. Such as the user created one in the stand-alone template part editor and never assigned its 'area' to be header. Either way, Im starting to think you are right. The search is weird in general and it doesn't really make sense to swap with another 'area' if things are defined correctly. So we can try disabling the button if there are no other options to replace with... Do you think we should scrap the 'search' feature altogether? |
Hopefully the dedicated header / footer block variations make that edge case even less likely.
We can probably leave it in for now, and explore it in more detail in #31747. |
|
Updated! The replace button should no longer be present if there are no other template parts of the specified area to switch to. |
|
Code LGTM and it seems to work well! ✨ Two things:
|
Updated 😁
Technically you shouldn't be able to see it, it just seemed like a better fallback than the previous So unless someone adds that component somewhere else without a condition and they are running a theme that supplies no template parts and somehow triggers FSE, I don't think there should be a way for that to appear. |
There was a problem hiding this comment.
I think this makes sense and works well on the design side.
Shaun's idea to enable folks to create a new template part in this scenario is worth exploring, but we can do that in a follow-up or as part of #31750
|
Ah, one small thing, can we make the fallback copy contextual?
Perhaps worth putting it inside an informational There is also a typo: "Searchnig". |
packages/block-library/src/template-part/edit/utils/create-template-part-id.js
Outdated
Show resolved
Hide resolved
Ah yeah. I did remove that specific fallback altogether as that situation shouldn't currently happen but it probably does make sense to keep it just in case (just as with the other fallback 😆 )!
I wouldn't think so. That fallback message should never actually appear in the first place, and if it did I think it would be plenty visible where it is so I'm not sure a notice would add much? |
Re-added this fallback and made it contextual. Although, it shouldn't be observable as things are currently laid out (buttons hidden, etc.). 😁 |
|
Should we make this button primary in this case? It's weird to have a secondary button here without any other buttons.
|
|
@david-szabo97 that makes sense to me! Updated it to be primary when its the only button:
Although side note to other folks that this will probably not be seen often as it requires the theme to not supply any template parts as well. |
|
It's much better now!
That's fine 😄 🚢 🚢 🚢 🚢 🚢 🚢 |
|
Is that message correct? It says "pick an existing one from the list" but there is no list, correct? Maybe it should be more general about the purpose of the template part, rather than explaining the available options that may or may not exist. |
|
Hi @shaunandrews ! 👋
Thats the copy that has been in place on the placeholder for a while. Its currently being updated on another PR - #31721 (comment) - "Choose an existing template part or create a new one". But you are right, in this edge case there would be no "Choose" option either. 🤔 |
|
It could display a separate copy in that edge case that there are none to choose from, we could come up with a copy that is more general as you say, or we could look at just automatically triggering the 'create new' step in that case. Id be happy to open a follow up to explore options too. |
Il start by adding that in here for that edge case to just be "Create a new template part" and re can explore other alternatives later if necessary. |
Addison-Stavlo
deleted the
try/template-part-replace-exclude-current-entity
branch
Description
Following up on suggestions from #31520 - here we have removed the currently used template part from the replacement options and hide the selection component if no corresponding template parts are present.
This also slightly updates the area group labels to be a little more clear: "Area: Header" as opposed to just "Header".
How has this been tested?
Test current template part creation/selection flows, as well as replacement flows. Verify that:
Screenshots
Types of changes
Checklist:
*.native.jsfiles for terms that need renaming or removal).