[Site Editor]: Re-architect templates addition #42457
Merged
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
ntsekouras
added
[Type] Code Quality
ntsekouras
requested review from
mcsf,
scruffian,
mtias,
jameskoster,
carolinan and
jorgefilipecosta
|
Size Change: +174 B (0%) Total Size: 1.26 MB
ℹ️ View Unchanged
|
|
Seems to be working well for me. In the menu, do you think we could display the taxonomies as:
Instead of
That way the formatting of the menu item will match the name of the template that gets created. |
@jameskoster done. I also pushed a small fix where I appended |
packages/edit-site/src/components/add-new-template/new-template.js
Outdated
Show resolved
Hide resolved
| taxonomies, | ||
| entitiesConfig.taxonomy, | ||
| onClickMenuItem | ||
| ); | ||
| const missingTemplates = [ |
There was a problem hiding this comment.
We are returning a new array reference on every rerender. The array computation is also not trivial, e.g.: it has sorting operations. I guess we compute missingTemplates inside a useMemo.
There was a problem hiding this comment.
Are you referring to missingTemplates? If yes I'm not sure what we'll gain with useMemo here..
| menuItem[ 0 ] | ||
| const { defaultTaxonomiesMenuItems, taxonomiesMenuItems } = | ||
| useTaxonomiesMenuItems( onClickMenuItem ); | ||
| const { defaultPostTypesMenuItems, postTypesMenuItems } = |
There was a problem hiding this comment.
Why are we separating between defaults ( defaultPostTypesMenuItems) and non-defaults (postTypesMenuItems) it seems we can just return a single array and remove the separation code. Then all of the items would pass by the next forEach loop, but just the defaults would be found on enhancedMissingDefaultTemplateTypes.
There was a problem hiding this comment.
This way we can control where we want to append the extra menu items besides the default ones. It's also related to the current sorting which might be subject to change(DEFAULT_TEMPLATE_SLUGS and the such) - not in this PR 😄
jorgefilipecosta
approved these changes
Jul 15, 2022
ntsekouras
added
[Feature] Site Editor
ntsekouras
changed the title
ntsekouras
force-pushed
the
update/re-architect-templates-addition
branch
from
c3b1f26
to
9403668
Compare
bph
pushed a commit
that referenced
this pull request
Jul 19, 2022
* [Full Site Editing]: Re-architect templates addition * update taxonomy menu item title * feedback
mburridge
added
the
Needs Dev Note
|
Added the Needs Dev Note label in case this needs a dev note (either individual or as part of a "misc" dev note) for WP 6.1 release. |
38 tasks
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.
What?
Follow up of: #41875
Addresses: #41875 (comment)
Updated architecture for new templates creation
Related: #42191 (comment)
The first implementation for adding templates used a
pure config per entityapproach, that seemed to work well. What we had was a config per entities and their special cases(ex. taxonomy, category, tag, etc..) and lot's of code was reused based on this. With the naming conflicts issue it become clear that we needed to handle all entities of the same type in the same place.This PR removes the configs and absorbs them into some bigger monolithic helper hooks per entity:
taxonomy, postType. This way can solve the naming conflicts (and any similar issue that might occur in the future). There is still a lot of reusable code, especially in the template suggestions. Another change was that these new hooks also absorb the new specific template's title and description, so we can manage menu items and templates info in the same place.Naming conflicts issue
In site editor a user can create various templates and as of #41875, can create any taxonomy template. The design to create a template uses a taxonomy's
labels.The problem here is that some taxonomies(ex: Product categories from WooCommerce) have some identical labels with Category. This results to:
Categoryis the WP category andSingle taxonomy: Categoryis the Products category that happens to have the same label. We face the same situation in the create template's description.How?
This could occur at post types as well and is handled in this PR. The only unique identifier seems to be the
slugand if there are naming conflicts we add this info to the menu item and templates title.After
Screen.Recording.2022-07-15.at.10.34.29.AM.mov
Testing instructions