-
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
Site Editor: Navigation templates #25739
Conversation
Size Change: +168 B (0%) Total Size: 1.18 MB
ℹ️ View Unchanged
|
Looking forward ideas about how we should handle empty menus. For example
EDIT: Probably this wouldn't work in practice, since we might have other kind of items in the menu. So probably the best way is to create a context and store what should be visible. And nested menus..... |
ac95bcf
to
5b76047
Compare
I don't think we need to show this in the sidebar anymore. At least I haven't seen it in any of latest design iterations. It might make more sense in Document Settings dropdown maybe. Also I don't think we need a Current Template section. That should be obvious from active item and title in the header? |
It's probably fine to leave it as a placeholder for now until we add more items for it. And yes, this should be a separate issue for navigation component and not handled in this PR. |
I left it there so we can overwrite templates. I think it will be replaced by the + button later 🤔 |
In that I case I think it's better to introduce it when we add the +/overwrite functions then? |
cc @ockham since you worked on template resolution before :) |
packages/edit-site/src/components/left-sidebar/navigation-panel/index.js
Show resolved
Hide resolved
packages/edit-site/src/components/left-sidebar/navigation-panel/constants.js
Outdated
Show resolved
Hide resolved
packages/edit-site/src/components/left-sidebar/navigation-panel/template-navigation-items.js
Outdated
Show resolved
Hide resolved
packages/edit-site/src/components/left-sidebar/navigation-panel/template-navigation-items.js
Outdated
Show resolved
Hide resolved
packages/edit-site/src/components/left-sidebar/navigation-panel/template-navigation-items.js
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.
This works nice, thanks @david-szabo97!
I'll open two follow up issues shortly:
- Audit and improve the performances of the new auto-draft creation.
- Discuss with design how to handle template overwrite/revert.
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.
Nice work @david-szabo97! ✨
@@ -177,6 +202,13 @@ function filter_rest_wp_template_collection_params( $query_params ) { | |||
* @return array Filtered $args. | |||
*/ | |||
function filter_rest_wp_template_query( $args, $request ) { | |||
// Create auto-drafts for each theme template files. | |||
$block_template_files = gutenberg_get_template_paths(); |
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.
As already mentioned, accessing these files on each request is going to have a negative impact on performance. We can probably store this info once on theme activation and we'll also need it after theme updates.
// General | ||
'front-page': { | ||
title: 'Front page', | ||
description: '', |
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 could've added "Display the contents of your Front Page" here, or something along the similar lines.
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.
@david-szabo97 @vindl — Let's not forget i18n. Constants files like this should be i18n-ready from the start, otherwise getting everything ready for a testable Site Editor around WP 5.6 will be much more difficult.
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.
@mcsf My fault, I should have caught this in review!
I'll spin up a PR ASAP.
); | ||
|
||
return ( | ||
<NavigationMenu |
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 could've delayed adding this until we had actual content for it.
## Description Discovered while working on WordPress/wordpress-develop#1267. First introduced in `lib/template-loader.php` in #25739. No longer used per #26650 (where its callsites started using [`_gutenberg_get_template_paths`](https://github.com/WordPress/gutenberg/pull/26650/files#diff-f5b03c388f81fea69d0ababd289047e20deaad43084ad6e00ec14a5613e25136R60) in `lib/templates-sync.php` -- now in [`lib/full-site-editing/block-templates.php`](https://github.com/WordPress/gutenberg/blob/d7714aa8adf19277da1f0ea83b20be1cf234e50c/lib/full-site-editing/block-templates.php#L17)).
Description
A couple of things done in this PR to match the draft #23939 (comment))
Themes
Current Theme
groupCurrent Template
groupTemplatesMenu
)TemplatePartsMenu
)TemplatesPagesMenu
)TemplatesPostsMenu
)overflow:hidden
on theNavigation
component)How has this been tested?
yarn wp-env start
Screenshots
Types of changes
New feature
Checklist: