Unable to assign Menu location when creating the first menu

[dinamiko]

Replying to designsimply:

I also tested 32672.2.diff, but I spotted an error in the console and the first menu gets listed twice in the Menu Locations panel if you follow the testing steps exactly and click Save & Publish before going to Menu Locations (​screenshot).

@gabrielperezs and I took a look at this during WCEU's contributor day. Both patches apply without issue, but introduce a new bug with the Menu Locations.

We were not able to reproduce the duplicate menus in the Locations Panel, but we are noticing that the newly created menus will not appear in the Locations Panel, without a Save & Publish action. Otherwise, the menu remains empty.

Steps to reproduce.

1.) Apply 32672.2.diff​
2.) Add a new menu via the Customizer's Menu Panel.
3.) Select a menu location from the Menu Panel.
4.) Without saving, navigate to the Menu Locations Panel.

The newly created menu will not appear. This is presumably because the newly created menu is never saved.

[markoheijnen]

customize-nav-menus.js?ver=4.3-alpha-32280-src:1869 Uncaught TypeError: Cannot read property 'get' of undefined

[designsimply]

I did spot the error markoheijnen mentioned the first time I tried testing this, but I wasn't recording.

TypeError: api.section(...) is undefined1 customize-nav-menus.js:1583:11

Went back to record, tried testing two more times, and couldn't reproduce:

• ​59s - using the menu location dropdown
• ​1m1s - using the Primary Menu checkbox

^{Seen at whatisinfinity.com/wp-admin/customize.php tested with WordPress 4.3-alpha-32819 using Firefox 38.0.5 on Mac OSX 10.10.3.}

Aside: dinamiko, your screencasts are great! Thanks for including them.

@dinamiko and @markoheijnen, any chance you could update trunk and try it again?

[markoheijnen]

I was up-to-date but you need to delete all the menus first. Only then you will see the error.

[markoheijnen]

I deleted the if statement if menus are created so the locations do get added.

This is still a todo if a menu should be created or not. I could live either way. Since if the menu is empty, the customizer isn't saving it.

[valendesigns]

@markoheijnen I'll test your patch later tonight. Thanks!

[valendesigns]

Sorry for the delay! I've tested the patch and it does work, the only issue I found was that it allowed empty select options to be appended into the location dropdowns. I changed the JS a little to account for this in 32672.2.diff​.

[designsimply]

I was up-to-date but you need to delete all the menus first. Only then you will see the error.

Aha. Thanks!

I re-tested 32672.diff on WP 4.3-alpha-32975 and it worked for me: ​42s screencast.

I also tested 32672.2.diff, but I spotted an error in the console and the first menu gets listed twice in the Menu Locations panel if you follow the testing steps exactly and click Save & Publish before going to Menu Locations (​screenshot).

[stevenkword]

Replying to designsimply:

I also tested 32672.2.diff, but I spotted an error in the console and the first menu gets listed twice in the Menu Locations panel if you follow the testing steps exactly and click Save & Publish before going to Menu Locations (​screenshot).

@gabrielperezs and I took a look at this during WCEU's contributor day. Both patches apply without issue, but introduce a new bug with the Menu Locations.

We were not able to reproduce the duplicate menus in the Locations Panel, but we are noticing that the newly created menus will not appear in the Locations Panel, without a Save & Publish action. Otherwise, the menu remains empty.

Steps to reproduce.

1.) Apply 32672.2.diff​
2.) Add a new menu via the Customizer's Menu Panel.
3.) Select a menu location from the Menu Panel.
4.) Without saving, navigate to the Menu Locations Panel.

The newly created menu will not appear, presumably because the newly created menu is never saved.

We are only seeing this behavior in twentyfifteen and twentyeleven.

[westonruter]

Just took a quick look at the patch. Why the change to check for the name property being undefined as opposed to the setting being false? Note that false means the setting is marked for deletion.

[valendesigns]

Replying to westonruter:

Just took a quick look at the patch. Why the change to check for the name property being undefined as opposed to the setting being false? Note that false means the setting is marked for deletion.

Because there were situations when setting().name was undefined and setting() was not returning false. So only checking for false didn't stop the console error messages.

[valendesigns]

#32871 was marked as a duplicate.

[westonruter]

@valendesigns: When I tried 32672.2.diff​ I didn't see newly-added menus appearing in the nav menu locations' dropdowns because the menu option would only get added in that api.bind( 'add' ) call, whereas the api.bind( 'change' ) call is then just updating the existing option. When a new nav menu is added, it first gets added with an empty object {} and then right after gets updated to the the full nav menu object that is to be created; this ensures it is marked dirty. So the add callback fires first to inject the new option with the placeholder ID, and then when the change callback fires it supplies the name to the option.

I've restored the original false check in 32672.3.diff​ and I've added a different safeguard for ensuring a string is passed as the name. Nevertheless, I was not able to reproduce the JS error even without making sure displayNavMenuName was passed a string. Perhaps the JS error was happening before displayNavMenuName was introduced in [33071].

If anyone can reproduce the JS error now with this latest path applied to trunk, please share the details.

[westonruter]

Additional change: ​https://github.com/xwp/wordpress-develop/commit/9a4fbe8570237ff9c05242d0876244026b7fc018 All patches: ​https://github.com/xwp/wordpress-develop/pull/97

[westonruter]

Note that most of the changes in the patch are whitespace, as an if statement is removed. Patch without whitespace changes: ​https://github.com/xwp/wordpress-develop/pull/97/files?w=1

[valendesigns]

@westonruter Your updated patch solves the issues and there are no more errors. Without the patch I could not see the menu locations control and I would get this error in the console:

TypeError: navMenuLocationSetting is undefined
	element.set( navMenuLocationSetting.get() === control.params.menu_id );

line 1862: wp-admin/js/customize-nav-menus.js

[westonruter]

Fix was committed in [33094]. I made a typo by putting the wrong ticket number (#32858).