nav-menu.js menu item added event

[welcher]

I am writing a plugin that adds a checkbox to each item in the admin Menu that is being edited. The checkbox is used to either bulk delete items in the menu or to insert a new one after it. Here is the plugin on github - ​https://github.com/ryanwelcher/Bulk-Remove-Menu-Items

Currently, I am overriding the addMenuToBottom() method in nav-menu.js to achieve the goal. I would love to see an event be triggered after the item is added. This seems like a much more efficient and elegant - not to mention less hacky - way of approaching this.

I have attached a patch with an initial approach that passes the markup of the item as a parameter. Any input is welcome.

[adamsilverstein]

Hey Ryan!

Thanks for the patch. Interesting idea for a plugin, the menu system certainly gets unwieldy when long (or always :)).

I think triggering an event on document is perfectly reasonable, although if you are going to reuse the $(menuMarkup) selector it should be cached. I also see this as another argument for standardizing Actions and Filters in JavaScript in core; until that bolder budges, triggering on document is the way to go.

[adamsilverstein]

cache $(menuMarkup)

[adamsilverstein]

In 31218.2.diff​:

• cache the $(menuMarkup) selector

[welcher]

Thanks for the suggestions and patch edits @adamsilverstein.

I also wanted to add this changeset from the plugin I am working on that demonstrates the differences based on the availability of an event:

​https://github.com/ryanwelcher/Bulk-Remove-Menu-Items/compare/core-event

[welcher]

Adding an .gif to demonstrate how my plugin responds to the event being fired. The checkboxes are added by the plugin and when checked, the new menu item is moved below the checked item after being added to the menu.

[welcher]

Updated JSDOCS

[azaozz]

Agree with @adamsilverstein, don't see a problem adding another "global" event. The thing is when this code changes these events will most likely go, i.e. it's not possible to have backwards-compatibility similar to PHP's actions and filters.

[welcher]

Updating patch to add a similar event to the addMenuItemToTop() method as per @azaozz's comments in Slack.

[adamsilverstein]

31218_5.diff​: slight code standards cleanup (whitespace!).

@welcher - Thanks for the JS docs, now seems sorely lacking from the addMenuItemToTop function, can you add that? :)

[welcher]

Add JSDocks to addMenuToTop

[welcher]

Replying to adamsilverstein:

31218_5.diff​: slight code standards cleanup (whitespace!).

@welcher - Thanks for the JS docs, now seems sorely lacking from the addMenuItemToTop function, can you add that? :)

Patch updated. I also remove an extraneous parameter in the JS docs that didn't exists in either method declaration and indicated how each of the method adds the new menu item.

[welcher]

Changing the name of the event being triggered

[welcher]

I have changed the name of the event being triggered to match the menu-customizer plugin. If this is committed into core now then any plugins/themes that responding to the event can still do so once it the admin page is replaced by the customizer.

[welcher]

Updated JSDOCS to reflect event name change

[welcher]

No movement and extremely out of date. Closing.

[welcher]

Thought about it and I think this is still valid. Patch applies still.

[welcher]

@chriscct7 let me know if I can do anything further to move this along. Open to suggestions as well.

[jorbin]

No momentum pushing this over the line, so it's going to miss 4.5.

[meatloaf1024]

Is there any movement on this? I am creating a little plugin that could really benefit from this functionality.

[adamsilverstein]

@welcher in 31218.5.diff​ I updated the patch with another event right before items are removed from the menu, again passing a jQuery object handle to the menu DOM element itself. @jorbin pointed out this might be useful and I agree.

I took the brackets out of the data passed in your trigger calls as it was passing the element as a single element array.

Can you review and see if this still works for your use case?

[welcher]

@adamsilverstein tested the patch against my plugin and other than a minor tweak to get it working - I had to wrap the menuMarkup in $() - it works exactly as expected in my plugin.

Thanks!

[adamsilverstein]

31218.6.diff​

• Add some QUnit tests for the expected event triggers

[adamsilverstein]

31218.7.diff​:

• QUnit tests: fail if the events aren't triggered

[adamsilverstein]

@welcher can you try the unit tests and verify they make sense and work correctly for you?

thanks.

[welcher]

@adamsilverstein I ran the tests locally and ran into errors. Specifically:

Uncaught TypeError: assert.done is not a function@ 122 ms
Source:
​http://localhost:63342/src/tests/qunit/wp-admin/js/nav-menu.js:8

and then on subsequent loads:

Uncaught TypeError: Cannot read property 'equal' of undefined@ 182 ms
Source:
​http://localhost:63342/src/tests/qunit/wp-admin/js/nav-menu.js:7

This is my first time running the QUnit sweet so it may be my setup. I am running them in the browser.

[adamsilverstein]

31218.8.diff​ corrects the issue you were seeing @welcher, can you try again please?

[adamsilverstein]

31218.9.diff​:

• Cleanup
• Increase timeout to 1500 ms to ensure events have a chance to fire

[adamsilverstein]

In 31218.10.diff​:

• Switch the failure callback to assert that the expected number of tests had completed. No longer tries to clear the timeout.

With the patch, all tests pass:

Without the patch, the expected events don't fire and the fail check fails:

[welcher]

I just ran the tests locally and everything seems a-ok.

[adamsilverstein]

In 39928:

Menus: trigger an event when menu items are added or removed.

Fire a menu-item-added event after a menu item is added to the DOM. Fire a menu-removing-item event before a menu item is removed from the DOM. Enables hooking into and responding to menu changes.

Props welcher, adamsilverstein.
Fixes #31218.

[adamsilverstein]

Reopening because we are seeing some intermittent test failures in Travis -

Expected 3 assertions, but 5 were run - looks like this is happening because the delayed test that verifies all hooks have fired is running too early.

[adamsilverstein]

In 31218.12.diff​ increase the timeout for the wpNavMenu Qunit test to 3 seconds

[adamsilverstein]

In 41675:

Menus: Increase timeout for QUnit wpNavMenu trigger tests.

Fix an (intermittent) issue where the final assertion verifying all hooks have been triggered ran too early, resulting in the test failing.

Fixes #31218.

[harishaas]

You can manually trigger a custom JavaScript event after you've added your menu item. You would need to modify your plugin to dispatch this event after adding the item, and then you can listen for this event in your custom JavaScript code. Here's an example:

javascript

// Inside your plugin code after adding the menu item
var event = new Event('menu-item-added');
document.dispatchEvent(event);

In your custom JavaScript code, you can listen for this event:

javascript

document.addEventListener('menu-item-added', function (event) {
    // Handle the event here
});

Mutation Observer: You can use a MutationObserver to watch for changes in the DOM and react when a new menu item is added. This approach is more generic and doesn't rely on custom events. Here's a basic example:

javascript

var menuList = document.querySelector('.menu');

var observer = new MutationObserver(function (mutationsList) {
    for (var mutation of mutationsList) {
        if (mutation.type === 'childList') {
            // A child node (menu item) was added
            // Handle the added menu item here
        }
    }
});

observer.observe(menuList, { childList: true });
This code will trigger whenever a new menu item is added to the menu list, allowing you to react to the addition.