Skip to content
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

Remove any inner blocks from the navigation block #6163

Closed
5 tasks
mikachan opened this issue Jul 6, 2022 · 4 comments · Fixed by #6313
Closed
5 tasks

Remove any inner blocks from the navigation block #6163

mikachan opened this issue Jul 6, 2022 · 4 comments · Fixed by #6313
Labels
[Pri] High [Theme] Archeo Automatically generated label for Archeo. [Theme] Livro [Theme] Remote Automatically generated label for Remote. [Theme] Skatepark Automatically generated label for Skatepark. [Theme] Stewart

Comments

@mikachan
Copy link
Member

mikachan commented Jul 6, 2022

We should remove the page-list block (and any other inner blocks) as an inner block of the navigation block, prior to this GB PR being merged: WordPress/gutenberg#42182.

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 only applies to block themes that have inner blocks inside the navigation block. These are the ones I can find, but it's worth double-checking:

  • Archeo
  • Livro
  • Remote
  • Skatepark
  • Stewart
@mikachan mikachan added [Theme] Archeo Automatically generated label for Archeo. [Theme] Livro [Theme] Stewart [Theme] Skatepark Automatically generated label for Skatepark. [Theme] Remote Automatically generated label for Remote. labels Jul 6, 2022
@pbking
Copy link
Contributor

pbking commented Jul 8, 2022

Wouldn't doing this mean that any site that has these themes activated, but hasn't defined a custom menu (thus is showing the pages) means that their navigation would be wiped with this change?

Definitely agree no future themes should do this, but are we sure it's the right call to remove these for existing themes?

It's a tough tradeoff 'cause if we don't then activating these themes wouldn't have the best user experience.

@getdave
Copy link
Contributor

getdave commented Jul 11, 2022

Great point.

The front of the site will still render the pages as a fallback. With this change only the editor won't show anything.

However, in the (near) future we're proposing the Editor also automatically shows a list of pages as a fallback. So ultimately it will work out.

Do you think we should try and ship both those things together to avoid the problem you highlight?

@pbking
Copy link
Contributor

pbking commented Jul 12, 2022

Do you think we should try and ship both those things together to avoid the problem you highlight?

No, actually I hadn't realized that the view would continue to render pages as a fallback so that's good enough for me.

Thank you for clarifying!

@getdave
Copy link
Contributor

getdave commented Jul 13, 2022

And just to be clear, currently only the front of the site will continue to render pages as a fallback. We're yet to add that for the editor but it's a goal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Pri] High [Theme] Archeo Automatically generated label for Archeo. [Theme] Livro [Theme] Remote Automatically generated label for Remote. [Theme] Skatepark Automatically generated label for Skatepark. [Theme] Stewart
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants