Template parts: use the template actions component for template parts patterns #54173
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
ramonjd
added
[Block] Template Part
|
Not sure about whether this PR introduces too much functionality by simply dropping the It works apparently, but it seems too good to be true 😄 |
|
Size Change: +1.44 kB (0%) Total Size: 1.52 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Thanks for putting this together @ramonjd 👍
It tests as advertised for me however I think where we're sending the user after deleting a custom template part needs tweaking. I've left an inline comment around that topic.
Personally, I don't think it hurts to introduce the renaming/delete actions as well. There were plans to allow those via the command palette, so if nothing else that shows a need to be able to perform those actions when editing a template part.
Following on from that, if we are adding that functionality for template parts, we should do the same for other patterns. Rather than close one gap, only to open another.
That could always be covered via a follow-up though, what do you think?
Comment on lines
49
to
51
| onRemove={ () => { | ||
| navigator.goTo( `/${ postType }/all` ); | ||
| } } |
There was a problem hiding this comment.
We should probably make the onRemove navigate back to the same location as the back button i.e. the category page when the user has come from there to edit the template part. The backPath value could be reused here.
There was a problem hiding this comment.
Thanks for picking up on this, and for the suggestion 🙇🏻
💯 Sounds good! I appreciate the feedback. |
|
Nice! No problem including the other actions here imo. They're working well and I agree with @aaronrobertshaw's suggestion about the post-deletion destination. One small detail I noticed: when resetting a template part from the inspector the resulting Snackbar includes an undo affordance, it would be good to include that here if possible?
Let's mimic the confirmdialog that appears on the main pattern management view, IE use the name rather than 'template part': 
We might do the same in the 'Clear customizations' action too, but let's look into that separately. |
I get the feeling it is intentional that undo isn't enabled here. It's the same component used by the table list, where an undo wouldn't really be appropriate. But it could be refactored. Here are a couple of things:
So you can see that it's probably worth another PR 😄
Point "3" above would cover that I reckon.
Done! Just FYI, this change will affect all deletions from the sidebar AND the template table list. 
|
|
Flaky tests detected in 252549e. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6092149503
|
|
Update to the above comment in relation to adding an Undo button. I've been playing around, and any changes would have to take the Undo stack into account. When users clear customizations in edit mode, then switch to view mode without saving, the sidebar doesn't know that the customization have been cleared and still allows "clearing", which leads to some confusing UI and potentially destructive actions. 2023-09-07.12.57.47.mp4I think to prevent this we'd have to fetch the raw theme template and compare it with the editor contents maybe. |
|
@ramonjd sorry for the distraction with the undo – let's leave it for now. There's no undo when resetting a template from the details panel so we'll be consistent with that. Perhaps in a follow-up we could add a confirmDialog instead of adding an 'Undo' to the Snackbar. I assume that would be less awkward to handle?
I think that's fine, it makes the UI more flexible which seems like a win. |
jameskoster
approved these changes
Sep 7, 2023
No worries at all. It was good for me to explore the options and learn about what's possible, just in case it comes up again. Thank you! |
My brain is not functioning today. I want to say "yes" but I can't imagine it without testing first. I guess if we force a save after Undo in all occasions it might. 😅 |
ramonjd
added a commit
that referenced
this pull request
Sep 8, 2023
…ete action to single pattern view in the site editor sidebar in view mode. See: #54173 (review)
1 task
I've thrown together a messy PR to gauge what would be involved. The PR itself works, but I think there's a lot of room for refactor/consolidation: |
Ah, I just meant that after clicking "Clear customisations" in the details panel an "Are you sure?" |
ramonjd
added a commit
that referenced
this pull request
Sep 14, 2023
…ete action to single pattern view in the site editor sidebar in view mode. See: #54173 (review)
ramonjd
added a commit
that referenced
this pull request
Sep 21, 2023
…ete action to single pattern view in the site editor sidebar in view mode. See: #54173 (review)
ramonjd
added a commit
that referenced
this pull request
Sep 25, 2023
…ete action to single pattern view in the site editor sidebar in view mode. See: #54173 (review) Starting to consolidate rename-menu-item.js Tightening up conditions. Removed unused rename component. Set decoded title var Ensure correct back path when deleting templates
ramonjd
added a commit
that referenced
this pull request
Oct 13, 2023
…ete action to single pattern view in the site editor sidebar in view mode. See: #54173 (review) Starting to consolidate rename-menu-item.js Tightening up conditions. Removed unused rename component. Set decoded title var Ensure correct back path when deleting templates
41 tasks
ramonjd
added a commit
that referenced
this pull request
Dec 5, 2023
…ete action to single pattern view in the site editor sidebar in view mode. See: #54173 (review) Starting to consolidate rename-menu-item.js Tightening up conditions. Removed unused rename component. Set decoded title var Ensure correct back path when deleting templates
ramonjd
added a commit
that referenced
this pull request
Dec 12, 2023
…ete action to single pattern view in the site editor sidebar in view mode. See: #54173 (review) Starting to consolidate rename-menu-item.js Tightening up conditions. Removed unused rename component. Set decoded title var Ensure correct back path when deleting templates
ramonjd
added a commit
that referenced
this pull request
Dec 21, 2023
…ete action to single pattern view in the site editor sidebar in view mode. See: #54173 (review) Starting to consolidate rename-menu-item.js Tightening up conditions. Removed unused rename component. Set decoded title var Ensure correct back path when deleting templates
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.
Maybe resolves #54146 not sure
What?
Adds template actions to custom and theme template parts. It goes a bit further than #54146 requires as it also allows renaming and deleting custom template parts.
TODO
How?
Using the existing
<TemplateActions />component used in /packages/edit-site/src/components/sidebar-navigation-screen-template/index.jsTesting Instructions
In a block theme, make some changes to a theme template part (header or footer). Also create a new custom template part.
The same actions available to templates should be present:
When deleting custom template parts, ensure the confirmation dialogues match the entity type ("template type"). This also affects deleting in the table list view (
/wp-admin/site-editor.php?path=%2Fwp_template%2Fall).These actions should not be available for patterns that are not template parts.