fix: Allow navigation child blocks in contentOnly mode#74854
fix: Allow navigation child blocks in contentOnly mode#74854
Conversation
Allow all navigation child blocks to be inserted in the isolated navigation editor, even when in contentOnly mode. This fixes the regression where custom blocks with parent: ["core/navigation"] and blocks in navigation's allowedBlocks array were filtered out. Fixes #73826
|
Warning: Type of PR label mismatch To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.
Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task. |
| // Special case: Allow all navigation child blocks in contentOnly mode | ||
| if ( rootBlockName === 'core/navigation' ) { |
There was a problem hiding this comment.
The crux of this PR is that it's a temporary workaround that restores the original functionality.
The downside is that it might be masking problems as we move forward with developing contentOnly mode.
|
Size Change: +48 B (0%) Total Size: 3.09 MB
ℹ️ View Unchanged
|
|
I don't believe we should be modifying the contentOnly logic to accommodate a single block, when the cause of this issue is we're using contentOnly mode as a hack in the Nav instance of the isolated editor to avoid displaying its design tools, while still giving full access to all Nav child blocks. Nav is the only block that has its own isolated editor, so it's most definitely an edge case 😄 which I think should point to a custom solution for that isolated editor, instead of relying on contentOnly mode, which doesn't fulfil the requirements for the Nav isolated editor, and hacking it around to make things work for that case. I'd rather we removed contentOnly mode from the Nav isolated editor and instead disabled design tools for it. |
|
Thanks for proposing a fix. It's good to see some progress on this issue. I definitely understand this is a hard one to resolve. I think my main concern is that I believe this makes all of navigation's inner blocks exempt from contentOnly, even outside of the navigation editor. So a block that's not content would suddenly be considered content. It feels too broadly applied.
I think it's not only about that, but the navigation block shouldn't be removable. I think I pointed it out in the issue, the thing I don't fully understand is it that the Post Content block allows anything inserted into it (when editing a post using 'Show Template'), yet it's also set to That's an inconsistency that doesn't feel right to me, but maybe it also hints at a possible fix. |
|
Reading through the comment here - #73826 (comment), that gives me an idea. It seems like navigation (write) mode was removed, and then suddenly this contentOnly insertion behavior applied more widely than it previously did. I wonder if the fix might be to add a clause similar to the Previously write mode implied contentOnly behavior applied to all the blocks in the editor. Since it's removal the thing that now applies contentOnly behavior (with the pattern editing experiment active) is a section block, so perhaps the write mode condition could be replaced by Outside of sections the contentOnly insertion behavior wouldn't apply. |
|
With the 6.9.1 RC release approaching, I'm wondering how to address this issue.
I agree with this, but I don't have any idea how to implement it yet 😅 |
|
By the way, a PR to backport Gutenberg PRs to 6.9.1 is ready and should probably be merged tomorrow. #74806 |
|
Personally, I think that as a short term solution, it might be ok to merge this PR. |
If you're talking about merging to trunk, then I don't want that to happen, I think it would cause bugs with Pattern Editing. Perhaps merging only to the 6.9.1 branch might be ok. I think it'd need some testing to see what it might break, but there are not many usages of
I mentioned an option here - #74854 (comment). I personally think it would probably work ok. The only places that have The navigation editor doesn't have sections, so it'd go back to the previous behavior of allowing any block. |
Write mode was never taken out from behind the experiment flag, and it has since been deleted altogether. |
|
Ok, so it might be fine to merge this PR to 6.9.1 branch, but then consider a different fix for 7.0. |
I'm trying to implement this, but I haven't been involved in the recent development of contentOnly mode and haven't had much luck yet. |
|
I'm not sure if it will be ready in time for 6.9.1, but if anyone has the bandwidth, I'd appreciate it if you could try submitting a PR against the |
|
Since 6.9.1 RC1 has been released, let's explore the ideal approach for the 7.0 release. |
|
Sorry I've been very tight on bandwidth so haven't been able to push this forward. Shall we work backwards from the Nav block behaviour we want/expect?
Do we agree with this? Did I miss any nuance? If so then it seems the only things we need to add in the custom editor (assuming we adjust contentOnly mode as above) is:
|
In contentOnly mode we restrict available blocks to content ones. The Navigation block has non-content blocks such as Spacer, which I don't believe should be available in contentOnly mode. We limit this to make the editing experience simpler, and it's easy enough to switch into design mode if you want to insert other types of blocks. I believe consistency in behaviour will benefit users, particularly inexperienced ones, more than being able to insert Spacer blocks in the Nav. Apart from that, the other items on the list make sense! |
To add to what was mentioned above, users can't select or edit blocks that are not
I'm not sure what this one means, would you be able to clarify?
The 'Content' tab is also shown where a block supports it in contentOnly mode. There aren't many blocks that make use of it currently (except for when the Block Fields experiment is active).
My understanding is that the settings are important for nav link/submenu blocks, that tab shows the controls for setting text, link and description. Maybe you meant the nav block itself shouldn't show these things. |
What?
Fixes #73826
Allows all navigation child blocks to be inserted in the isolated navigation editor, even when the navigation block is in
contentOnlymode. This fixes a regression introduced in PR #72043 where custom blocks withparent: ["core/navigation"]and blocks in navigation'sallowedBlocksarray were incorrectly filtered out.Why?
When editing a navigation block in isolation (from Appearance > Editor > Navigation), the navigation block is set to
contentOnlymode to hide non-content controls. PR #72043 introducedcontentRolesupport, allowing blocks to mark themselves withsupports: { contentRole: true }to be insertable incontentOnlymode. However, this approach didn't consider existing third-party blocks that were already valid navigation children but would need to be updated to addcontentRolesupport.This is a regression that broke functionality that worked in WordPress 6.8, affecting custom blocks like Polylang's navigation language switcher and core blocks like Spacer and Login/out. Requiring third-party blocks to add
contentRolesupport would break backward compatibility and require plugin authors to update their blocks.How?
Modified
isContainerInsertableToInContentOnlyModeinpackages/block-editor/src/store/private-selectors.jsto add a special case for navigation blocks. When inserting into acore/navigationblock, the function now checks if the block being inserted is a valid navigation child by:parent: ["core/navigation"]in its block.jsonallowedBlocksarrayIf the block is a valid navigation child, it bypasses the
contentOnlyrestrictions and allows insertion. This preserves existingcontentOnlybehavior for other contexts while specifically addressing the navigation block case.Scope: This change is isolated to the navigation block insertion logic and doesn't affect other
contentOnlymode behavior, making it a safe and targeted fix that's easier to consider for merge.Testing Instructions
parent: ["core/navigation"]in its block.jsonallowedBlocks(like Spacer, Login/out) can also be inserted