Respect uncontrolled inner blocks on Navigation block in editor and front of site #42182
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
getdave
changed the title
|
Size Change: +5 B (0%) Total Size: 1.27 MB
ℹ️ View Unchanged
|
getdave
requested review from
ntwb,
nerrad,
ajitbohra and
tellthemachines
as code owners
draganescu
approved these changes
Jul 6, 2022
5 tasks
|
@scruffian @MaggieCabrera Do we think we can ship this now or should we wait until we get the "Page List block as a fallback" working in the editor? |
|
@WordPress/block-themers do you have an opinion on this PR? It effects how themes that use a page list block in their Nav block within templates will behave. Description has full context. |
My opinion would be to ship this now as I think it's a good improvement to the block. We should highlight the implementation change in release notes, but I don't think it should wait until we have the same behaviour in the editor. |
getdave
force-pushed
the
update/respect-uncontrolled-inner-blocks-on-nav-block-in-editor
branch
from
2683652
to
ff5669f
Compare
|
TL;DR
Upon reflection I think we should wait on #42563 before merging this. The reason is that until we have Pages as a part of the Nav block's fallback states then Themes will want to use Page List in their templates in order to match what already happens on the front of the site. Once #42563 is done then the block will auto-handle displaying a list of Pages in the Editor as the Nav block's fallback state thereby removing the need to manually utilise a Page List as an inner block. @draganescu The issue we will have in using a Page List block as a fallback is that it will be an uncontrolled inner block. At which point thanks to this PR the block will always respect those inner blocks. This means that as a user you might insert a Nav block only to have a Page List foisted upon you for ever more. What we actually want is for the Page List block to act as a fallback/placeholder only. If the user doesn't manually interact with the Nav block then it [the Page List] should be discarded and the block still considered empty. |
This was referenced Jul 27, 2022
scruffian
force-pushed
the
update/respect-uncontrolled-inner-blocks-on-nav-block-in-editor
branch
from
ff5669f
to
4c52005
Compare
|
@draganescu what's the deal with this PR now #42735 is merged? |
|
I think this one can be closed and you to be a co-author of #42735 as that was based on this one /o'\ |
|
Closing in favour of #42735 |
|
I've suggested we write a dev note for WP 6.1 based on changes that were outlined in this PR which ultimately ended up in another PR. |
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?
This change means that Themes should not be including inner blocks in their Navigation blocks in their templates/parts. The only exception to this would be if they wanted to opt out of some of the current / future menu picking optimisations and automations provided by the block (e.g. carrying across menus on Theme switch).
This PR updates the Navigation block to prefer and preserve uncontrolled inner blocks over all other types of Navigation "content" that a Navigation block can display.
This means that if there are any uncontrolled inner blocks present the block will:
wp_navigation).refis present.Important: The major change with this approach is that if any themes use the Page List block as a pseudo "placeholder" in their Navigation block (as follows):
...then that Theme is effectively opting out of any current/future automatic picking of menus (e.g. on Theme switch).
For example, currently if there is a single
wp_navigationpost present on the site then the Nav block will automatically pick and use that menu when the Nav block is inserted. This is useful for scenarios such as Theme switching as the menu will effectively be automatically "carried over" to the new Theme without requiring user intervention. In the future we anticipate this functionality being extended and improved.Why?
In #38291 (comment) we identified that the block is currently exhibiting different behaviour between the Editor and the front of the site.
This PR normalizes the behaviour so that the block will always respect any uncontrolled inner blocks if they are present.
This allows use cases such as creating Navigation patterns in the editor.
How?
In the effect which attempts to automatically assign a Navigation Post to the block, we now test for uncontrolled inner blocks. If these are present then we abort the automated behaviour thereby allowing the uncontrolled blocks to be rendered.
Note that if the user makes any changes to the uncontrolled blocks to make them "dirty" then they will still be automatically converted into a
wp_navigationpost (existing behaviour).Testing Instructions
Basic Testing
wp_navigation) post. You can do this by creating an empty Navigation block and adding some items and saving in the multi-entity saving flow.wp_navigationpost (tip: gotowp-admin/edit.php?post_type=wp_navigation).<!-- wp:navigation --><!-- wp:page-list /--><!-- /wp:navigation -->.refattribute and that you can still see in the block markup for the inner blocks.Theme Switch testing
<!-- wp:page-list /-->.Screenshots or screencast
Screen.Capture.on.2022-07-06.at.12-12-12.mov