From 2149ed6edae59783eda9c787c2f6f49f991f2ee4 Mon Sep 17 00:00:00 2001 From: Kamran Abdul Aziz Date: Tue, 26 May 2026 20:22:31 +0530 Subject: [PATCH 1/3] Fix menu item selection across pagination --- src/js/_enqueues/lib/nav-menu.js | 124 ++++++++++++++++++++++++++++++- 1 file changed, 120 insertions(+), 4 deletions(-) diff --git a/src/js/_enqueues/lib/nav-menu.js b/src/js/_enqueues/lib/nav-menu.js index 79917c8447f1a..a8c8993fdb65c 100644 --- a/src/js/_enqueues/lib/nav-menu.js +++ b/src/js/_enqueues/lib/nav-menu.js @@ -35,6 +35,7 @@ isRTL: !! ( 'undefined' != typeof isRtl && isRtl ), negateIfRTL: ( 'undefined' != typeof isRtl && isRtl ) ? -1 : 1, lastSearch: '', + selectedMenuItems: {}, // Functions that run on init. init : function() { @@ -191,9 +192,11 @@ re = /menu-item\[([^\]]*)/; processMethod = processMethod || api.addMenuItemToBottom; + api.syncSelectedMenuItems( t ); + menuItems = $.extend( {}, api.getSelectedMenuItems( t ) ); // If no items are checked, bail. - if ( !checkboxes.length ) + if ( !checkboxes.length && $.isEmptyObject( menuItems ) ) return false; // Show the Ajax spinner. @@ -202,18 +205,21 @@ // Retrieve menu item data. $(checkboxes).each(function(){ var t = $(this), + menuItemData, listItemDBIDMatch = re.exec( t.attr('name') ), listItemDBID = 'undefined' == typeof listItemDBIDMatch[1] ? 0 : parseInt(listItemDBIDMatch[1], 10); if ( this.className && -1 != this.className.indexOf('add-to-top') ) processMethod = api.addMenuItemToTop; - menuItems[listItemDBID] = t.closest('li').getItemData( 'add-menu-item', listItemDBID ); + menuItemData = t.closest('li').getItemData( 'add-menu-item', listItemDBID ); + menuItems[api.getSelectedMenuItemKey( menuItemData, listItemDBID )] = menuItemData; }); // Add the items. api.addItemToMenu(menuItems, processMethod, function(){ // Deselect the items and hide the Ajax spinner. checkboxes.prop( 'checked', false ); + api.clearSelectedMenuItems( t ); t.find( '.button-controls .select-all' ).prop( 'checked', false ); t.find( '.button-controls .spinner' ).removeClass( 'is-active' ); t.updateParentDropdown(); @@ -409,6 +415,106 @@ }); }, + getSelectedMenuItemsKey : function( metabox ) { + return metabox.attr( 'id' ) || metabox.closest( '.postbox' ).attr( 'id' ) || ''; + }, + + getMenuItemCheckboxId : function( checkbox ) { + var listItemDBIDMatch, + re = /menu-item\[([^\]]*)/; + + listItemDBIDMatch = re.exec( checkbox.attr( 'name' ) ); + + if ( ! listItemDBIDMatch ) { + return 0; + } + + return 'undefined' == typeof listItemDBIDMatch[1] ? 0 : parseInt( listItemDBIDMatch[1], 10 ); + }, + + updateSelectedMenuItem : function( checkbox ) { + var listItemDBID, menuItemData, + metabox = checkbox.closest( '.posttypediv, .taxonomydiv' ), + key = api.getSelectedMenuItemsKey( metabox ); + + if ( ! key ) { + return; + } + + listItemDBID = api.getMenuItemCheckboxId( checkbox ); + + if ( ! listItemDBID ) { + return; + } + + api.selectedMenuItems[ key ] = api.selectedMenuItems[ key ] || {}; + menuItemData = checkbox.closest( 'li' ).getItemData( 'add-menu-item', listItemDBID ); + + if ( checkbox.prop( 'checked' ) ) { + api.selectedMenuItems[ key ][ api.getSelectedMenuItemKey( menuItemData, listItemDBID ) ] = menuItemData; + } else { + delete api.selectedMenuItems[ key ][ api.getSelectedMenuItemKey( menuItemData, listItemDBID ) ]; + } + }, + + getSelectedMenuItemKey : function( menuItemData, listItemDBID ) { + var key; + + if ( menuItemData['menu-item-object-id'] || menuItemData['menu-item-url'] ) { + key = [ + menuItemData['menu-item-type'] || '', + menuItemData['menu-item-object'] || '', + menuItemData['menu-item-object-id'] || '', + 'custom' === menuItemData['menu-item-type'] ? menuItemData['menu-item-url'] || '' : '', + 'custom' === menuItemData['menu-item-type'] ? menuItemData['menu-item-title'] || '' : '' + ].join( ':' ); + + return key.replace( /[^\w-]/g, '_' ); + } + + return listItemDBID || ''; + }, + + syncSelectedMenuItems : function( metabox ) { + metabox.find( '.tabs-panel-active .categorychecklist li input.menu-item-checkbox' ).each(function() { + api.updateSelectedMenuItem( $( this ) ); + }); + }, + + getSelectedMenuItems : function( metabox ) { + var key = api.getSelectedMenuItemsKey( metabox ); + + if ( ! key || ! api.selectedMenuItems[ key ] ) { + return {}; + } + + return api.selectedMenuItems[ key ]; + }, + + clearSelectedMenuItems : function( metabox ) { + var key = api.getSelectedMenuItemsKey( metabox ); + + if ( key ) { + delete api.selectedMenuItems[ key ]; + } + }, + + restoreSelectedMenuItems : function( metabox ) { + var selectedItems = api.getSelectedMenuItems( metabox ); + + metabox.find( '.categorychecklist li input.menu-item-checkbox' ).each(function() { + var checkbox = $( this ), + menuItemData, + listItemDBID = api.getMenuItemCheckboxId( checkbox ); + + menuItemData = checkbox.closest( 'li' ).getItemData( 'add-menu-item', listItemDBID ); + + if ( selectedItems[ api.getSelectedMenuItemKey( menuItemData, listItemDBID ) ] ) { + checkbox.prop( 'checked', true ); + } + }); + }, + countMenuItems : function( depth ) { return $( '.menu-item-depth-' + depth ).length; }, @@ -1597,6 +1703,7 @@ // Upon changing tabs, we want to uncheck all checkboxes. $( 'input', wrapper ).prop( 'checked', false ); + api.clearSelectedMenuItems( target.closest( '.posttypediv, .taxonomydiv' ) ); $('.tabs-panel-active', wrapper).removeClass('tabs-panel-active').addClass('tabs-panel-inactive'); $('#' + panelId, wrapper).removeClass('tabs-panel-inactive').addClass('tabs-panel-active'); @@ -1625,8 +1732,11 @@ } else if ( target.is( ':checked' ) ) { items.prop( 'checked', true ); } + + api.syncSelectedMenuItems( $( '#' + selectAreaMatch ) ); } } else if ( target.hasClass( 'menu-item-checkbox' ) ) { + api.updateSelectedMenuItem( target ); selectAreaMatch = target.closest( '.tabs-panel-active' ).parent().attr( 'id' ); if ( selectAreaMatch ) { items = $( '#' + selectAreaMatch + ' .tabs-panel-active .menu-item-title input' ); @@ -1654,12 +1764,16 @@ * links thus excluding the current page ``. See ticket #35577. */ $( '#nav-menu-meta' ).on( 'click', 'a.page-numbers', function() { - var $container = $( this ).closest( '.inside' ); + var $container = $( this ).closest( '.inside' ), + $metabox = $( this ).closest( '.posttypediv, .taxonomydiv' ); + + api.syncSelectedMenuItems( $metabox ); $.post( ajaxurl, this.href.replace( /.*\?/, '' ).replace( /action=([^&]*)/, '' ) + '&action=menu-get-metabox', function( resp ) { var metaBoxData = JSON.parse( resp ), - toReplace; + toReplace, + $updatedMetabox; if ( -1 === resp.indexOf( 'replace-id' ) ) { return; @@ -1674,6 +1788,8 @@ // Update the post type menu meta box with new content from the response. $container.html( metaBoxData.markup ); + $updatedMetabox = $( document.getElementById( metaBoxData['replace-id'] ) ); + api.restoreSelectedMenuItems( $updatedMetabox ); } ); From 3f2cfe04440573f12d21cc6642f184d78101a59f Mon Sep 17 00:00:00 2001 From: Kamran Abdul Aziz Date: Fri, 7 Aug 2026 14:40:05 +0530 Subject: [PATCH 2/3] Add QUnit tests for menu item selection cache --- tests/qunit/wp-admin/js/nav-menu.js | 439 ++++++++++++++++++++++++++++ 1 file changed, 439 insertions(+) diff --git a/tests/qunit/wp-admin/js/nav-menu.js b/tests/qunit/wp-admin/js/nav-menu.js index c630d078297de..7d35e3f889504 100644 --- a/tests/qunit/wp-admin/js/nav-menu.js +++ b/tests/qunit/wp-admin/js/nav-menu.js @@ -94,3 +94,442 @@ } )( window.QUnit, jQuery ); + +/*global wpNavMenu */ +( function( QUnit, $ ) { + var api = window.wpNavMenu, + originalPost, + placeholder, + fixture; + + /** + * Returns the next negative placeholder ID. + * + * Walker_Nav_Menu_Checklist indexes every field name with the global + * $_nav_menu_placeholder, which restarts at -1 on each request. Reloading + * a checklist therefore renders the same item under a different ID. + * + * @return {number} The next placeholder. + */ + function nextPlaceholder() { + placeholder = placeholder - 1; + return placeholder; + } + + /** + * Starts a fresh placeholder run, as a new request would. + */ + function newRequest() { + placeholder = 0; + } + + /** + * Builds a single checklist item. + * + * Field names are indexed by the placeholder while the checkbox value + * carries the real object ID, matching + * Walker_Nav_Menu_Checklist::start_el(). + * + * @param {number} objectId Value of the object ID field. + * @param {Object} options Optional title, object, type, url, checked. + * @return {string} The `li` markup. + */ + function checklistItem( objectId, options ) { + options = options || {}; + + var name = 'menu-item[' + nextPlaceholder() + ']', + title = options.title || ( 'Item ' + objectId ), + object = 'undefined' === typeof options.object ? 'page' : options.object, + type = options.type || 'post_type', + url = options.url || ( 'https://example.org/?p=' + objectId ), + checked = options.checked ? ' checked="checked"' : ''; + + return '
  • ' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
    • '; + } + + /** + * Wraps items in a meta box with an active and an inactive tab panel. + * + * @param {string} id Meta box ID. + * @param {string} className `posttypediv` or `taxonomydiv`. + * @param {string[]} activeItems Items for the active panel. + * @param {string[]} inactiveItems Items for the inactive panel. + * @return {string} The meta box markup. + */ + function metabox( id, className, activeItems, inactiveItems ) { + return '
    ' + + '
    ' + + '
      ' + activeItems.join( '' ) + '
    ' + + '
    ' + + '
    ' + + '
      ' + ( inactiveItems || [] ).join( '' ) + '
    ' + + '
    ' + + '

    ' + + '' + + '' + + '

    ' + + '2' + + '
    '; + } + + /** + * Renders the meta boxes inside the menus screen containers the event + * handlers are delegated from. + * + * @param {string} metaboxMarkup One or more meta boxes. + */ + function renderScreen( metaboxMarkup ) { + fixture.html( + '' + + '' + + '' + + '' + ); + + api.attachTabsPanelListeners(); + } + + /** + * Counts the cached selections for a meta box. + * + * @param {jQuery} metaboxEl Meta box element. + * @return {number} Number of cached items. + */ + function countSelected( metaboxEl ) { + return Object.keys( api.getSelectedMenuItems( metaboxEl ) ).length; + } + + /** + * Finds a checklist checkbox by its object ID value. + * + * @param {jQuery} context Element to search within. + * @param {number} objectId Object ID. + * @return {jQuery} The checkbox. + */ + function checkboxFor( context, objectId ) { + return context.find( 'input.menu-item-checkbox[value="' + objectId + '"]' ); + } + + QUnit.module( 'nav-menu: menu item selections across pagination', { + beforeEach: function() { + /* + * getItemData() is registered by wpNavMenu.jQueryExtensions(), + * which normally runs from wpNavMenu.init() on the menus screen. + */ + api.jQueryExtensions(); + api.selectedMenuItems = {}; + fixture = $( '#qunit-fixture' ); + newRequest(); + + window.ajaxurl = window.ajaxurl || '/wp-admin/admin-ajax.php'; + originalPost = $.post; + }, + afterEach: function() { + $.post = originalPost; + api.selectedMenuItems = {}; + } + } ); + + QUnit.test( 'getMenuItemCheckboxId() reads the placeholder out of the field name.', function( assert ) { + renderScreen( metabox( 'posttype-page', 'posttypediv', [ checklistItem( 42 ) ] ) ); + + assert.strictEqual( + api.getMenuItemCheckboxId( fixture.find( 'input.menu-item-checkbox' ) ), + -1, + 'The first row is indexed by placeholder -1, not by its object ID.' + ); + } ); + + QUnit.test( 'getMenuItemCheckboxId() returns 0 for an unrelated field.', function( assert ) { + fixture.html( '' ); + + assert.strictEqual( + api.getMenuItemCheckboxId( fixture.find( 'input' ) ), + 0, + 'A name that is not a menu item field yields 0.' + ); + } ); + + QUnit.test( 'getSelectedMenuItemsKey() uses the meta box ID.', function( assert ) { + renderScreen( metabox( 'posttype-page', 'posttypediv', [ checklistItem( 1 ) ] ) ); + + assert.strictEqual( + api.getSelectedMenuItemsKey( fixture.find( '#posttype-page' ) ), + 'posttype-page', + 'Each meta box gets its own cache key.' + ); + } ); + + QUnit.test( 'getSelectedMenuItemsKey() falls back to the enclosing postbox ID.', function( assert ) { + fixture.html( '
    ' ); + + assert.strictEqual( + api.getSelectedMenuItemsKey( fixture.find( '.posttypediv' ) ), + 'add-page', + 'A meta box without its own ID uses the postbox ID.' + ); + } ); + + QUnit.test( 'getSelectedMenuItemsKey() returns an empty string when there is no meta box.', function( assert ) { + assert.strictEqual( + api.getSelectedMenuItemsKey( $() ), + '', + 'No stray cache bucket is created for clicks outside a meta box.' + ); + } ); + + QUnit.test( 'getSelectedMenuItemKey() survives the placeholder changing between renders.', function( assert ) { + var firstRender = { + 'menu-item-type': 'post_type', + 'menu-item-object': 'page', + 'menu-item-object-id': '11', + 'menu-item-url': 'https://example.org/?p=11' + }, + secondRender = $.extend( {}, firstRender ); + + assert.strictEqual( + api.getSelectedMenuItemKey( firstRender, -1 ), + api.getSelectedMenuItemKey( secondRender, -7 ), + 'The same page keeps one key even though its placeholder changed.' + ); + } ); + + QUnit.test( 'getSelectedMenuItemKey() keeps two different pages apart.', function( assert ) { + var first = { + 'menu-item-type': 'post_type', + 'menu-item-object': 'page', + 'menu-item-object-id': '11' + }, + second = $.extend( {}, first, { 'menu-item-object-id': '12' } ); + + assert.notStrictEqual( + api.getSelectedMenuItemKey( first, -1 ), + api.getSelectedMenuItemKey( second, -2 ), + 'Two different pages get different keys.' + ); + } ); + + QUnit.test( 'getSelectedMenuItemKey() keeps two custom links apart.', function( assert ) { + var first = { + 'menu-item-type': 'custom', + 'menu-item-object': 'custom', + 'menu-item-object-id': '-1', + 'menu-item-url': 'https://example.org/one', + 'menu-item-title': 'One' + }, + second = $.extend( {}, first, { + 'menu-item-url': 'https://example.org/two', + 'menu-item-title': 'Two' + } ); + + assert.notStrictEqual( + api.getSelectedMenuItemKey( first, -1 ), + api.getSelectedMenuItemKey( second, -2 ), + 'Custom links sharing a placeholder do not overwrite each other.' + ); + } ); + + QUnit.test( 'The cache stores the full item data, not just a marker.', function( assert ) { + renderScreen( metabox( 'posttype-page', 'posttypediv', [ checklistItem( 11, { checked: true } ) ] ) ); + + var metaboxEl = fixture.find( '#posttype-page' ), + cached; + + api.syncSelectedMenuItems( metaboxEl ); + cached = api.getSelectedMenuItems( metaboxEl ); + cached = cached[ Object.keys( cached )[0] ]; + + assert.strictEqual( cached['menu-item-object-id'], '11', 'The object ID is stored.' ); + assert.strictEqual( cached['menu-item-type'], 'post_type', 'The type is stored.' ); + assert.strictEqual( cached['menu-item-object'], 'page', 'The object is stored.' ); + assert.strictEqual( cached['menu-item-title'], 'Item 11', 'The title is stored.' ); + assert.strictEqual( cached['menu-item-url'], 'https://example.org/?p=11', 'The URL is stored.' ); + } ); + + QUnit.test( 'syncSelectedMenuItems() ignores inactive tab panels.', function( assert ) { + renderScreen( metabox( + 'posttype-page', + 'posttypediv', + [ checklistItem( 11, { checked: true } ) ], + [ checklistItem( 99, { checked: true } ) ] + ) ); + + var metaboxEl = fixture.find( '#posttype-page' ); + + api.syncSelectedMenuItems( metaboxEl ); + + assert.strictEqual( countSelected( metaboxEl ), 1, 'Only the visible panel is cached.' ); + } ); + + QUnit.test( 'Clicking a checkbox caches it, and clicking again removes it.', function( assert ) { + renderScreen( metabox( 'posttype-page', 'posttypediv', [ checklistItem( 11 ) ] ) ); + + var metaboxEl = fixture.find( '#posttype-page' ), + checkbox = checkboxFor( metaboxEl, 11 ); + + checkbox.trigger( 'click' ); + assert.strictEqual( countSelected( metaboxEl ), 1, 'The click handler cached the item.' ); + + checkbox.trigger( 'click' ); + assert.strictEqual( countSelected( metaboxEl ), 0, 'Clicking again removed it.' ); + } ); + + QUnit.test( 'Clicking select all caches every visible row.', function( assert ) { + renderScreen( metabox( 'posttype-page', 'posttypediv', [ + checklistItem( 11 ), + checklistItem( 12 ), + checklistItem( 13 ) + ] ) ); + + var metaboxEl = fixture.find( '#posttype-page' ); + + metaboxEl.find( '.select-all' ).trigger( 'click' ); + + assert.strictEqual( countSelected( metaboxEl ), 3, 'All three rows were cached.' ); + } ); + + QUnit.test( 'Clicking a tab clears the cache for that meta box.', function( assert ) { + renderScreen( metabox( 'posttype-page', 'posttypediv', [ checklistItem( 11, { checked: true } ) ] ) ); + + var metaboxEl = fixture.find( '#posttype-page' ); + + api.syncSelectedMenuItems( metaboxEl ); + assert.strictEqual( countSelected( metaboxEl ), 1, 'The item starts out cached.' ); + + metaboxEl.prepend( '' ); + metaboxEl.find( '.nav-tab-link' ).trigger( 'click' ); + + assert.strictEqual( countSelected( metaboxEl ), 0, 'Switching tabs emptied the cache.' ); + } ); + + QUnit.test( 'Paginating away and back restores the earlier ticks.', function( assert ) { + renderScreen( metabox( 'posttype-page', 'posttypediv', [ + checklistItem( 11, { checked: true } ), + checklistItem( 12 ), + checklistItem( 13, { checked: true } ) + ] ) ); + + var metaboxEl = fixture.find( '#posttype-page' ), + pageTwo, + pageOneAgain; + + /* + * Each response is a fresh request, so the placeholder run restarts + * and the same rows come back under different IDs. + */ + newRequest(); + pageTwo = metabox( 'posttype-page', 'posttypediv', [ checklistItem( 21 ), checklistItem( 22 ) ] ); + + $.post = function( url, data, success ) { + success( JSON.stringify( { 'replace-id': 'posttype-page', markup: pageTwo } ) ); + }; + fixture.find( 'a.page-numbers' ).trigger( 'click' ); + + metaboxEl = fixture.find( '#posttype-page' ); + assert.strictEqual( + metaboxEl.find( 'input.menu-item-checkbox:checked' ).length, + 0, + 'No page 2 row is ticked by mistake.' + ); + assert.strictEqual( countSelected( metaboxEl ), 2, 'Both page 1 selections are held.' ); + + newRequest(); + pageOneAgain = metabox( 'posttype-page', 'posttypediv', [ + checklistItem( 11 ), + checklistItem( 12 ), + checklistItem( 13 ) + ] ); + + $.post = function( url, data, success ) { + success( JSON.stringify( { 'replace-id': 'posttype-page', markup: pageOneAgain } ) ); + }; + fixture.find( 'a.page-numbers' ).trigger( 'click' ); + + metaboxEl = fixture.find( '#posttype-page' ); + assert.strictEqual( checkboxFor( metaboxEl, 11 ).prop( 'checked' ), true, 'Item 11 is ticked again.' ); + assert.strictEqual( checkboxFor( metaboxEl, 12 ).prop( 'checked' ), false, 'Item 12 stays unticked.' ); + assert.strictEqual( checkboxFor( metaboxEl, 13 ).prop( 'checked' ), true, 'Item 13 is ticked again.' ); + } ); + + QUnit.test( 'Taxonomy meta boxes keep their selections across pagination.', function( assert ) { + renderScreen( metabox( 'taxonomy-category', 'taxonomydiv', [ + checklistItem( 5, { type: 'taxonomy', object: 'category', title: 'News' } ), + checklistItem( 6, { type: 'taxonomy', object: 'category', title: 'Sport' } ) + ] ) ); + + var metaboxEl = fixture.find( '#taxonomy-category' ), + pageOneAgain; + + checkboxFor( metaboxEl, 5 ).trigger( 'click' ); + + newRequest(); + pageOneAgain = metabox( 'taxonomy-category', 'taxonomydiv', [ + checklistItem( 5, { type: 'taxonomy', object: 'category', title: 'News' } ), + checklistItem( 6, { type: 'taxonomy', object: 'category', title: 'Sport' } ) + ] ); + + $.post = function( url, data, success ) { + success( JSON.stringify( { 'replace-id': 'taxonomy-category', markup: pageOneAgain } ) ); + }; + fixture.find( 'a.page-numbers' ).trigger( 'click' ); + + metaboxEl = fixture.find( '#taxonomy-category' ); + assert.strictEqual( checkboxFor( metaboxEl, 5 ).prop( 'checked' ), true, 'The category is ticked again.' ); + assert.strictEqual( checkboxFor( metaboxEl, 6 ).prop( 'checked' ), false, 'The other category is untouched.' ); + } ); + + QUnit.test( 'Adding to the menu sends cached items from every page.', function( assert ) { + renderScreen( metabox( 'posttype-page', 'posttypediv', [ checklistItem( 11, { checked: true } ) ] ) ); + + var metaboxEl = fixture.find( '#posttype-page' ), + originalAdd = api.addItemToMenu, + pageTwo, + sent; + + api.syncSelectedMenuItems( metaboxEl ); + + newRequest(); + pageTwo = metabox( 'posttype-page', 'posttypediv', [ checklistItem( 21 ), checklistItem( 22 ) ] ); + + $.post = function( url, data, success ) { + success( JSON.stringify( { 'replace-id': 'posttype-page', markup: pageTwo } ) ); + }; + fixture.find( 'a.page-numbers' ).trigger( 'click' ); + + metaboxEl = fixture.find( '#posttype-page' ); + checkboxFor( metaboxEl, 21 ).trigger( 'click' ); + + api.addItemToMenu = function( menuItems ) { + sent = menuItems; + }; + metaboxEl.addSelectedToMenu( api.addMenuItemToBottom ); + api.addItemToMenu = originalAdd; + + var objectIds = $.map( sent, function( item ) { + return item['menu-item-object-id']; + } ).sort(); + + assert.deepEqual( + objectIds, + [ '11', '21' ], + 'The page 1 selection and the page 2 selection are both sent.' + ); + } ); + +} )( window.QUnit, jQuery ); From e76e2f36b0808689dfd3a2ebc6c67ceb4e446611 Mon Sep 17 00:00:00 2001 From: Kamran Abdul Aziz Date: Fri, 7 Aug 2026 14:41:26 +0530 Subject: [PATCH 3/3] Keep placeholder rows selected across pagination Rows that do not map to a real object, such as Home and post type archives, are rendered with the negative $_nav_menu_placeholder as their object ID. That counter restarts on every request, so keying cached selections on it dropped those rows when the checklist reloaded. Identify them by URL and title instead, which stay the same between renders. Real pages, posts and terms keep using their object ID. --- src/js/_enqueues/lib/nav-menu.js | 21 +++-- tests/qunit/wp-admin/js/nav-menu.js | 133 ++++++++++++++++++++++++++++ 2 files changed, 149 insertions(+), 5 deletions(-) diff --git a/src/js/_enqueues/lib/nav-menu.js b/src/js/_enqueues/lib/nav-menu.js index a8c8993fdb65c..78da0247374bb 100644 --- a/src/js/_enqueues/lib/nav-menu.js +++ b/src/js/_enqueues/lib/nav-menu.js @@ -458,15 +458,26 @@ }, getSelectedMenuItemKey : function( menuItemData, listItemDBID ) { - var key; + var key, + objectId = parseInt( menuItemData['menu-item-object-id'], 10 ), - if ( menuItemData['menu-item-object-id'] || menuItemData['menu-item-url'] ) { + /* + * Rows that do not map to a real object, such as Home and post + * type archives, are rendered with the negative + * $_nav_menu_placeholder as their object ID. That counter + * restarts on every request, so the same row comes back with a + * different ID after the checklist is reloaded. Identify those + * rows by URL and title instead, which stay the same. + */ + isPlaceholder = isNaN( objectId ) || objectId <= 0; + + if ( ! isPlaceholder || menuItemData['menu-item-url'] ) { key = [ menuItemData['menu-item-type'] || '', menuItemData['menu-item-object'] || '', - menuItemData['menu-item-object-id'] || '', - 'custom' === menuItemData['menu-item-type'] ? menuItemData['menu-item-url'] || '' : '', - 'custom' === menuItemData['menu-item-type'] ? menuItemData['menu-item-title'] || '' : '' + isPlaceholder ? '' : objectId, + isPlaceholder ? menuItemData['menu-item-url'] || '' : '', + isPlaceholder ? menuItemData['menu-item-title'] || '' : '' ].join( ':' ); return key.replace( /[^\w-]/g, '_' ); diff --git a/tests/qunit/wp-admin/js/nav-menu.js b/tests/qunit/wp-admin/js/nav-menu.js index 7d35e3f889504..16a580e8e676c 100644 --- a/tests/qunit/wp-admin/js/nav-menu.js +++ b/tests/qunit/wp-admin/js/nav-menu.js @@ -157,6 +157,37 @@ ''; } + /** + * Builds the synthetic Home row. + * + * When no static front page is set, nav-menu.php gives Home the + * placeholder as its object ID, so the ID changes on every render. + * + * @return {string} The `li` markup. + */ + function homeItem() { + return checklistItem( placeholder - 1, { + type: 'custom', + object: 'custom', + title: 'Home', + url: 'https://example.org/' + } ); + } + + /** + * Builds a post type archive row, which also uses a placeholder ID. + * + * @return {string} The `li` markup. + */ + function archiveItem() { + return checklistItem( placeholder - 1, { + type: 'post_type_archive', + object: 'post', + title: 'Post Archives', + url: 'https://example.org/archive/' + } ); + } + /** * Wraps items in a meta box with an active and an inactive tab panel. * @@ -325,6 +356,40 @@ ); } ); + QUnit.test( 'getSelectedMenuItemKey() identifies Home by URL, not by its placeholder.', function( assert ) { + var firstRender = { + 'menu-item-type': 'custom', + 'menu-item-object': 'custom', + 'menu-item-object-id': '-1', + 'menu-item-url': 'https://example.org/', + 'menu-item-title': 'Home' + }, + secondRender = $.extend( {}, firstRender, { 'menu-item-object-id': '-4' } ); + + assert.strictEqual( + api.getSelectedMenuItemKey( firstRender, -1 ), + api.getSelectedMenuItemKey( secondRender, -4 ), + 'Home keeps one key across renders even though its object ID is a placeholder.' + ); + } ); + + QUnit.test( 'getSelectedMenuItemKey() identifies a post type archive across renders.', function( assert ) { + var firstRender = { + 'menu-item-type': 'post_type_archive', + 'menu-item-object': 'post', + 'menu-item-object-id': '-2', + 'menu-item-url': 'https://example.org/archive/', + 'menu-item-title': 'Post Archives' + }, + secondRender = $.extend( {}, firstRender, { 'menu-item-object-id': '-9' } ); + + assert.strictEqual( + api.getSelectedMenuItemKey( firstRender, -2 ), + api.getSelectedMenuItemKey( secondRender, -9 ), + 'A post type archive keeps one key across renders.' + ); + } ); + QUnit.test( 'getSelectedMenuItemKey() keeps two custom links apart.', function( assert ) { var first = { 'menu-item-type': 'custom', @@ -467,6 +532,74 @@ assert.strictEqual( checkboxFor( metaboxEl, 13 ).prop( 'checked' ), true, 'Item 13 is ticked again.' ); } ); + QUnit.test( 'A ticked Home row survives pagination.', function( assert ) { + renderScreen( metabox( 'posttype-page', 'posttypediv', [ homeItem(), checklistItem( 12 ) ] ) ); + + var metaboxEl = fixture.find( '#posttype-page' ), + homeId = metaboxEl.find( 'input.menu-item-checkbox' ).first().val(), + pageOneAgain; + + metaboxEl.find( 'input.menu-item-checkbox' ).first().trigger( 'click' ); + assert.strictEqual( countSelected( metaboxEl ), 1, 'Home was cached.' ); + + // A later request renders Home under a different placeholder. + newRequest(); + nextPlaceholder(); + nextPlaceholder(); + pageOneAgain = metabox( 'posttype-page', 'posttypediv', [ homeItem(), checklistItem( 12 ) ] ); + + $.post = function( url, data, success ) { + success( JSON.stringify( { 'replace-id': 'posttype-page', markup: pageOneAgain } ) ); + }; + fixture.find( 'a.page-numbers' ).trigger( 'click' ); + + metaboxEl = fixture.find( '#posttype-page' ); + assert.notStrictEqual( + metaboxEl.find( 'input.menu-item-checkbox' ).first().val(), + homeId, + 'Home really did come back under a different object ID.' + ); + assert.strictEqual( + metaboxEl.find( 'input.menu-item-checkbox' ).first().prop( 'checked' ), + true, + 'Home is still ticked.' + ); + } ); + + QUnit.test( 'A ticked post type archive row survives pagination.', function( assert ) { + renderScreen( metabox( 'posttype-post', 'posttypediv', [ archiveItem(), checklistItem( 31 ) ] ) ); + + var metaboxEl = fixture.find( '#posttype-post' ), + archiveId = metaboxEl.find( 'input.menu-item-checkbox' ).first().val(), + pageOneAgain; + + metaboxEl.find( 'input.menu-item-checkbox' ).first().trigger( 'click' ); + assert.strictEqual( countSelected( metaboxEl ), 1, 'The archive row was cached.' ); + + // A later request renders the archive under a different placeholder. + newRequest(); + nextPlaceholder(); + nextPlaceholder(); + pageOneAgain = metabox( 'posttype-post', 'posttypediv', [ archiveItem(), checklistItem( 31 ) ] ); + + $.post = function( url, data, success ) { + success( JSON.stringify( { 'replace-id': 'posttype-post', markup: pageOneAgain } ) ); + }; + fixture.find( 'a.page-numbers' ).trigger( 'click' ); + + metaboxEl = fixture.find( '#posttype-post' ); + assert.notStrictEqual( + metaboxEl.find( 'input.menu-item-checkbox' ).first().val(), + archiveId, + 'The archive row really did come back under a different object ID.' + ); + assert.strictEqual( + metaboxEl.find( 'input.menu-item-checkbox' ).first().prop( 'checked' ), + true, + 'The archive row is still ticked.' + ); + } ); + QUnit.test( 'Taxonomy meta boxes keep their selections across pagination.', function( assert ) { renderScreen( metabox( 'taxonomy-category', 'taxonomydiv', [ checklistItem( 5, { type: 'taxonomy', object: 'category', title: 'News' } ),