-
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
Add Navigation Menus to Template Parts screen sidebar in Browse Mode #51492
Add Navigation Menus to Template Parts screen sidebar in Browse Mode #51492
Conversation
packages/edit-site/src/components/sidebar-navigation-screen-navigation-menu/index.js
Outdated
Show resolved
Hide resolved
f7a9f24
to
827eeea
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Size Change: +921 B (0%) Total Size: 1.4 MB
ℹ️ View Unchanged
|
Trying to test this throws:
|
Flaky tests detected in 827eeea. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5269504492
|
827eeea
to
77b8389
Compare
I rebased the branch and I can build it 🤷 Did you check your Node version? |
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'm not sure if this is really a bug, I think that's the intended behaviour, although I do agree we should change it :) |
packages/edit-site/src/components/sidebar-navigation-screen-navigation-menu/index.js
Outdated
Show resolved
Hide resolved
packages/edit-site/src/components/sidebar-navigation-screen-navigation-menu/index.js
Show resolved
Hide resolved
packages/edit-site/src/components/sidebar-navigation-screen-template-part/index.js
Outdated
Show resolved
Hide resolved
...site/src/components/sidebar-navigation-screen-template-part/template-part-navigation-menu.js
Outdated
Show resolved
Hide resolved
...ite/src/components/sidebar-navigation-screen-template-part/template-part-navigation-menus.js
Outdated
Show resolved
Hide resolved
Yes, actually I agree. It's not a problem of this PR. |
...ite/src/components/sidebar-navigation-screen-template-part/template-part-navigation-menus.js
Outdated
Show resolved
Hide resolved
…mplate-part/template-part-navigation-menus.js
…mplate-part/template-part-navigation-menu.js
@@ -0,0 +1,3 @@ | |||
.edit-site-sidebar-navigation-screen-template-part-navigation-menu__title.components-heading { | |||
margin-bottom: $grid-unit-10; |
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.
@jasmussen it seems strange to have a different spacing for the heading component just on this one screen, do you think we should make this change to all heading components in browse mode? Maybe in a different PR?
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.
Yes, @jameskoster is still hashing out the metrics of the heading and subheading components, but they should all be the same. Good catch.
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.
Ok, I'll open a followup for this :)
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.
Done here: #51547
I have a report from @draganescu that this doesn't work with Archeo theme has been using. Needs investigating. |
* Restore sidebar in focus mode on Pattern click through in Browse Mode Library (#51897) Brings back #51492 * [Command center]: Add preferences and keyboard shortcuts commands (#51862) * [Command center]: Add preferences and keyboard shortcuts commands * update labels * [Site Editor]: Fix `library` command path (#51837) * Updating social link attributes (#51997) * Try: Update template titles (#51428) * Update template titles * Fix typo Co-authored-by: Alex Stine <[email protected]> * Revert Index rename * "front page" -> "homepage" * Update 404 Page make more sense given the template appears in the Pages panel too. * Front Page * home title + description * Revert Home name change, and move compat files * separate code for wp versions * update tests --------- Co-authored-by: Alex Stine <[email protected]> Co-authored-by: ntsekouras <[email protected]> * Update text color (#51965) * Modal: Add small top padding to the content so that avoid cutting off the visible outline when hovering items (#51829) * Site Editor: Fix focus cutoff in add template modal * Add padding-top to the modal content * Remove unnecessary padding-top * Remove unnecessary padding-top * Update changelog * Revert top padding from block patterns list * Revert top padding from block patterns list * Remove unnecessary changes * Remove unnecessary changes * Add CSS inline comment * Change padding metrics * Rest API: rename navigation fallback classes from WP_ to Gutenberg_ (#51959) * The `WP_REST_Navigation_Fallback_Controller` class has been committed to core and therefore results in a naming conflict and unit test failures. Ideally `WP_REST_Navigation_Fallback_Controller` should have been named `WP_REST_Navigation_Fallback_Controller_Gutenberg` and extended `WP_REST_Navigation_Fallback_Controller`. But we can conditionally load the file instead. * Renamed WP_Classic_To_Block_Menu_Converter to Gutenberg_Classic_To_Block_Menu_Converter Load WP_REST_Navigation_Fallback_Controller dependencies in load.php * Renamed all 6.3 classes to have the Gutenberg_ prefix. This should avoid compat errors and hopefully some confusion later. * Also rename test files for completeness * Updated deprecation notices to refer to Gutenberg classes * Fix phpunit failures (#51950) * Fix phpunit failures * Add comment * Update comment with actual reason this fix works --------- Co-authored-by: Dave Smith <[email protected]> Co-authored-by: Nik Tsekouras <[email protected]> Co-authored-by: Ramon <[email protected]> Co-authored-by: James Koster <[email protected]> Co-authored-by: Alex Stine <[email protected]> Co-authored-by: Aki Hamano <[email protected]> Co-authored-by: Robert Anderson <[email protected]>
…ordPress#51492) * Proof of concept * Rename function Addresses WordPress#51492 (comment) * Space docblocks * Update packages/edit-site/src/components/sidebar-navigation-screen-template-part/template-part-navigation-menus.js * Update packages/edit-site/src/components/sidebar-navigation-screen-template-part/template-part-navigation-menu.js * remove unneeded comment * Output a string when a navigation has no title --------- Co-authored-by: Ben Dwyer <[email protected]>
… Library (WordPress#51897) Brings back WordPress#51492
What?
Displays any Navigation Menus in the Template Part in the sidebar of the single Templates Part screen in the Browse Mode sidebar.
Closes #50704
Why?
Part of requirements from #50579.
When browsing a template part that contains Navigation block(s) such as the common
Header
template part, you want to be able to view or even edit any menus displayed there.How?
If there is a single menu then it is displayed directly inline as an editable tree.
If there are multiple menus then they are displayed as a list and you can click through to the Navigation block focus mode (not yet merged) to edit the menu.
Todos
Fix bug where clicking through to a single menu and then going back causes you to go to theOutside scope of this PR.Navigation
section rather than back to the Template Part.Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast
Screen.Capture.on.2023-06-14.at.16-41-51.mp4