-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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 inspector: Surface related patterns #55091
Conversation
packages/edit-site/src/components/sidebar-edit-mode/template-panel/template-actions.js
Show resolved
Hide resolved
if ( ! title && ! description ) { | ||
return null; | ||
} | ||
|
||
const onTemplateSelect = async ( selectedTemplate ) => { | ||
await editEntityRecord( 'postType', postType, postId, { |
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.
Since record
is already an entity can we edit it directly?
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.
We cannot directly mutate a record as that way the store update would not propagate. Instead, we need to call editEntityRecord as we are doing currently.
) | ||
), | ||
} ) ); | ||
return patterns.filter( |
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.
Should we also filter out the currently used pattern? Can we if we don't know which one it is?
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.
I don't think it's possible to determine the pattern used after it has been applied as there is no information available about the pattern used.
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.
Maybe we can save the slug in the metadata like we do in #59251 - then we will have a way to know.
Size Change: -99 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
I don't want to derail but I am confused about why a template is switched with a pattern and not another template. |
Yeah I agree this is confusing. I was initially very confused and built a mechanism for switching templates for other templates, but then I realised that doesn't make sense. If you switch one template part for another then you end up with two template parts that are the same. I think the flow that is actually more useful (although not the one in #44582) is creating a way to switch template parts from within a template. Maybe I'll try to spin up a PR to do that as well... |
To get back on topic: I prefer this direct visual preview over the previous option because that option was difficult to find. Some design tweaks are needed, but I understand this is early and a draft. |
@@ -64,6 +96,15 @@ export default function TemplatePanel() { | |||
> | |||
<TemplateAreas /> | |||
</SidebarCard> | |||
<p> | |||
{ __( | |||
'Choose a predefined pattern to switch up the look of your template.' // TODO - make this dynamic? |
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.
I think this can be shortened.
But I also think the switch context is important so that the user doesn't think it will just place another pattern in the editor.
Flaky tests detected in 06d4c97. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7975920487
|
6d2025d
to
3bd79a7
Compare
This works but makes the impact of the low performance way in which we preview patterns even more visible:
In the current modal version at least once the fiddling with patterns is done the baseline performance of the editor UI is not broken. |
template.part.swap.movTook this for a spin and have a few notes:
|
It may be better to put it in a PanelBody, perhaps closed initially. :/ |
return null; | ||
} | ||
|
||
return ( |
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.
FYI, to help with performance, in the other places that we list patterns we have added client-side pagination, eg. https://github.com/WordPress/gutenberg/blob/trunk/packages/edit-site/src/components/page-patterns/patterns-list.js#L210, you may want to think about also adding it here, but could be a follow up.
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.
When displaying patterns for a particular template or a part of it, we anticipate that we won't have to show too many patterns. However, if this does become a problem, we can revisit the idea of adding pagination in the future.
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.
Removing this PR from the 6.5 board as I removed the associated issue due to lack of movement in the last few months. Happy to add back if that changes but with barely over a week until beta 1 want to be mindful about any last minute updates. |
Squashed commits: [7bc5ee8707] template-panel/index rebase [4d47f77146] Make titles tooltips (+8 squashed commits) Squashed commits: [3488f2f] Layout tweaks [f450cd5] remove CSS [b3990a7] remove unused file [3bd79a7] update comment [e07cd80] Add comments [aaae875] Let users select a pattern [4aa4272] fetch patterns not template parts [3f3da4b] Add template parts to template parts (whoops)
8d1d5c3
to
515bd8a
Compare
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
The PR was rebased and a Panel was added. |
👋🏻 Looks like this commit was merged with a failing performance test:
What do you folks think should happen? This change deletes Maybe we should update the test to look for "Transform into" instead. I'll have a shot. |
…e HTML structure. Updating performance tests to find new elements.
…e HTML structure. (#59259) Updating performance tests to find new elements. Co-authored-by: ramonjd <[email protected]> Co-authored-by: andrewserong <[email protected]>
It would be interesting to see the impact of this change on performance. Patterns are parsed and prepared during the load ( The performance test uses the cc @ellatrix |
…e HTML structure. (#59259) Updating performance tests to find new elements. Co-authored-by: ramonjd <[email protected]> Co-authored-by: andrewserong <[email protected]>
Yes, I talked to @youknowriad about this. IMO we should be changing a lot of the perf tests to use TT4 which is a "real world" theme. |
…e HTML structure. (#59259) Updating performance tests to find new elements. Co-authored-by: ramonjd <[email protected]> Co-authored-by: andrewserong <[email protected]>
…e HTML structure. (#59259) Updating performance tests to find new elements. Co-authored-by: ramonjd <[email protected]> Co-authored-by: andrewserong <[email protected]>
What?
Fixes #44582
This adds a pattern selector in the template inspector panel. Also removes the replace button added in #54609.
Why?
This makes it much easier for users to switch out templates and template parts for patterns.
How?
Testing Instructions
Screenshots or screencast