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

Implement suitable fallback for Nav block on front end of site when no menu selected #36724

Merged
merged 10 commits into from
Nov 22, 2021

Conversation

getdave
Copy link
Contributor

@getdave getdave commented Nov 22, 2021

Description

In #36721 we learnt that the Nav block doesn't currently have a suitable fallback on the front end of the site when a menu is not selected.

This PR implements in the simplest way possible by falling back to rendering a core/page-list block with it's default attributes.

As Nav is already set up to handle displaying this block the styles all work perfectly out of the box.

if we want to limit the number of items shown then we'll need to modify core/page-list to have a maxPages prop so we can control the number of items. We might be able to provide this via context. I suggest we do this in a separate PR. Update: I added a commit which introduces a maxPages attribute to the Nav block. We can then pass this to the Page List block via context to control the number of items that are displayed.

Addresses part of #36721

How has this been tested?

Before testing use Fakerpress Plugin (or similar) to add a load of pages to your site. Some should be nested parent/child.

  1. Open Site Editor.
  2. Remove any existing Nav block.
  3. Add Nav block from scratch. Don't add any items!
  4. Save your empty Nav block and publish the post.
  5. Go to front of site.
  6. See Page List rendered by default even though no Menu has been selected.
  7. Return to Editor and add a menu for your nav block.
  8. Save and return to front of site.
  9. Check that your menu is rendered.

Screenshots

Screenshot 2021-11-22 at 13 16 37

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

@getdave getdave self-assigned this Nov 22, 2021
@getdave getdave added the [Block] Navigation Affects the Navigation Block label Nov 22, 2021
@getdave getdave marked this pull request as ready for review November 22, 2021 13:01
@getdave
Copy link
Contributor Author

getdave commented Nov 22, 2021

Some additional feedback here #36721 (comment)

@getdave
Copy link
Contributor Author

getdave commented Nov 22, 2021

As a followup I'm going to implement finding a Nav Menu. I have the code ready once this is merged.

Update: it's ready at #36740

@getdave getdave merged commit 7839432 into trunk Nov 22, 2021
@getdave getdave deleted the try/page-list-as-nav-block-front-end-fallback branch November 22, 2021 16:18
@github-actions github-actions bot added this to the Gutenberg 12.1 milestone Nov 22, 2021
@getdave getdave added the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Nov 23, 2021
@noisysocks noisysocks removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Nov 28, 2021
noisysocks pushed a commit that referenced this pull request Nov 29, 2021
…no menu selected (#36724)

* Use get_pages as block fallback

* Simplify by using page list block

* Output special class when rendering the fallback

* Remove requirement for parsing the block

* Fix linting

* Limit max number of pages to 4 via context API

* 2nd attempt to fix PHP linting

* Revert debugging

* Make attribute/context prop "unstable"

* More linting
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Affects the Navigation Block
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants