Menu CSS Classes are saved as comma separated inside the Customiser

[paulwilde]

If you have a menu item with more than 1 CSS Class, it converts the spaces into a comma separated list.

If you alter the CSS classes, or introduce a new one it then merges all of the classes into a single string therefore breaking the output (and thus layout). You have to manually change all the commas into spaces if you want to update a menu item.

Unfortunately, I've had to steer clients away from the Customiser because of this bug.

Screenshots attached.

[tyxla]

Nav menu customizer: on menu control rendering, manually concatenate the classes element to avoid the automatic toString() concatenation with a comma glue.

[tyxla]

Since the classes property is an array, it is automatically toString()-ed before being inserted as a value of the "CSS Classes" input field.

The above patch handles this by manually turning the array into string, concatenating the classes by a space " ". This will avoid the automatic toString() which turns the spaces into commas.

Side note: We can implement this for any array properties, but I consider that we should do this only for classes property. Reason: we should respect potential menu item element properties for other types of fields that would use arrays (for example sets of checkboxes or <select multiple>). Such fields can currently be added by overriding the default admin menu walker.

[westonruter]

@tyxla Patch looks good, thanks. I would make one small change, however, and that is to not override the settingValue object value, because since it is returned by reference from setting() it will have a side-effect of mutating the underlying setting's value. So I think this would be better:

src/wp-admin/js/customize-nav-menus.js

@@ -1132 +1132 @@
                                         }
                                 });
                                 if ( settingValue ) {
-                                        element.set( settingValue[ property ] );
+                                        if ( 'classes' === property && _.isArray( settingValue[ property ] ) ) {
+                                                element.set( settingValue[ property ].join( " " ) );
+                                        } else {
+                                                element.set( settingValue[ property ] );
+                                        }
                                 }
                         });

@paulwilde Can you confirm this fixes the issue you reported?

[paulwilde]

I can confirm that the patch fixes the issue.

Edit: The snippet above also works.

[westonruter]

Aside, since xfn and classes are sanitized in the same way, I was curious why the fix wasn't also needed for xfn. Turns out that wp_setup_nav_menu_item() ensures the former is an array but the latter is not:

$menu_item->classes = ! isset( $menu_item->classes ) ? (array) get_post_meta( $menu_item->ID, '_menu_item_classes', true ) : $menu_item->classes;
$menu_item->xfn = ! isset( $menu_item->xfn ) ? get_post_meta( $menu_item->ID, '_menu_item_xfn', true ) : $menu_item->xfn;

And also in wp_update_nav_menu_item(), it stores classes in postmeta as an array, but xfn is stored as a string:

$args['menu-item-classes'] = array_map( 'sanitize_html_class', explode( ' ', $args['menu-item-classes'] ) );
$args['menu-item-xfn'] = implode( ' ', array_map( 'sanitize_html_class', explode( ' ', $args['menu-item-xfn'] ) ) );

Alas.

[westonruter]

In 34788:

Customize: Fix nav_menu_item CSS classes array being incorrectly presented in input field as comma-delimited list.

Instead of using Array.toString() to serialize an array with comma delimiters, explicitly join the array using spaces instead. Also ensure that xfn is handled properly if it ever gets stored as an array.

Props tyxla, westonruter.
Fixes #34111.

[westonruter]

In 34789:

Customize: Fix nav_menu_item CSS classes array being incorrectly presented in input field as comma-delimited list.

Instead of using Array.toString() to serialize an array with comma delimiters, explicitly join the array using spaces instead. Also ensure that xfn is handled properly if it ever gets stored as an array.

Cherry-picks [34788].

Props tyxla, westonruter.
Fixes #34111 for 4.3.

[westonruter]

In 35308:

Customizer: Prevent nav_menu_item settings from becoming dirty when their controls are set up.

Since wp_setup_nav_menu_item() returns the classes property as an array but the Customizer manages the value as a string, the setting needs to initially export the value as a string. This prevents the classes property type change from causing the setting to get marked as dirty even though nothing changed. This is a regression from [34788].

See #34111.