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

Nav block: enable creation from existing WP Menus #18869

Merged
merged 67 commits into from
Jun 10, 2020

Conversation

getdave
Copy link
Contributor

@getdave getdave commented Dec 2, 2019

Screenshot 2020-05-29 at 15 40 38

This PR allow the creation of Nav Block (items) from existing WP Menus. Requires the experimental Menu REST API Plugin. The intent is that this endpoint gets merged into WordPress Core thereby enabling this functionality on the Block to become a reality. (update: the endpoints are now merged into Gutenberg Core).

This is visually based on this mockup from @shaunandrews.

Note: the creation of the Nav Block from the Menu is a one time action. It does not keep the Nav Block items in sync with the Menu.

Depends on #20292
Closes #18575

Todo

The following items are outstanding on this PR:

  • Update e2e tests to match new design.
  • Fix or workaround Axe accessibility tests errors in component dependencies.
  • Fix or work around React state update on an unmounted component relating to ToolbarContainer component (again not modified in this PR so unrelated???).
  • Fix CSS logic to ensure the dividing line is always below the last "Menu" item. We cannot rely on there always being a "Create from pages" option. See https://d.pr/i/qfvMbT.

Testing Instructions

If you notice any "lag" on inserting/selecting the Block you are probably experiencing #22823.

Does this match the approved design?

Pre-test setup

  • Remove all Menus (Appearance -> Menus) from your WP site.
  • Switch to this branch. Rebuild. Any issues with Prettier please see here).

Test Flow

  • Create a New Post

  • Add Nav Block

  • You should not see any Menu options in the placeholder dropdown as there are no menus.

  • Goto Apperance -> Menus and create a new Menu but do not add any items to it.

  • Create a New Post

  • Add Nav Block

  • You should see the Menu you created within the dropdown. Select the Menu from the dropdown and click the "Create" button.

  • You should see an empty Nav Block created with no Nav Item Blocks - this reflects the empty state of your WP Menu.

  • Goto Appearance -> Menus and add some items to your Menu (be sure to hit "Save" to save your Menu else it will still have 0 items!).

  • Create a New Post

  • Add Nav Block

  • You should still see the Menu you created within the dropdown. Select the Menu from the dropdown and click the "Create" button.

  • You should see the Nav Block created with all of the items that exist in your WP Menu.

  • Test that you can still create from Pages and create an empty Nav block.

  • Also try the tests above on a limited network connection (simulate using Devtools "Network" tab).

How has this been tested?

  • Manual testing
  • e2e tests

Screenshots

Screen Capture on 2020-05-29 at 15-40-12

Types of changes

New feature (non-breaking change which adds functionality)

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • 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. .

@getdave getdave self-assigned this Dec 2, 2019
@WunderBart
Copy link
Member

Wanted to test this one, but I'm getting an error when running npm run dev (after a clean npm install):

Build Progress: [==============================] 100%
[1] 
[1] > [email protected] dev:packages /Users/bart/Code/WunderBart/gutenberg
[1] > node ./bin/packages/watch.js
[1] 
[1] internal/modules/cjs/loader.js:800
[1]     throw err;
[1]     ^
[1] 
[1] Error: Cannot find module '/Users/bart/Code/WunderBart/gutenberg/packages/edit-site/package.json'
[1] Require stack:
[1] - /Users/bart/Code/WunderBart/gutenberg/bin/packages/get-packages.js
[1] - /Users/bart/Code/WunderBart/gutenberg/bin/packages/watch.js
[1]     at Function.Module._resolveFilename (internal/modules/cjs/loader.js:797:15)
[1]     at Function.Module._load (internal/modules/cjs/loader.js:690:27)
[1]     at Module.require (internal/modules/cjs/loader.js:852:19)
[1]     at require (internal/modules/cjs/helpers.js:74:18)
[1]     at hasModuleField (/Users/bart/Code/WunderBart/gutenberg/bin/packages/get-packages.js:35:21)
[1]     at apply (/Users/bart/Code/WunderBart/gutenberg/node_modules/lodash/lodash.js:476:27)
[1]     at /Users/bart/Code/WunderBart/gutenberg/node_modules/lodash/lodash.js:5259:20
[1]     at arrayEvery (/Users/bart/Code/WunderBart/gutenberg/node_modules/lodash/lodash.js:558:12)
[1]     at /Users/bart/Code/WunderBart/gutenberg/node_modules/lodash/lodash.js:5258:18
[1]     at apply (/Users/bart/Code/WunderBart/gutenberg/node_modules/lodash/lodash.js:474:27) {
[1]   code: 'MODULE_NOT_FOUND',
[1]   requireStack: [
[1]     '/Users/bart/Code/WunderBart/gutenberg/bin/packages/get-packages.js',
[1]     '/Users/bart/Code/WunderBart/gutenberg/bin/packages/watch.js'
[1]   ]
[1] }
[1] npm 
[1] ERR! code ELIFECYCLE
[1] npm ERR! errno 1
[1] npm 
[1] ERR! [email protected] dev:packages: `node ./bin/packages/watch.js`
[1] npm ERR! Exit status 1
[1] npm ERR! 
[1] npm ERR! Failed at the [email protected] dev:packages script.
[1] npm 
[1] ERR! This is probably not a problem with npm. There is likely additional logging output above.

Master branch is working fine for me.

Env:

  • node v12.13.1 (from .nvmrc)
  • npm v6.12.1

@getdave
Copy link
Contributor Author

getdave commented Jan 9, 2020

@marekhrabe and I are going to take this up full time again starting tomorrow.

@getdave getdave force-pushed the try/nav-block-create-from-menus branch from 2a8c8f2 to 2af97da Compare January 9, 2020 14:25
@getdave getdave added [Block] Navigation Affects the Navigation Block [Status] In Progress Tracking issues with work in progress labels Jan 9, 2020
@getdave
Copy link
Contributor Author

getdave commented Jan 9, 2020

@youknowriad I think this feature is important for continuity with WP Core. However, we cannot land it until the REST API supports Menus.

However, the REST API team already have a feature Plugin for Menu endpoints that needs testing. We can develop against this Plugin.

However, in order to merge the feature we'll need to wait on WP Core to release the Menu endpoints. That could take some time.

I felt it was worth asking - is there any way we would consider adding an "experimental" endpoint to Gutenberg to allow READ access to Menus as a stop-gap until the WP Core REST API is updated with an official endpoint?

cc @apeatling

@youknowriad
Copy link
Contributor

@getdave this seems like a good use case to bring the required endpoints from that plugin to Gutenberg and test it properly.

@spacedmonkey
Copy link
Member

@youknowriad The menus endpoint is being activily developed in the plugin still. We still have one PR that is outstand. We then need to have unit tests and tidy up the code. So to me, this still makes sense for this to live in it's own plugin for now.

If you want to use it in your code base, you could bring it in using composer?

@youknowriad
Copy link
Contributor

If you want to use it in your code base, you could bring it in using composer?

I don't mind using composer here personally, The concern is the extra tooling but we can probably try it.

@spacedmonkey
Copy link
Member

I have created a PR for menu endpoints - #20292

@draganescu draganescu force-pushed the try/nav-block-create-from-menus branch from b1ff735 to 451d9f6 Compare March 4, 2020 19:12
@github-actions
Copy link

github-actions bot commented Mar 4, 2020

Size Change: +2.07 kB (0%)

Total Size: 1.13 MB

Filename Size Change
build/block-library/index.js 129 kB +1.6 kB (1%)
build/block-library/style-rtl.css 7.96 kB +238 B (2%)
build/block-library/style.css 7.96 kB +237 B (2%)
build/components/style-rtl.css 19.5 kB -7 B (0%)
build/components/style.css 19.5 kB -4 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.62 kB 0 B
build/api-fetch/index.js 3.4 kB 0 B
build/autop/index.js 2.83 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.77 kB 0 B
build/block-directory/style-rtl.css 892 B 0 B
build/block-directory/style.css 892 B 0 B
build/block-editor/index.js 106 kB 0 B
build/block-editor/style-rtl.css 11.8 kB 0 B
build/block-editor/style.css 11.8 kB 0 B
build/block-library/editor-rtl.css 7.88 kB 0 B
build/block-library/editor.css 7.89 kB 0 B
build/block-library/theme-rtl.css 684 B 0 B
build/block-library/theme.css 686 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 48.1 kB 0 B
build/components/index.js 195 kB 0 B
build/compose/index.js 9.31 kB 0 B
build/core-data/index.js 11.4 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.44 kB 0 B
build/date/index.js 5.47 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 3.17 kB 0 B
build/edit-navigation/index.js 8.24 kB 0 B
build/edit-navigation/style-rtl.css 918 B 0 B
build/edit-navigation/style.css 919 B 0 B
build/edit-post/index.js 303 kB 0 B
build/edit-post/style-rtl.css 5.6 kB 0 B
build/edit-post/style.css 5.6 kB 0 B
build/edit-site/index.js 16.6 kB 0 B
build/edit-site/style-rtl.css 2.96 kB 0 B
build/edit-site/style.css 2.96 kB 0 B
build/edit-widgets/index.js 9.34 kB 0 B
build/edit-widgets/style-rtl.css 2.4 kB 0 B
build/edit-widgets/style.css 2.4 kB 0 B
build/editor/editor-styles-rtl.css 425 B 0 B
build/editor/editor-styles.css 428 B 0 B
build/editor/index.js 44.8 kB 0 B
build/editor/style-rtl.css 4.26 kB 0 B
build/editor/style.css 4.27 kB 0 B
build/element/index.js 4.64 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.72 kB 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.51 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.12 kB 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/media-utils/index.js 5.29 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 kB 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.5 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 14.8 kB 0 B
build/server-side-render/index.js 2.68 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.85 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@draganescu draganescu force-pushed the try/nav-block-create-from-menus branch from 451d9f6 to 7c5d23c Compare March 9, 2020 07:47
@draganescu draganescu assigned draganescu and unassigned marekhrabe and getdave Mar 9, 2020
@draganescu draganescu marked this pull request as ready for review March 9, 2020 16:12
getdave and others added 19 commits June 10, 2020 14:21
Due to loading spinner we now have to wait on the dropdown before interacting.
This fix will work once a complementary update has been applied to CustomSelectControl to enable the options to have a custom classname applied.
@getdave getdave force-pushed the try/nav-block-create-from-menus branch from 4a766b2 to 792937d Compare June 10, 2020 13:23
@getdave
Copy link
Contributor Author

getdave commented Jun 10, 2020

Thanks for working on this @getdave.

You are very welcome. Thanks for all the reviews and help. Also @draganescu (and others).

I still feel like this is a lot of code to add to the one file (navigation/edit.js), but having said that, the feature is working well.

Agreed. For clarity for others coming here in the future, the feature has now been:

This makes me confident it will serve the need although there are certainly some UX and a11y improvements that could be looked at. As a result I've raised a number of followup issues that have been added to the Navigation project board.

I'll look to merge this asap.

@getdave getdave merged commit 7567111 into master Jun 10, 2020
@getdave getdave deleted the try/nav-block-create-from-menus branch June 10, 2020 14:06
@github-actions github-actions bot added this to the Gutenberg 8.4 milestone Jun 10, 2020
@paaljoachim
Copy link
Contributor

Great work Dave!!

@collimarco
Copy link

This feature seems merged, but in the latest Wordpress version I can't find any Nav block...

Why? How is it called?

@paaljoachim
Copy link
Contributor

paaljoachim commented Aug 20, 2021

Hey @collimarco Marco

The Nav block is for the moment only present in the Gutenberg plugin.
Here is it with the Gutenberg plugin activated:
Screen Shot 2021-08-20 at 15 34 10

It is missing when the Gutenberg plugin is not activated.
Screen Shot 2021-08-20 at 15 34 57

The Nav block was not included into WordPress 5.8 because it is still in a kind of experimental phase. Which means it is for now only included in the Gutenberg plugin. It should likely be included into WordPress 5.9 when it is released in December.

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 Needs Copy Review Needs review of user-facing copy (language, phrasing) [Status] In Progress Tracking issues with work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Nav Block - create from existing Menus